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