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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>
Received on 2011-04-04 14:58:55 CEST