[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

r916286

From: Paul Burba <ptburba_at_gmail.com>
Date: Tue, 17 May 2011 16:34:28 -0400

> Author: kameshj
> Date: Thu Feb 25 13:40:22 2010
> New Revision: 916286
>
> URL: http://svn.apache.org/viewvc?rev=916286&view=rev
> Log:
> With the below apache configuration(See the trailing slash at the end
> of '/svn/').
>
> <Location /svn/>
> DAV svn
> SVNParentPath /repositories
> #See the trailing slash on the master URI also can cause the confusion.
> SVNMasterURI http://master/svn/
> SVNAdvertiseV2Protocol Off
> </Location>
>
> We get the following error on the client side.
>
> svn: Commit failed (details follow):
> svn: MKACTIVITY of
> '/svn/demujin/!svn/act/4b6d547c-018d-4e02-9d3f-2b283076cc06': Could
> not
> read status line: connection was closed by server (http://localhost)

Hi Kamesh,

Are there any threads or IRC logs in which this is discussed
(particularly a more detailed replication)? While reviewing r916286
and r917512 I tried to replicate this problem by adding a trailing '/'
to the location and SVNMasterURI. I *did* get an error, just a
different one:

C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave>svn
st
M file.txt

C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave>svn
ci -m "commit to slave"
svn: Commit failed (details follow):
svn: OPTIONS of 'http://localhost/svn-test-work/slave': 200 OK
(http://localhost)

What's worse is that I get this error with a server at the latest
trunk (r1104523), trunk right after you fixed this bug (r917512), and
my own local attempt to backport this branch to 1.6.x (addressing the
conflicts and the use of 1.7 APIs). They all fail the same way.

Any insight is appreciated.

Paul

> On the server(proxy) it is an assertion error on the following code
> block from subversion/mod_dav_svn/mirror.c:proxy_request_fixup
>
> assert((uri_segment[0] == '\0')
> || (uri_segment[0] == '/'));
>
> For the above configuration we get the uri_segment with the value
> 'reponame/some/path/inside/the/repo'.
>
> We fix this by canonicalizing the 'root_dir'(The one in Location) and
> 'uri.path'(Path portion of Master URI).
> * subversion/mod_dav_svn/dav_svn.h
> (dav_svn__get_root_dir): Document that root_dir is in canonicalized form.
> * subversion/mod_dav_svn/mod_dav_svn.c
> (create_dir_config): Canonicalize the root_dir.
> * subversion/mod_dav_svn/mirror.c
> (dav_svn__location_in_filter, dav_svn__location_body_filter):
> As root_dir is in canonical form canonicalize the uri.path too to avoid
> spurious errors.
> (dav_svn__location_header_filter): As root_dir is canonical we need to
> explicitly introduce the path seperator.
>
> Suggested by: julianfoad
>
> Modified:
> subversion/trunk/subversion/mod_dav_svn/dav_svn.h
> subversion/trunk/subversion/mod_dav_svn/mirror.c
> subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
>
> Modified: subversion/trunk/subversion/mod_dav_svn/dav_svn.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/dav_svn.h?rev=916286&r1=916285&r2=916286&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/dav_svn.h (original)
> +++ subversion/trunk/subversion/mod_dav_svn/dav_svn.h Thu Feb 25 13:40:22 2010
> @@ -352,7 +352,7 @@
> /* Return the activities db */
> const char * dav_svn__get_activities_db(request_rec *r);
>
> -/* Return the root directory */
> +/* Return the root directory in canonicalized form */
> const char * dav_svn__get_root_dir(request_rec *r);
>
>
>
> Modified: subversion/trunk/subversion/mod_dav_svn/mirror.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mirror.c?rev=916286&r1=916285&r2=916286&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/mirror.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/mirror.c Thu Feb 25 13:40:22 2010
> @@ -128,7 +128,7 @@
> locate_ctx_t *ctx = f->ctx;
> apr_status_t rv;
> apr_bucket *bkt;
> - const char *master_uri, *root_dir;
> + const char *master_uri, *root_dir, *canonicalized_uri;
> apr_uri_t uri;
>
> /* Don't filter if we're in a subrequest or we aren't setup to
> @@ -143,7 +143,11 @@
> (that is, if our root path matches that of the master server). */
> apr_uri_parse(r->pool, master_uri, &uri);
> root_dir = dav_svn__get_root_dir(r);
> - if (strcmp(uri.path, root_dir) == 0) {
> + if (uri.path)
> + canonicalized_uri = svn_dirent_canonicalize(uri.path, r->pool);
> + else
> + canonicalized_uri = uri.path;
> + if (strcmp(canonicalized_uri, root_dir) == 0) {
> ap_remove_input_filter(f);
> return ap_get_brigade(f->next, bb, mode, block, readbytes);
> }
> @@ -156,7 +160,7 @@
>
> if (!f->ctx) {
> ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> - ctx->remotepath = uri.path;
> + ctx->remotepath = canonicalized_uri;
> ctx->remotepath_len = strlen(ctx->remotepath);
> ctx->localpath = root_dir;
> ctx->localpath_len = strlen(ctx->localpath);
> @@ -226,7 +230,7 @@
> start_foo += strlen(master_uri);
> new_uri = ap_construct_url(r->pool,
> apr_pstrcat(r->pool,
> - dav_svn__get_root_dir(r),
> + dav_svn__get_root_dir(r), "/",
> start_foo, NULL),
> r);
> apr_table_set(r->headers_out, "Location", new_uri);
> @@ -240,7 +244,7 @@
> request_rec *r = f->r;
> locate_ctx_t *ctx = f->ctx;
> apr_bucket *bkt;
> - const char *master_uri, *root_dir;
> + const char *master_uri, *root_dir, *canonicalized_uri;
> apr_uri_t uri;
>
> /* Don't filter if we're in a subrequest or we aren't setup to
> @@ -255,7 +259,11 @@
> (that is, if our root path matches that of the master server). */
> apr_uri_parse(r->pool, master_uri, &uri);
> root_dir = dav_svn__get_root_dir(r);
> - if (strcmp(uri.path, root_dir) == 0) {
> + if (uri.path)
> + canonicalized_uri = svn_dirent_canonicalize(uri.path, r->pool);
> + else
> + canonicalized_uri = uri.path;
> + if (strcmp(canonicalized_uri, root_dir) == 0) {
> ap_remove_output_filter(f);
> return ap_pass_brigade(f->next, bb);
> }
> @@ -268,7 +276,7 @@
>
> if (!f->ctx) {
> ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> - ctx->remotepath = uri.path;
> + ctx->remotepath = canonicalized_uri;
> ctx->remotepath_len = strlen(ctx->remotepath);
> ctx->localpath = root_dir;
> ctx->localpath_len = strlen(ctx->localpath);
>
> Modified: subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c?rev=916286&r1=916285&r2=916286&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c Thu Feb 25
> 13:40:22 2010
> @@ -169,7 +169,8 @@
> /* NOTE: dir==NULL creates the default per-dir config */
> dir_conf_t *conf = apr_pcalloc(p, sizeof(*conf));
>
> - conf->root_dir = dir;
> + if (dir)
> + conf->root_dir = svn_dirent_canonicalize(dir, p);
> conf->bulk_updates = CONF_FLAG_ON;
> conf->v2_protocol = CONF_FLAG_ON;
>
Received on 2011-05-17 22:34:58 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.