Hi, I can confirm that I have tested a previous version of the fix. Currently there is a separate test case for to test mirroring: subversion/tests/cmdline/dav-mirror-autocheck.sh. This works fine with your patch.
I wanted to include this in the normal davautocheck (subversion/tests/cmdline/davautocheck.sh) so we can test mirroring as well as part of the normal testing, but I dropped the ball due to other business, really sorry about that. For me it would be acceptable to commit the patch - I can do that in the next two or three days unless someone objects. I will try to have a look at completing the migration to davautocheck.se as well - feel fee to take a look if you have the time. Kind regards, Daniel Den fre 29 maj 2026 kl 16:20 skrev Jordan Peck <[email protected]>: > Hi, update on the patch. I've tidied the code for the bug fix and also > included an updated proxy/mirror test (dav-mirror-autocheck.sh) to > catch/test the issue. > > I've run the updated test without my bugfix and as expected, it fails with > this error: > svnmucc: E175015: The HTTP method 'COPY' is not allowed on > '/slave/!svn/rvr/2/trunk' > svnmucc: E160013: Can't delete node at 'branch' as it does not exist > > With the bugfix the tests all pass > > It would be great to get this fix in. Let me know if you need anything > else from me. > > Thanks, > Jordan > > On Wed, 25 Mar 2026 at 18:02, Jordan Peck <[email protected]> wrote: > >> Hi, just wanted to bump this. I'm not sure if this is on anyone's radar, >> it would be great to get this fix integrated. >> Thanks >> >> On Thu, 29 Jan 2026 at 15:22, Jordan Peck <[email protected]> >> wrote: >> >>> Patch bugfix. >>> I discovered the previous patch had a URL encoding issue that caused >>> COPY/MOVE >>> operations to fail for paths containing special characters (e.g., >>> spaces). >>> >>> The Destination header value is already URL-encoded by the client >>> (e.g., "/svn/repo/file%20name.txt"). The fix now properly handles this >>> by: >>> >>> 1. Encoding root_dir before comparison with the already-encoded path >>> 2. Encoding master_path before concatenation >>> 3. NOT re-encoding the final path (which would double-encode the >>> remainder portion, turning %20 into %2520) >>> >>> This follows the same pattern used by the existing body rewrite filters >>> (dav_svn__location_in_filter and dav_svn__location_body_filter), which >>> also encode their search patterns because "incoming request body has >>> it encoded already." >>> >>> I have attached the updated patch. >>> >>> Thanks, >>> Jordan >>> >>> >>> On Wed, 21 Jan 2026 at 10:30, Daniel Sahlberg < >>> [email protected]> wrote: >>> >>>> Den tis 20 jan. 2026 kl 20:56 skrev Jordan Peck via dev < >>>> [email protected]>: >>>> >>>>> Small update, after some testing I'm now running our company SVN >>>>> server with this patch. Everything is working smoothly and the proxy >>>>> issues >>>>> are resolved. >>>>> >>>>> I've attached an updated version of the patch which fixes some code >>>>> formatting. >>>>> >>>>> Thanks, >>>>> Jordan >>>>> >>>> >>>> Thank you so much for the patch and the detailed report! >>>> >>>> I've taken a quick look and it all seems good, but I need to study the >>>> surrounding code in more detail to fully understand it. However I havn't >>>> found the time yet. >>>> >>>> To any other takers! We have a test for this in >>>> https://svn.apache.org/repos/asf/subversion/trunk/subversion/tests/cmdline/dav-mirror-autocheck.sh >>>> >>>> Updating the settings around lines 369-376 (ie, changing the >>>> MASTER_LOCATION and SLAVE_LOCATION as indicated by the comments): >>>> [[[ >>>> # location directive elements for master,slave,sync >>>> # tests currently work if master==slave,fail if different >>>> # ** Should different locations for each work? >>>> #MASTER_LOCATION="master" >>>> #SLAVE_LOCATION="slave" >>>> MASTER_LOCATION="repo" >>>> SLAVE_LOCATION="repo" >>>> SYNC_LOCATION="sync" >>>> ]]] >>>> >>>> Without the patch, the test fails with: >>>> [[[ >>>> ... >>>> dav-mirror-autocheck.sh: running svnmucc test to >>>> http://127.0.0.1:1989/slave >>>> r1 committed by jrandom at 2026-01-21T10:20:43.736512Z >>>> r2 committed by jrandom at 2026-01-21T10:20:43.861355Z >>>> svnmucc: E175015: The HTTP method 'COPY' is not allowed on >>>> '/slave/!svn/rvr/2/trunk' >>>> svnmucc: E160013: Can't delete node at 'branch' as it does not exist >>>> ... >>>> ]]] >>>> >>>> With the patch, the test succeeds, as it does when the MASTER_LOCATION >>>> and SLAVE_LOCATION are the same. >>>> >>>> dav-mirror-autocheck.sh is run from a checkout of /trunk as follows: >>>> $ APACHE_MPM=event ./subversion/tests/cmdline/dav-mirror-autocheck.sh >>>> /tmp/test-work >>>> >>>> davautocheck is a separate target in Makefile. I'd like to add >>>> dav-mirror-autocheck the same way (it seems to be easy enough) possibly >>>> also making a "check it all" target for release testing. >>>> >>>> Cheers, >>>> Daniel >>>> >>>> >>>> >>>>> On Mon, 19 Jan 2026 at 23:38, C. Michael Pilato <[email protected]> >>>>> wrote: >>>>> >>>>>> Not really. I suspect the pool of reviewers is just small and >>>>>> occupied with other activity. All volunteers 'round here. >>>>>> >>>>>> On Mon, Jan 19, 2026 at 12:48 PM Jordan Peck <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Hi Mike, >>>>>>> Thanks for having a look, is there anything else I have to do for >>>>>>> this to be considered for merging into trunk? >>>>>>> >>>>>>> Thanks, >>>>>>> Jordan >>>>>>> >>>>>>> On Wed, 14 Jan 2026 at 20:22, C. Michael Pilato <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> I'm too long out of the game to offer meaningful review and testing >>>>>>>> on this, but I definitely appreciate the thorough write-up. And given >>>>>>>> the >>>>>>>> comments I left sprinkled in that source file (so many years ago...) >>>>>>>> about >>>>>>>> how inefficient some of the filtering was when the master/slave >>>>>>>> repository >>>>>>>> paths were identical, chances are pretty good that I did most of my >>>>>>>> testing >>>>>>>> in exactly that configuration. >>>>>>>> >>>>>>>> -- Mike >>>>>>>> >>>>>>>> On Wed, Jan 14, 2026 at 7:18 AM Jordan Peck via dev < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> I've identified and fixed a bug in mod_dav_svn's master/slave proxy >>>>>>>>> functionality (SVNMasterURI) that causes COPY and MOVE operations >>>>>>>>> to >>>>>>>>> fail with a 400 error when the slave and master repositories are >>>>>>>>> hosted at different paths. >>>>>>>>> >>>>>>>>> >>>>>>>>> STEPS TO REPRODUCE >>>>>>>>> ================== >>>>>>>>> >>>>>>>>> 1. Configure a master SVN server with the repository at /repo: >>>>>>>>> >>>>>>>>> # Master server (master-server.example.com) >>>>>>>>> <Location /repo> >>>>>>>>> DAV svn >>>>>>>>> SVNPath /var/svn/myrepo >>>>>>>>> </Location> >>>>>>>>> >>>>>>>>> 2. Configure a slave SVN server with the repository at a different >>>>>>>>> path (/svn/repo) and SVNMasterURI pointing to the master: >>>>>>>>> >>>>>>>>> # Slave server (slave-server.example.com) >>>>>>>>> <Location /svn/repo> >>>>>>>>> DAV svn >>>>>>>>> SVNPath /var/svn/myrepo-slave >>>>>>>>> SVNMasterURI http://master-server.example.com/repo >>>>>>>>> </Location> >>>>>>>>> >>>>>>>>> 3. Ensure the slave repository is a synced mirror of the master >>>>>>>>> (via svnsync). >>>>>>>>> >>>>>>>>> 4. Attempt a COPY operation through the slave to create a branch: >>>>>>>>> >>>>>>>>> svn copy http://slave-server.example.com/svn/repo/trunk \ >>>>>>>>> >>>>>>>>> http://slave-server.example.com/svn/repo/branches/new-feature \ >>>>>>>>> -m "Creating new feature branch" >>>>>>>>> >>>>>>>>> >>>>>>>>> EXPECTED RESULT >>>>>>>>> =============== >>>>>>>>> >>>>>>>>> The COPY request is proxied to the master and the branch is created >>>>>>>>> successfully. >>>>>>>>> >>>>>>>>> >>>>>>>>> ACTUAL RESULT >>>>>>>>> ============= >>>>>>>>> >>>>>>>>> The operation fails with HTTP 400 Bad Request. Server logs show the >>>>>>>>> master receiving a Destination header with path >>>>>>>>> /svn/repo/branches/new-feature which does not exist on the master >>>>>>>>> (the correct path would be /repo/branches/new-feature). >>>>>>>>> >>>>>>>>> >>>>>>>>> ROOT CAUSE >>>>>>>>> ========== >>>>>>>>> >>>>>>>>> In subversion/mod_dav_svn/mirror.c, the proxy_request_fixup() >>>>>>>>> function >>>>>>>>> correctly rewrites the Request-URI when proxying requests to the >>>>>>>>> master. >>>>>>>>> However, WebDAV COPY and MOVE operations specify their destination >>>>>>>>> in >>>>>>>>> the Destination HTTP header, which is not rewritten. >>>>>>>>> >>>>>>>>> The existing filters handle: >>>>>>>>> - Request body rewriting (dav_svn__location_in_filter) >>>>>>>>> - Response body rewriting (dav_svn__location_body_filter) >>>>>>>>> - Response Location header rewriting >>>>>>>>> (dav_svn__location_header_filter) >>>>>>>>> >>>>>>>>> But there is no rewriting for the incoming Destination request >>>>>>>>> header. >>>>>>>>> >>>>>>>>> When paths are identical on slave and master, this bug is masked >>>>>>>>> because the path portion extracted from the Destination header >>>>>>>>> happens >>>>>>>>> to be valid on both servers. The bug only manifests when the >>>>>>>>> repository >>>>>>>>> paths differ. >>>>>>>>> >>>>>>>>> >>>>>>>>> FIX >>>>>>>>> === >>>>>>>>> >>>>>>>>> The attached patch adds rewriting of the Destination header in >>>>>>>>> proxy_request_fixup(). It parses the header, replaces the slave's >>>>>>>>> root >>>>>>>>> path with the master's path, and updates the header before the >>>>>>>>> request >>>>>>>>> is proxied. >>>>>>>>> >>>>>>>>> I have tested this fix and can confirm it fixes the issue in my >>>>>>>>> testing. >>>>>>>>> >>>>>>>>> Please review the attached patch. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Jordan Peck >>>>>>>>> >>>>>>>>
