This is further fall out from the 54611 and the subsequent attempt to fix the ABI in PR 55397. An easy way to see this issue is to setup mod_dav/mod_dav_svn like so: <Location /svn> DAV svn SVNPath /path/to/repo </Location> And then do the following svnadmin create /path/to/repo echo something | svnmucc -m 'add a file' -- put - 'http://127.0.0.1/svn/a<b' svn ls http://127.0.0.1/svn Various versions of Subversion will produce different error message. For example (but by no means likely to be exhaustive): svn: E130003: Malformed XML: not well-formed (invalid token) svn: E175002: PROPFIND of '/svn/!svn/rvr/2': 207 Multi-Status (http://127.0.0.1) svn: E175009: XML parsing failed: (207 Multi-Status) dav_xml_escape_uri() assumes that the URI is already encoded and as such presumes that it only needs to do XML escaping if there is an '&' character in the URI. Since characters like '<' and '>' would already be encoded by the URI encoding. Prior to PR 55397 this function also URI encoded, but this was removed since it resulted in double encoding. During the work for PR 55397 my attempt to audit for all places Subversion set resource->uri fields apparently missed the PROPFIND walker case (or I wrongly presumed that the uri we being set was already encoded). This presents the problem that now not even Subversion in consistent. Which means we essentially have some cases where resource->uri must be encoded and some cases where it does not. Changing the filename to 'a<&b' will make the symptom disappear for 1.7.x/1.8.x Subversion clients because that will trigger XML encoding from dav_xml_escape_uri(). However, 1.6.x clients (and probably older clients) are not happy the the URI not being URI encoded (even though the XML is valid now) and produce an error like this: svn: Unable to parse URL '/svn/!svn/bc/1/a<&b' A similar behavior was noted by Stephane Chazelas on PR 55397, though he saw the issue with a filename with a space in it (again valid XML but not valid URI encoding) and running log on the path. Stephane had also made a bug over at Ubuntu here: https://bugs.launchpad.net/ubuntu/+source/subversion/+bug/1284641 So simply fixing to properly emit valid XML is not enough in and of itself. It will take further thought and investigation as to what the proper fix is here.
Created attachment 31602 [details] potential patch The following patch should fix it. I'm still working on making sure there isn't some additional side-effects from it. Posting it here in case some finds it useful in the interim. [[[ Fix PR 56480: PROPFIND walker doesn't encode hrefs properly Reverts r1529559 partially (specifically the dav_xml_escape_uri) bit. Reverts r1531505 entirely. * modules/dav/main/mod_dav.c (dav_xml_escape_uri): Revert the piece of r1529559 that removes the URI escaping from this function. * modules/dav/main/mod_dav.h (dav_resource): Note the inconsistency in the documentation. * modules/dav/fs/repos.c (dav_fs_get_resource): Don't use the unparsed_uri to set the uri field of the resource. This is the correct fix for the double encoding in mod_dav_fs that led to the dav_xml_escape_uri() change and r1531505. (dav_fs_walker, dav_fs_append_uri): Revert r1531505 changes. ]]]
Created attachment 31705 [details] potential patch version 4 Improved patch that fixes something I found in the process of writing a test suite for mod_dav_fs to validate the previous fix. [[[ Fix PR 56480: PROPFIND walker doesn't encode hrefs properly Reverts r1529559 partially (specifically the dav_xml_escape_uri) bit. Reverts r1531505 entirely. * modules/dav/main/mod_dav.c (dav_xml_escape_uri): Revert the piece of r1529559 that removes the URI escaping from this function. * modules/dav/main/props.c (dav_do_prop_subreq): Escape the URI before doing a sub request with it. This resolves some properties like getcontenttype from failing to be returned for files that contain characters that require encoding in their path. * modules/dav/main/mod_dav.h (dav_resource): Note the inconsistency in the documentation. * modules/dav/fs/repos.c (dav_fs_get_resource): Don't use the unparsed_uri to set the uri field of the resource. This is the correct fix for the double encoding in mod_dav_fs that led to the dav_xml_escape_uri() change and r1531505. (dav_fs_walker, dav_fs_append_uri): Revert r1531505 changes. ]]]
Fixed and released in 2.4.10