kameshj_at_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
Received on 2010-02-25 15:59:55 CET