kame...@apache.org wrote: > Author: kameshj > Date: Thu Feb 25 13:40:22 2010 > New Revision: 916286
Hi Kamesh. Some review comments below ... > 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) > > > 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); Oops, you called "dirent_canonicalize" on a URI. > + 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); And here. > + 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); And is this "root_dir" meant to be a disk path or a URI? I'm not sure, myself. > conf->bulk_updates = CONF_FLAG_ON; > conf->v2_protocol = CONF_FLAG_ON; - Julian
