You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2011/05/17 22:34:28 UTC
r916286
> 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;
>
Re: r916286
Posted by Paul Burba <pt...@gmail.com>.
On Mon, May 23, 2011 at 12:10 PM, Kamesh Jayachandran <ka...@collab.net> wrote:
> On 05/23/2011 09:09 PM, Paul Burba wrote:
>>
>> On Mon, May 23, 2011 at 10:13 AM, Kamesh Jayachandran<ka...@collab.net>
>> wrote:
>>>
>>> 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@1126459
>>
>> Hi Kamesh,
>>
>> Hmmm, I get yet another error if the Location and SVNMasterURI have a
>> trailing '/':
>>
>>
>> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave-1.7-r1126459>svn
>> st
>> M file.txt
>>
>> slave-1.7-r1126459>svn ci -m "Commit to proxy with trailing / in
>> Location and SVNMasterURI"
>> ..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
>> svn: E175002: Commit failed (details follow):
>> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\ra.c:356: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
>> svn: E175002: Unable to connect to a repository at URL
>> 'http://localhost/svn-test-work/slave'
>> ..\..\..\subversion\libsvn_ra_neon\util.c:1556: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra_neon\util.c:1152: (apr_err=175002)
>> svn: E175002: The OPTIONS request returned invalid XML in the
>> response: XML parse error at line 1: no element found
>> (http://localhost/svn-test-work/slave)
>>
>> Without the trailing '/' this commit succeeds as expected.
>>
>>> I built trunk@916285(prior to my fix) and saw this error and with
>>> trunk@916286 this error has gone away.
>>>
>>> May be something to do with win32 build.
>>
>> I have not used the write-through proxy functionality before so maybe
>> I botched something with my configuration? I attached my httpd.conf
>> in the event something problematic jumps out at you. Though, as I
>> said, everything works fine without the trailing '/'.
>>
>>> Can you attach your patch against r916285, I will build it/test it and
>>> let
>>> you know what is wrong?
>>
>> I don't have a patch against trunk@916285, I don't have any patch
>> against trunk at all. The only patch I mentioned is for the backport
>> of r916286 and r917512 against 1.6.x. Is this what you meant?
>>
>> Paul
>
> Yes I could reproduce your original error even while doing a checkout using
> my distro 1.6 client binaries against the following configuration.
>
>
> This issue has nothing to do with write through proxy as it happens even
> while checking out.
Yes, I am seeing this too with a patched 1.6.x and svn co/up. I
didn't notice before because I had checked out the wc prior to
tweaking httpd.conf. So yes, this is a separate issue. I'll remove
the relevant part of my veto justification, but the r916286 group
still needs a backport branch.
> <Location /svn-test-work/slave/>
> DAV svn
> SVNPath /repositories/thanu
> SVNMasterURI http://127.0.0.1:18080/svn-test-work/master/
> AuthType Basic
> AuthName Subversion
> AuthBasicProvider csvn-file-users
> AuthzSVNAccessFile "conf.d/svn_access_file"
> SVNPathAuthz short_circuit
> Require valid-user
> </Location>
>
> This error do not occur in the following configurations.
>
> * when SVNParentPath is used instead of SVNPath
>
> * When Location do not have trailing slash.
I can't get commits to the proxy to work when using SVNParentPath.
I've tried both 1.6.x@112663 (no patch) and trunk@1126621 and I can't
commit through the proxy. I get this error:
C:\SVN\sandbox\proxy-WC-sandbox\slave>svn ci -m ""
..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175013)
..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175013)
..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175013)
svn: E175013: Commit failed (details follow):
..\..\..\subversion\libsvn_client\commit_util.c:1628: (apr_err=175013)
..\..\..\subversion\libsvn_client\commit_util.c:1628: (apr_err=175013)
..\..\..\subversion\libsvn_delta\path_driver.c:174: (apr_err=175013)
..\..\..\subversion\libsvn_ra_neon\commit.c:789: (apr_err=175013)
..\..\..\subversion\libsvn_ra_neon\commit.c:333: (apr_err=175013)
..\..\..\subversion\libsvn_ra_neon\util.c:600: (apr_err=175013)
svn: E175013: Access to
'/svn-test-work/slaves/slave/!svn/act/5a79d633-58b1-d749-ae77-d8a919fc8147'
forbidden
Until I can get this to work[1] and replicate the problem you fixed in
the r916286 I can't do much to review the change (obviously!) but I
don't want to hold you up any further if you want to create backport
branch and try to get the group in for 1.6.17.
Sorry for the confusion,
Paul
[1] And that will be a while because I need to stop spinning my wheels
with this and get back to 1.7 stuffs.
> <snip of error>
> [kamesh@kamesh tmp]$ svn co http://127.0.0.1/svn-test-work/slave
> svn: OPTIONS of 'http://127.0.0.1/svn-test-work/slave': 200 OK
> (http://127.0.0.1)
> </snip>
>
> With regards
> Kamesh Jayachandran
>
Re: r916286
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 05/23/2011 09:09 PM, Paul Burba wrote:
> On Mon, May 23, 2011 at 10:13 AM, Kamesh Jayachandran<ka...@collab.net> wrote:
>> 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@1126459
> Hi Kamesh,
>
> Hmmm, I get yet another error if the Location and SVNMasterURI have a
> trailing '/':
>
> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave-1.7-r1126459>svn
> st
> M file.txt
>
> slave-1.7-r1126459>svn ci -m "Commit to proxy with trailing / in
> Location and SVNMasterURI"
> ..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
> svn: E175002: Commit failed (details follow):
> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\ra.c:356: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
> svn: E175002: Unable to connect to a repository at URL
> 'http://localhost/svn-test-work/slave'
> ..\..\..\subversion\libsvn_ra_neon\util.c:1556: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra_neon\util.c:1152: (apr_err=175002)
> svn: E175002: The OPTIONS request returned invalid XML in the
> response: XML parse error at line 1: no element found
> (http://localhost/svn-test-work/slave)
>
> Without the trailing '/' this commit succeeds as expected.
>
>> I built trunk@916285(prior to my fix) and saw this error and with
>> trunk@916286 this error has gone away.
>>
>> May be something to do with win32 build.
> I have not used the write-through proxy functionality before so maybe
> I botched something with my configuration? I attached my httpd.conf
> in the event something problematic jumps out at you. Though, as I
> said, everything works fine without the trailing '/'.
>
>> Can you attach your patch against r916285, I will build it/test it and let
>> you know what is wrong?
> I don't have a patch against trunk@916285, I don't have any patch
> against trunk at all. The only patch I mentioned is for the backport
> of r916286 and r917512 against 1.6.x. Is this what you meant?
>
> Paul
Yes I could reproduce your original error even while doing a checkout
using my distro 1.6 client binaries against the following configuration.
This issue has nothing to do with write through proxy as it happens even
while checking out.
<Location /svn-test-work/slave/>
DAV svn
SVNPath /repositories/thanu
SVNMasterURI http://127.0.0.1:18080/svn-test-work/master/
AuthType Basic
AuthName Subversion
AuthBasicProvider csvn-file-users
AuthzSVNAccessFile "conf.d/svn_access_file"
SVNPathAuthz short_circuit
Require valid-user
</Location>
This error do not occur in the following configurations.
* when SVNParentPath is used instead of SVNPath
* When Location do not have trailing slash.
<snip of error>
[kamesh@kamesh tmp]$ svn co http://127.0.0.1/svn-test-work/slave
svn: OPTIONS of 'http://127.0.0.1/svn-test-work/slave': 200 OK
(http://127.0.0.1)
</snip>
With regards
Kamesh Jayachandran
Re: r916286
Posted by Paul Burba <pt...@gmail.com>.
> I have not used the write-through proxy functionality before so maybe
> I botched something with my configuration? I attached my httpd.conf...
Resending. The attached conf file didn't come through to the list
(though Kamesh got it).
Paul
Re: r916286
Posted by Paul Burba <pt...@gmail.com>.
On Mon, May 23, 2011 at 11:53 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> On 05/23/2011 09:09 PM, Paul Burba wrote:
>>
>> On Mon, May 23, 2011 at 10:13 AM, Kamesh Jayachandran<ka...@collab.net>
>> wrote:
>>>
>>> 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@1126459
>>
>> Hi Kamesh,
>>
>> Hmmm, I get yet another error if the Location and SVNMasterURI have a
>> trailing '/':
>>
>>
>> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave-1.7-r1126459>svn
>> st
>> M file.txt
>>
>> slave-1.7-r1126459>svn ci -m "Commit to proxy with trailing / in
>> Location and SVNMasterURI"
>> ..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
>> svn: E175002: Commit failed (details follow):
>> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
>> ..\..\..\subversion\libsvn_client\ra.c:356: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
>> svn: E175002: Unable to connect to a repository at URL
>> 'http://localhost/svn-test-work/slave'
>> ..\..\..\subversion\libsvn_ra_neon\util.c:1556: (apr_err=175002)
>> ..\..\..\subversion\libsvn_ra_neon\util.c:1152: (apr_err=175002)
>> svn: E175002: The OPTIONS request returned invalid XML in the
>> response: XML parse error at line 1: no element found
>> (http://localhost/svn-test-work/slave)
>>
>
> May be you suffer from issue 3445
>>
>> Without the trailing '/' this commit succeeds as expected.
>>
>
> But issue 3445 should be present irrespective of '/' slash.
>>>
>>> I built trunk@916285(prior to my fix) and saw this error and with
>>> trunk@916286 this error has gone away.
>>>
>>> May be something to do with win32 build.
>>
>> I have not used the write-through proxy functionality before so maybe
>> I botched something with my configuration? I attached my httpd.conf
>> in the event something problematic jumps out at you. Though, as I
>> said, everything works fine without the trailing '/'.
>>
>
> Ok.. I will copy you snippets in my environment to see if you caught some
> lurking bugs.
>>>
>>> Can you attach your patch against r916285, I will build it/test it and
>>> let
>>> you know what is wrong?
>>
>> I don't have a patch against trunk@916285, I don't have any patch
>> against trunk at all. The only patch I mentioned is for the backport
>> of r916286 and r917512 against 1.6.x. Is this what you meant?
>>
>
> Sorry, I was wrong, I meant your patch against 1.6.x branch as one of my
> colleague has win32 svn build setup for 1.6.x I can build the binaries to
> see if it is peculiar to win32.
Ok, I've attached that patch.
Paul
> With regards
> Kamesh Jayachandran
>
>
>> Paul
>
>
Re: r916286
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 05/23/2011 09:09 PM, Paul Burba wrote:
> On Mon, May 23, 2011 at 10:13 AM, Kamesh Jayachandran<ka...@collab.net> wrote:
>> 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@1126459
> Hi Kamesh,
>
> Hmmm, I get yet another error if the Location and SVNMasterURI have a
> trailing '/':
>
> C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave-1.7-r1126459>svn
> st
> M file.txt
>
> slave-1.7-r1126459>svn ci -m "Commit to proxy with trailing / in
> Location and SVNMasterURI"
> ..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
> svn: E175002: Commit failed (details follow):
> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
> ..\..\..\subversion\libsvn_client\ra.c:356: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
> svn: E175002: Unable to connect to a repository at URL
> 'http://localhost/svn-test-work/slave'
> ..\..\..\subversion\libsvn_ra_neon\util.c:1556: (apr_err=175002)
> ..\..\..\subversion\libsvn_ra_neon\util.c:1152: (apr_err=175002)
> svn: E175002: The OPTIONS request returned invalid XML in the
> response: XML parse error at line 1: no element found
> (http://localhost/svn-test-work/slave)
>
May be you suffer from issue 3445
> Without the trailing '/' this commit succeeds as expected.
>
But issue 3445 should be present irrespective of '/' slash.
>> I built trunk@916285(prior to my fix) and saw this error and with
>> trunk@916286 this error has gone away.
>>
>> May be something to do with win32 build.
> I have not used the write-through proxy functionality before so maybe
> I botched something with my configuration? I attached my httpd.conf
> in the event something problematic jumps out at you. Though, as I
> said, everything works fine without the trailing '/'.
>
Ok.. I will copy you snippets in my environment to see if you caught
some lurking bugs.
>> Can you attach your patch against r916285, I will build it/test it and let
>> you know what is wrong?
> I don't have a patch against trunk@916285, I don't have any patch
> against trunk at all. The only patch I mentioned is for the backport
> of r916286 and r917512 against 1.6.x. Is this what you meant?
>
Sorry, I was wrong, I meant your patch against 1.6.x branch as one of my
colleague has win32 svn build setup for 1.6.x I can build the binaries
to see if it is peculiar to win32.
With regards
Kamesh Jayachandran
> Paul
Re: r916286
Posted by Paul Burba <pt...@gmail.com>.
On Mon, May 23, 2011 at 10:13 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> 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@1126459
Hi Kamesh,
Hmmm, I get yet another error if the Location and SVNMasterURI have a
trailing '/':
C:\SVN\src-branch-1.6.x\Debug\subversion\tests\cmdline\svn-test-work\working_copies\slave-1.7-r1126459>svn
st
M file.txt
slave-1.7-r1126459>svn ci -m "Commit to proxy with trailing / in
Location and SVNMasterURI"
..\..\..\subversion\svn\commit-cmd.c:190: (apr_err=175002)
..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
..\..\..\subversion\libsvn_client\commit.c:846: (apr_err=175002)
svn: E175002: Commit failed (details follow):
..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
..\..\..\subversion\libsvn_client\commit.c:644: (apr_err=175002)
..\..\..\subversion\libsvn_client\ra.c:356: (apr_err=175002)
..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
..\..\..\subversion\libsvn_ra\ra_loader.c:496: (apr_err=175002)
svn: E175002: Unable to connect to a repository at URL
'http://localhost/svn-test-work/slave'
..\..\..\subversion\libsvn_ra_neon\util.c:1556: (apr_err=175002)
..\..\..\subversion\libsvn_ra_neon\util.c:1152: (apr_err=175002)
svn: E175002: The OPTIONS request returned invalid XML in the
response: XML parse error at line 1: no element found
(http://localhost/svn-test-work/slave)
Without the trailing '/' this commit succeeds as expected.
> I built trunk@916285(prior to my fix) and saw this error and with
> trunk@916286 this error has gone away.
>
> May be something to do with win32 build.
I have not used the write-through proxy functionality before so maybe
I botched something with my configuration? I attached my httpd.conf
in the event something problematic jumps out at you. Though, as I
said, everything works fine without the trailing '/'.
> Can you attach your patch against r916285, I will build it/test it and let
> you know what is wrong?
I don't have a patch against trunk@916285, I don't have any patch
against trunk at all. The only patch I mentioned is for the backport
of r916286 and r917512 against 1.6.x. Is this what you meant?
Paul
Re: r916286
Posted by Kamesh Jayachandran <ka...@collab.net>.
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@1126459
I built trunk@916285(prior to my fix) and saw this error and with
trunk@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;
>>