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