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
Fix 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.
* subversion/tests/cmdline/dav-mirror-autocheck.sh
Add a testcase for a regression issue triggered by r917523.
* subversion/mod_dav_svn/mirror.c
(dav_svn__location_header_filter): Remove redundant url-encoding of new_uri.
Patch by: Vijayaguru G <vijay{_AT_}collab.net>
Index: subversion/mod_dav_svn/mirror.c
===================================================================
--- subversion/mod_dav_svn/mirror.c (revision 1084891)
+++ subversion/mod_dav_svn/mirror.c (working copy)
@@ -241,7 +241,6 @@
dav_svn__get_root_dir(r), "/",
start_foo, (char *)NULL),
r);
- new_uri = svn_path_uri_encode(new_uri, r->pool);
apr_table_set(r->headers_out, "Location", new_uri);
}
return ap_pass_brigade(f->next, bb);
Index: subversion/tests/cmdline/dav-mirror-autocheck.sh
===================================================================
--- subversion/tests/cmdline/dav-mirror-autocheck.sh (revision 1084891)
+++ subversion/tests/cmdline/dav-mirror-autocheck.sh (working copy)
@@ -399,10 +399,6 @@
say "svn log on $BASE_URL : "
$SVN --username jrandom --password rayjandom log -vq "$BASE_URL"
-# shut it down
-echo -n "${SCRIPT}: stopping httpd: "
-$HTTPD -f $HTTPD_CONFIG -k stop
-echo "."
# verify result: should be at rev 4 in both repos
# FIXME: do more rigorous verification here
@@ -421,5 +417,27 @@
say "PASS: master, slave are both at r4, as expected"
+# 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.
+
+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"
+
+if [ "$?" != "0" ] ;
+then
+ say "FAIL: committing a path which has space in it fails as there are extra
+ url-encodings happening in server side"
+else
+ say "PASS: committing a path which has space in it passes"
+fi
+
+# shut it down
+echo -n "${SCRIPT}: stopping httpd: "
+$HTTPD -f $HTTPD_CONFIG -k stop
+echo "."
exit 0
-
