[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1088602 - in /subversion/trunk/subversion: mod_dav_svn/mirror.c tests/cmdline/dav-mirror-autocheck.sh

From: vijay <vijay_at_collab.net>
Date: Tue, 05 Apr 2011 12:36:30 +0530

On Tuesday 05 April 2011 03:12 AM, Daniel Shahaf wrote:
> kameshj_at_apache.org wrote on Mon, Apr 04, 2011 at 12:55:38 -0000:
>> @@ -438,5 +434,29 @@ fi
>>
>> say "PASS: master, slave are both at r4, as expected"
>>
>> -exit 0
>> +# The following test case is for the regression issue triggered by r917523.
>> +# The revision r917523 do some url encodings to the paths and uris which are
>> +# not url-encoded. But there is one additional url-encoding of an uri which is
>> +# already encoded. With this extra encoding, committing a path to slave which
>> +# has space in it fails. Please see this thread
>> +# http://svn.haxx.se/dev/archive-2011-03/0641.shtml for more info.
>> +
>> +say "Test case for regression issue triggered by r917523"
>> +
>> +$svnmucc cp 2 "$BASE_URL/trunk" "$BASE_URL/branch new"
>> +$svnmucc put /dev/null "$BASE_URL/branch new/file" \
>> +--config-option servers:global:http-library=neon
>> +RETVAL=$?
>> +
> This looks bogus, why are you testing neon xor serf rather than both?
>
The reason why the particular commit operation uses neon is here
http://svn.haxx.se/dev/archive-2011-04/0021.shtml

snippet from the thread.

<snip>
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.

</snip>

The testcase is only for the regression issue triggered by r917523. If
we have something like "davmirrorautocheck" like davautocheck which
exercise all the tests in our test suite, then we can broaden the test
cases for proxy setup.

Thanks & Regards,
Vijayaguru
>> +if [ $RETVAL -eq 0 ] ; then
>> + say "PASS: committing a path which has space in it passes"
>> +else
>> + say "FAIL: committing a path which has space in it fails as there are extra
>> + url-encodings happening in server side"
>> +fi
Received on 2011-04-05 09:07:08 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.