On 05/18/2011 02:04 AM, Paul Burba wrote:
>> 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,
>
Sorry for the late response Paul.
> Are there any threads or IRC logs in which this is discussed
> (particularly a more detailed replication)?
No. Paul this error caught my eyes while testing something locally.
> 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.
I could not see this error(I mean everything works as expected with
<Location /svn/> and SVNMasterURI witj trailing '/') with against
trunk_at_1126459
I built trunk_at_916285(prior to my fix) and saw this error and with
trunk_at_916286 this error has gone away.
May be something to do with win32 build.
Can you attach your patch against r916285, I will build it/test it and
let you know what is wrong?
With regards
Kamesh Jayachandran
> 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-23 16:10:45 CEST