On 02/25/2010 08:29 PM, Julian Foad wrote:
[email protected] 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.
Is there any uri canonicalize function?.
FWIW here uri.path is the PATH portion of the URL, i.e something like
/svn/blah/blah
+ 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.
Same as above comment.
+ 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.
May be the disk path if the context is driven by <Directory
/some/path/to/repo>, though I never tried that way.
For the <Location /svn/> configuration root_dir is '/svn'(after
canonicaliztion.
Thanks for the review.
With regards
Kamesh Jayachandran
