You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by vijay <vi...@collab.net> on 2011/03/24 11:41:31 UTC
[PATCH] Remove redundant url-encoding added in r917523
Hi,
This patch adds a testcase for the regression triggered by r917523 and
fixes it.
The revision r917523 do some url-encodings to the paths and uris in
subversion/mod_dav_svn/mirror.c.
<snip>
$svn log -r917523
------------------------------------------------------------------------
r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) | 26 lines
With the below apache configuration(See the <space> character "/svn 1/").
<Location "/svn 1/">
DAV svn
SVNParentPath /repositories
</Location>
<Location "/svn 2/">
DAV svn
SVNParentPath /repositories-slave
SVNMasterURI "http://localhost/svn 1"
</Location>
Write through proxy is *not* happening and commit happens *directly*
inside the slave.
* subversion/mod_dav_svn/mirror.c
(proxy_request_fixup): URI encode the to be proxied file name.
(dav_svn__proxy_request_fixup): r->unparsed_uri is in url encoded
form while
root_dir is not in encoded form. So use r->uri to compare with
root_dir.
(dav_svn__location_in_filter): URL Encode the 'find & replace' urls as
the request body has it in url encoded format.
(dav_svn__location_header_filter): Encode the master_uri as the
response from
master has the Location header url encoded already. Set the
outgoing Location
header url encoded.
(dav_svn__location_body_filter): URL Encode the 'find & replace' urls as
the response body has it in url encoded format.
------------------------------------------------------------------------
</snip>
There is one extra url encoding in
mirror.c:dav_svn__location_header_filter() which encodes the Location
header response from master second time, which in turn causes failure
while committing a path to slave repo which has space in it. The space
in the path is encoded as "%20" first time, again it is encoded as
"%2520", which fails with error "File not found" while committing.
I have added a testcase for this issue in
subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used the
existing master and slave repo setup for my new test case.
I have a plan of introducing "davmirrorautocheck" which we can execute
like "make davmirrorautocheck", it will run all our python test suites
via write-through proxy. All commits to the slave will be proxied to the
master repo. The master repo's post commit hook will sync every commit
to the slave repo. We can check whether slave and master repo are in
sync in 'run_and_verify_commit'.
We can extensively test our write-through proxy by making use of all our
existing tests.
Attaching the patch and log message.
Thanks & Regards,
Vijayaguru
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by Kamesh Jayachandran <ka...@collab.net>.
Thanks Vijay for the detailed summary and the fix.
I committed it at r1088602.
With regards
Kamesh Jayachandran
On 04/04/2011 12:00 PM, vijay wrote:
> On Thursday 24 March 2011 07:41 PM, Kamesh Jayachandran wrote:
>> On 03/24/2011 05:54 PM, vijay wrote:
>>> Just now I came to know that it fails in neon only.
>>>
>>> I configured neon as a default ra_layer in my runtime configuration
>>> area.
>>>
>>> When I use serf as ra_layer, the commit succeeds.
>>
>> Yes that answers the failure I observed.
>>
>> Please provide a testcase that exhibits the same failure for both
>> neon and serf.
>
> The testcase exhibits the following behaviour when tested with neon
> and serf.
>
> RA_LAYER
> HTTP V2
> pre-HTTP V2
> Neon
> Fail
> Fail
> Serf
> Pass
> Fail
>
>
> There are two options to make the testcase fail for both neon and serf.
>
> 1.Disabling "SVNAdvertiseV2Protocol" by making it explicitly off in
> the slave server's Location directive in Apache configuration.
>
> 2. Passing the command line option "--config-option
> servers:global:http-library=neon" to svnmucc for the particular commit
> operation as it fails in neon consistently.
>
> I prefer to the second option(using neon for the particular commit)
> instead of disabling HTTP V2 in server side which may affect other
> testcases.
>
>> Meanwhile analysing why it succeeds in serf would teach something
>> interesting.
>>
>
> First, let me tell why it fails in neon.
>
> As in the test case, the slave repo is hosted in 127.0.0.1 and master
> repo is hosted in 127.0.0.2
>
> 1. For the particular commit which has space in its path, the CHECKOUT
> request from client reaches the slave repo.
>
> CHECKOUT /repo/!svn/ver/5/branch%20new HTTP/1.1
> User-Agent: SVN/1.7.0-dev (under development) neon/0.29.3
>
> 2. The request from the client is proxied by the slave to the master
> and the Master sends the response with Location header as follows.
>
> Header Name: [location], Value:
> [http://127.0.0.2:30679/repo//!svn/wrk/69179a87-38cb-43c4-945c-de1e0b297aad/branch%20new]
>
> 3. The slave encodes the location header again(r917523) and forward it
> to the client.
>
> Header Name: [location], Value:
> [http://127.0.0.1:26248/repo//!svn/wrk/5b604aca-a4b1-41fc-87ae-51299bf565b2/branch%2520new]
>
> The above location header is used in subsequent dav request which
> fails as it has double-encoding of the space.
>
> Here, PUT request fails as follows.
>
> PUT
> /repo/!svn/wrk/5b604aca-a4b1-41fc-87ae-51299bf565b2/branch%2520new/file HTTP/1.1
>
> svnmucc: E160013: File not found: transaction '5-5', path
> '/branch%20new/file'
>
> But why didn't it fail in serf?
>
> Serf has the implementations of HTTP V2 stuffs which does not use
> CHECKOUT request during commit.
>
> It processes as follows.
>
> OPTIONS -> POST -> PROPFIND -> PROPPATCH -> HEAD -> PUT -> MERGE -> DELETE
>
> It gets the transaction id from POST request and directly put the
> contents there.
>
> But I could see the same failure while committing with serf when I
> disable "SVNAdvertiseV2Protocol" , because it uses the CHECKOUT
> request there.
>
> I think we can use neon for the particular commit operation instead of
> disabling HTTP V2 wholly in server side.
>
> Please correct me if I am wrong.
>
> Anyway we can enhance these tests further once we started to implement
> "make davmirrorautocheck" which I am going to take as my next activity.
>
> Attaching the patch and log message.
>
> Thanks & Regards,
> Vijayaguru
>
>
>> With regards
>> Kamesh Jayachandran
>>>
>>> Thanks & Regards,
>>> vijayaguru
>>>
>>> On Thursday 24 March 2011 05:12 PM, Kamesh Jayachandran wrote:
>>>> Thanks for the Patch Vijay.
>>>>
>>>> In my FC14 your testcase passes both with and without patch.
>>>>
>>>> I am investigating....
>>>>
>>>> Will get back.
>>>>
>>>> With regards
>>>> Kamesh Jayachandran
>>>> On 03/24/2011 04:11 PM, vijay wrote:
>>>>> Hi,
>>>>>
>>>>> This patch adds a testcase for the regression triggered by r917523
>>>>> and fixes it.
>>>>>
>>>>> The revision r917523 do some url-encodings to the paths and uris
>>>>> in subversion/mod_dav_svn/mirror.c.
>>>>>
>>>>> <snip>
>>>>> $svn log -r917523
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>> r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) |
>>>>> 26 lines
>>>>>
>>>>> With the below apache configuration(See the <space> character
>>>>> "/svn 1/").
>>>>>
>>>>> <Location "/svn 1/">
>>>>> DAV svn
>>>>> SVNParentPath /repositories
>>>>> </Location>
>>>>> <Location "/svn 2/">
>>>>> DAV svn
>>>>> SVNParentPath /repositories-slave
>>>>> SVNMasterURI "http://localhost/svn 1"
>>>>> </Location>
>>>>>
>>>>> Write through proxy is *not* happening and commit happens
>>>>> *directly* inside the slave.
>>>>>
>>>>> * subversion/mod_dav_svn/mirror.c
>>>>> (proxy_request_fixup): URI encode the to be proxied file name.
>>>>> (dav_svn__proxy_request_fixup): r->unparsed_uri is in url
>>>>> encoded form while
>>>>> root_dir is not in encoded form. So use r->uri to compare with
>>>>> root_dir.
>>>>> (dav_svn__location_in_filter): URL Encode the 'find & replace'
>>>>> urls as
>>>>> the request body has it in url encoded format.
>>>>> (dav_svn__location_header_filter): Encode the master_uri as the
>>>>> response from
>>>>> master has the Location header url encoded already. Set the
>>>>> outgoing Location
>>>>> header url encoded.
>>>>> (dav_svn__location_body_filter): URL Encode the 'find & replace'
>>>>> urls as
>>>>> the response body has it in url encoded format.
>>>>>
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>> </snip>
>>>>>
>>>>> There is one extra url encoding in
>>>>> mirror.c:dav_svn__location_header_filter() which encodes the
>>>>> Location header response from master second time, which in turn
>>>>> causes failure while committing a path to slave repo which has
>>>>> space in it. The space in the path is encoded as "%20" first time,
>>>>> again it is encoded as "%2520", which fails with error "File not
>>>>> found" while committing.
>>>>>
>>>>> I have added a testcase for this issue in
>>>>> subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used
>>>>> the existing master and slave repo setup for my new test case.
>>>>>
>>>>> I have a plan of introducing "davmirrorautocheck" which we can
>>>>> execute like "make davmirrorautocheck", it will run all our python
>>>>> test suites via write-through proxy. All commits to the slave will
>>>>> be proxied to the master repo. The master repo's post commit hook
>>>>> will sync every commit to the slave repo. We can check whether
>>>>> slave and master repo are in sync in 'run_and_verify_commit'.
>>>>> We can extensively test our write-through proxy by making use of
>>>>> all our existing tests.
>>>>>
>>>>> Attaching the patch and log message.
>>>>>
>>>>> Thanks & Regards,
>>>>> Vijayaguru
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by vijay <vi...@collab.net>.
On Thursday 24 March 2011 07:41 PM, Kamesh Jayachandran wrote:
> On 03/24/2011 05:54 PM, vijay wrote:
>> Just now I came to know that it fails in neon only.
>>
>> I configured neon as a default ra_layer in my runtime configuration
>> area.
>>
>> When I use serf as ra_layer, the commit succeeds.
>
> Yes that answers the failure I observed.
>
> Please provide a testcase that exhibits the same failure for both neon
> and serf.
The testcase exhibits the following behaviour when tested with neon and
serf.
RA_LAYER
HTTP V2
pre-HTTP V2
Neon
Fail
Fail
Serf
Pass
Fail
There are two options to make the testcase fail for both neon and serf.
1.Disabling "SVNAdvertiseV2Protocol" by making it explicitly off in the
slave server's Location directive in Apache configuration.
2. Passing the command line option "--config-option
servers:global:http-library=neon" to svnmucc for the particular commit
operation as it fails in neon consistently.
I prefer to the second option(using neon for the particular commit)
instead of disabling HTTP V2 in server side which may affect other
testcases.
> Meanwhile analysing why it succeeds in serf would teach something
> interesting.
>
First, let me tell why it fails in neon.
As in the test case, the slave repo is hosted in 127.0.0.1 and master
repo is hosted in 127.0.0.2
1. For the particular commit which has space in its path, the CHECKOUT
request from client reaches the slave repo.
CHECKOUT /repo/!svn/ver/5/branch%20new HTTP/1.1
User-Agent: SVN/1.7.0-dev (under development) neon/0.29.3
2. The request from the client is proxied by the slave to the master and
the Master sends the response with Location header as follows.
Header Name: [location], Value:
[http://127.0.0.2:30679/repo//!svn/wrk/69179a87-38cb-43c4-945c-de1e0b297aad/branch%20new]
3. The slave encodes the location header again(r917523) and forward it
to the client.
Header Name: [location], Value:
[http://127.0.0.1:26248/repo//!svn/wrk/5b604aca-a4b1-41fc-87ae-51299bf565b2/branch%2520new]
The above location header is used in subsequent dav request which fails
as it has double-encoding of the space.
Here, PUT request fails as follows.
PUT
/repo/!svn/wrk/5b604aca-a4b1-41fc-87ae-51299bf565b2/branch%2520new/file
HTTP/1.1
svnmucc: E160013: File not found: transaction '5-5', path
'/branch%20new/file'
But why didn't it fail in serf?
Serf has the implementations of HTTP V2 stuffs which does not use
CHECKOUT request during commit.
It processes as follows.
OPTIONS -> POST -> PROPFIND -> PROPPATCH -> HEAD -> PUT -> MERGE -> DELETE
It gets the transaction id from POST request and directly put the
contents there.
But I could see the same failure while committing with serf when I
disable "SVNAdvertiseV2Protocol" , because it uses the CHECKOUT request
there.
I think we can use neon for the particular commit operation instead of
disabling HTTP V2 wholly in server side.
Please correct me if I am wrong.
Anyway we can enhance these tests further once we started to implement
"make davmirrorautocheck" which I am going to take as my next activity.
Attaching the patch and log message.
Thanks & Regards,
Vijayaguru
> With regards
> Kamesh Jayachandran
>>
>> Thanks & Regards,
>> vijayaguru
>>
>> On Thursday 24 March 2011 05:12 PM, Kamesh Jayachandran wrote:
>>> Thanks for the Patch Vijay.
>>>
>>> In my FC14 your testcase passes both with and without patch.
>>>
>>> I am investigating....
>>>
>>> Will get back.
>>>
>>> With regards
>>> Kamesh Jayachandran
>>> On 03/24/2011 04:11 PM, vijay wrote:
>>>> Hi,
>>>>
>>>> This patch adds a testcase for the regression triggered by r917523
>>>> and fixes it.
>>>>
>>>> The revision r917523 do some url-encodings to the paths and uris in
>>>> subversion/mod_dav_svn/mirror.c.
>>>>
>>>> <snip>
>>>> $svn log -r917523
>>>> ------------------------------------------------------------------------
>>>>
>>>> r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) |
>>>> 26 lines
>>>>
>>>> With the below apache configuration(See the <space> character "/svn
>>>> 1/").
>>>>
>>>> <Location "/svn 1/">
>>>> DAV svn
>>>> SVNParentPath /repositories
>>>> </Location>
>>>> <Location "/svn 2/">
>>>> DAV svn
>>>> SVNParentPath /repositories-slave
>>>> SVNMasterURI "http://localhost/svn 1"
>>>> </Location>
>>>>
>>>> Write through proxy is *not* happening and commit happens
>>>> *directly* inside the slave.
>>>>
>>>> * subversion/mod_dav_svn/mirror.c
>>>> (proxy_request_fixup): URI encode the to be proxied file name.
>>>> (dav_svn__proxy_request_fixup): r->unparsed_uri is in url encoded
>>>> form while
>>>> root_dir is not in encoded form. So use r->uri to compare with
>>>> root_dir.
>>>> (dav_svn__location_in_filter): URL Encode the 'find & replace'
>>>> urls as
>>>> the request body has it in url encoded format.
>>>> (dav_svn__location_header_filter): Encode the master_uri as the
>>>> response from
>>>> master has the Location header url encoded already. Set the
>>>> outgoing Location
>>>> header url encoded.
>>>> (dav_svn__location_body_filter): URL Encode the 'find & replace'
>>>> urls as
>>>> the response body has it in url encoded format.
>>>>
>>>> ------------------------------------------------------------------------
>>>>
>>>> </snip>
>>>>
>>>> There is one extra url encoding in
>>>> mirror.c:dav_svn__location_header_filter() which encodes the
>>>> Location header response from master second time, which in turn
>>>> causes failure while committing a path to slave repo which has
>>>> space in it. The space in the path is encoded as "%20" first time,
>>>> again it is encoded as "%2520", which fails with error "File not
>>>> found" while committing.
>>>>
>>>> I have added a testcase for this issue in
>>>> subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used
>>>> the existing master and slave repo setup for my new test case.
>>>>
>>>> I have a plan of introducing "davmirrorautocheck" which we can
>>>> execute like "make davmirrorautocheck", it will run all our python
>>>> test suites via write-through proxy. All commits to the slave will
>>>> be proxied to the master repo. The master repo's post commit hook
>>>> will sync every commit to the slave repo. We can check whether
>>>> slave and master repo are in sync in 'run_and_verify_commit'.
>>>> We can extensively test our write-through proxy by making use of
>>>> all our existing tests.
>>>>
>>>> Attaching the patch and log message.
>>>>
>>>> Thanks & Regards,
>>>> Vijayaguru
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 03/24/2011 05:54 PM, vijay wrote:
> Just now I came to know that it fails in neon only.
>
> I configured neon as a default ra_layer in my runtime configuration area.
>
> When I use serf as ra_layer, the commit succeeds.
Yes that answers the failure I observed.
Please provide a testcase that exhibits the same failure for both neon
and serf.
Meanwhile analysing why it succeeds in serf would teach something
interesting.
With regards
Kamesh Jayachandran
>
> Thanks & Regards,
> vijayaguru
>
> On Thursday 24 March 2011 05:12 PM, Kamesh Jayachandran wrote:
>> Thanks for the Patch Vijay.
>>
>> In my FC14 your testcase passes both with and without patch.
>>
>> I am investigating....
>>
>> Will get back.
>>
>> With regards
>> Kamesh Jayachandran
>> On 03/24/2011 04:11 PM, vijay wrote:
>>> Hi,
>>>
>>> This patch adds a testcase for the regression triggered by r917523
>>> and fixes it.
>>>
>>> The revision r917523 do some url-encodings to the paths and uris in
>>> subversion/mod_dav_svn/mirror.c.
>>>
>>> <snip>
>>> $svn log -r917523
>>> ------------------------------------------------------------------------
>>>
>>> r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) |
>>> 26 lines
>>>
>>> With the below apache configuration(See the <space> character "/svn
>>> 1/").
>>>
>>> <Location "/svn 1/">
>>> DAV svn
>>> SVNParentPath /repositories
>>> </Location>
>>> <Location "/svn 2/">
>>> DAV svn
>>> SVNParentPath /repositories-slave
>>> SVNMasterURI "http://localhost/svn 1"
>>> </Location>
>>>
>>> Write through proxy is *not* happening and commit happens *directly*
>>> inside the slave.
>>>
>>> * subversion/mod_dav_svn/mirror.c
>>> (proxy_request_fixup): URI encode the to be proxied file name.
>>> (dav_svn__proxy_request_fixup): r->unparsed_uri is in url encoded
>>> form while
>>> root_dir is not in encoded form. So use r->uri to compare with
>>> root_dir.
>>> (dav_svn__location_in_filter): URL Encode the 'find & replace'
>>> urls as
>>> the request body has it in url encoded format.
>>> (dav_svn__location_header_filter): Encode the master_uri as the
>>> response from
>>> master has the Location header url encoded already. Set the
>>> outgoing Location
>>> header url encoded.
>>> (dav_svn__location_body_filter): URL Encode the 'find & replace'
>>> urls as
>>> the response body has it in url encoded format.
>>>
>>> ------------------------------------------------------------------------
>>>
>>> </snip>
>>>
>>> There is one extra url encoding in
>>> mirror.c:dav_svn__location_header_filter() which encodes the
>>> Location header response from master second time, which in turn
>>> causes failure while committing a path to slave repo which has space
>>> in it. The space in the path is encoded as "%20" first time, again
>>> it is encoded as "%2520", which fails with error "File not found"
>>> while committing.
>>>
>>> I have added a testcase for this issue in
>>> subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used
>>> the existing master and slave repo setup for my new test case.
>>>
>>> I have a plan of introducing "davmirrorautocheck" which we can
>>> execute like "make davmirrorautocheck", it will run all our python
>>> test suites via write-through proxy. All commits to the slave will
>>> be proxied to the master repo. The master repo's post commit hook
>>> will sync every commit to the slave repo. We can check whether slave
>>> and master repo are in sync in 'run_and_verify_commit'.
>>> We can extensively test our write-through proxy by making use of all
>>> our existing tests.
>>>
>>> Attaching the patch and log message.
>>>
>>> Thanks & Regards,
>>> Vijayaguru
>>>
>>>
>>>
>>>
>>>
>>
>
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by vijay <vi...@collab.net>.
Just now I came to know that it fails in neon only.
I configured neon as a default ra_layer in my runtime configuration area.
When I use serf as ra_layer, the commit succeeds.
Thanks & Regards,
vijayaguru
On Thursday 24 March 2011 05:12 PM, Kamesh Jayachandran wrote:
> Thanks for the Patch Vijay.
>
> In my FC14 your testcase passes both with and without patch.
>
> I am investigating....
>
> Will get back.
>
> With regards
> Kamesh Jayachandran
> On 03/24/2011 04:11 PM, vijay wrote:
>> Hi,
>>
>> This patch adds a testcase for the regression triggered by r917523
>> and fixes it.
>>
>> The revision r917523 do some url-encodings to the paths and uris in
>> subversion/mod_dav_svn/mirror.c.
>>
>> <snip>
>> $svn log -r917523
>> ------------------------------------------------------------------------
>> r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) | 26
>> lines
>>
>> With the below apache configuration(See the <space> character "/svn
>> 1/").
>>
>> <Location "/svn 1/">
>> DAV svn
>> SVNParentPath /repositories
>> </Location>
>> <Location "/svn 2/">
>> DAV svn
>> SVNParentPath /repositories-slave
>> SVNMasterURI "http://localhost/svn 1"
>> </Location>
>>
>> Write through proxy is *not* happening and commit happens *directly*
>> inside the slave.
>>
>> * subversion/mod_dav_svn/mirror.c
>> (proxy_request_fixup): URI encode the to be proxied file name.
>> (dav_svn__proxy_request_fixup): r->unparsed_uri is in url encoded
>> form while
>> root_dir is not in encoded form. So use r->uri to compare with
>> root_dir.
>> (dav_svn__location_in_filter): URL Encode the 'find & replace' urls as
>> the request body has it in url encoded format.
>> (dav_svn__location_header_filter): Encode the master_uri as the
>> response from
>> master has the Location header url encoded already. Set the
>> outgoing Location
>> header url encoded.
>> (dav_svn__location_body_filter): URL Encode the 'find & replace'
>> urls as
>> the response body has it in url encoded format.
>>
>> ------------------------------------------------------------------------
>> </snip>
>>
>> There is one extra url encoding in
>> mirror.c:dav_svn__location_header_filter() which encodes the Location
>> header response from master second time, which in turn causes failure
>> while committing a path to slave repo which has space in it. The
>> space in the path is encoded as "%20" first time, again it is encoded
>> as "%2520", which fails with error "File not found" while committing.
>>
>> I have added a testcase for this issue in
>> subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used
>> the existing master and slave repo setup for my new test case.
>>
>> I have a plan of introducing "davmirrorautocheck" which we can
>> execute like "make davmirrorautocheck", it will run all our python
>> test suites via write-through proxy. All commits to the slave will be
>> proxied to the master repo. The master repo's post commit hook will
>> sync every commit to the slave repo. We can check whether slave and
>> master repo are in sync in 'run_and_verify_commit'.
>> We can extensively test our write-through proxy by making use of all
>> our existing tests.
>>
>> Attaching the patch and log message.
>>
>> Thanks & Regards,
>> Vijayaguru
>>
>>
>>
>>
>>
>
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by Kamesh Jayachandran <ka...@collab.net>.
Thanks for the Patch Vijay.
In my FC14 your testcase passes both with and without patch.
I am investigating....
Will get back.
With regards
Kamesh Jayachandran
On 03/24/2011 04:11 PM, vijay wrote:
> Hi,
>
> This patch adds a testcase for the regression triggered by r917523 and
> fixes it.
>
> The revision r917523 do some url-encodings to the paths and uris in
> subversion/mod_dav_svn/mirror.c.
>
> <snip>
> $svn log -r917523
> ------------------------------------------------------------------------
> r917523 | kameshj | 2010-03-01 19:18:01 +0530 (Mon, 01 Mar 2010) | 26
> lines
>
> With the below apache configuration(See the <space> character "/svn 1/").
>
> <Location "/svn 1/">
> DAV svn
> SVNParentPath /repositories
> </Location>
> <Location "/svn 2/">
> DAV svn
> SVNParentPath /repositories-slave
> SVNMasterURI "http://localhost/svn 1"
> </Location>
>
> Write through proxy is *not* happening and commit happens *directly*
> inside the slave.
>
> * subversion/mod_dav_svn/mirror.c
> (proxy_request_fixup): URI encode the to be proxied file name.
> (dav_svn__proxy_request_fixup): r->unparsed_uri is in url encoded
> form while
> root_dir is not in encoded form. So use r->uri to compare with
> root_dir.
> (dav_svn__location_in_filter): URL Encode the 'find & replace' urls as
> the request body has it in url encoded format.
> (dav_svn__location_header_filter): Encode the master_uri as the
> response from
> master has the Location header url encoded already. Set the
> outgoing Location
> header url encoded.
> (dav_svn__location_body_filter): URL Encode the 'find & replace'
> urls as
> the response body has it in url encoded format.
>
> ------------------------------------------------------------------------
> </snip>
>
> There is one extra url encoding in
> mirror.c:dav_svn__location_header_filter() which encodes the Location
> header response from master second time, which in turn causes failure
> while committing a path to slave repo which has space in it. The space
> in the path is encoded as "%20" first time, again it is encoded as
> "%2520", which fails with error "File not found" while committing.
>
> I have added a testcase for this issue in
> subversion/tests/cmdline/dav-mirror-autocheck.sh. I have just used the
> existing master and slave repo setup for my new test case.
>
> I have a plan of introducing "davmirrorautocheck" which we can execute
> like "make davmirrorautocheck", it will run all our python test suites
> via write-through proxy. All commits to the slave will be proxied to
> the master repo. The master repo's post commit hook will sync every
> commit to the slave repo. We can check whether slave and master repo
> are in sync in 'run_and_verify_commit'.
> We can extensively test our write-through proxy by making use of all
> our existing tests.
>
> Attaching the patch and log message.
>
> Thanks & Regards,
> Vijayaguru
>
>
>
>
>
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by vijay <vi...@collab.net>.
On Thursday 24 March 2011 10:09 PM, Peter Samuelson wrote:
>> <Location "/svn 1/">
>> DAV svn
>> SVNParentPath /repositories
>> </Location>
>> <Location "/svn 2/">
>> DAV svn
>> SVNParentPath /repositories-slave
>> SVNMasterURI "http://localhost/svn 1"
>> </Location>
> That seems like an incorrect configuration. The SVNMasterURI, like any
> other URI, should not have an unescaped space in it.
>
> I don't know if that changes the nature of the test or bug.
I am not using the above apache configuration for my test case. It was
used for r917523.
With the above apache configuration, write-through proxy was not
happening and commit went directly into slave before r917523. Kamesh
fixed it by encoding some paths and urls in
subversion/mod_dav_svn/mirror.c. But due to one extra encoding,
committing a path (from slave wc) which has a space in it fails. This
testcase tests the particular scenario.
You can check my test case's apache configuration in
subversion/tests/cmdline/dav-mirror-autocheck.sh:setup_config().
Thanks & Regards,
Vijayaguru
Re: [PATCH] Remove redundant url-encoding added in r917523
Posted by Peter Samuelson <pe...@p12n.org>.
> <Location "/svn 1/">
> DAV svn
> SVNParentPath /repositories
> </Location>
> <Location "/svn 2/">
> DAV svn
> SVNParentPath /repositories-slave
> SVNMasterURI "http://localhost/svn 1"
> </Location>
That seems like an incorrect configuration. The SVNMasterURI, like any
other URI, should not have an unescaped space in it.
I don't know if that changes the nature of the test or bug.
--
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/