Re: Interesting problem with ":" in mod_dav_svn XML output

Kalle Olavi Niemitalo wrote:
> Michael Sinz <Michael.Sinz@sinz.org> writes:
>
>
>>Well, it fixes it by prepending the "./" to the element, much like
>>my kludge patch, only with less overhead (same new allocation) and with
>>the "./" prepended only if there is a ":" before any "/"
>>
>>So, yes, this looks like a good way to fix this. (My other way was to
>>%3A the ":" but that is not needed by using the ap_os_escape_path() function.
>
> I think this was a good decision by the Apache team. There
> typically will be a slash before the colon, so escaping won't be
> necessary. If there isn't, "./::" is shorter than "%3A%3A" and
> has no semantic difference.

It is an interesting choice since it makes an escaped path dependant on
prior parts of it. While this is strictly correct, it also makes for bugs
like this one since the ap_escape_uri() did not look if there was a way
to deal with the ":" before a "/" case. They could have just as easily
done the escaping of ":" until they passed the first "/" in the path.

The real problem is that there is no good way to retrofit this into the
current Apache API since sometimes a complete URI is passed in, not just
the path part, and the ":" characters there need to remain in place.

(No one ever said the URI escaping/encoding is easy if you
only get all or part of a URI and are not told which it is :-)

>> <li>TestCase-\-/</li>
>
> Did you run this test on Windows? As ap_os_escape_path is
> documented to take "an OS path", I worry that a Windows version
> of Apache might treat the backslash as an OS-specific directory
> separator and helpfully map it to the forward slash used in
> hierarchic URIs. I did not find such code in server/util.c nor
> in os/win32/util_win32.c, but who knows whether it will be added
> later?

I have not run it on Windows but, if you look at the source to Apache,
the ap_escape_uri(a,b) is just a #define for ap_os_escape_path(a,b,1)
(see include/httpd.h)

I am rather sure that this filename would cause problems on a Windows
client but would not/should not be a problem for a Subversion server
running on Windows (mod_dav_svn or svnserver)

 From what I can see, the path escaping is for RFC 1808 paths and not
local OS paths other than a minor need for some extra work in the
client. It would be very broken to have Apache running on Windows
handle URLs differently than Apache running on "everything else"

(That is, externally - internally Apache needs to talk to the local OS
file system APIs and thus may need to different work but that is not
what this API is for)

The only difference I see for the escaping rules for different OS versions
is with respect to the T_ESCAPE_SHELL_CMD and it only adds two characters
for that case in WIN32/OS2 land (the \r and % characters)

The T_ESCAPE_PATH_SEGMENT and T_OS_ESCAPE_PATH are both constants for
all platforms. (T_ESCAPE_PATH_SEGMENT just adds "/" to the T_OS_ESCAPE_PATH)

This can all be seen in the server/gen_test_char.c code, which generates
the character escaping tables that the various escape functions use.

Also, from reading the code and some notes, it seems that the "ap_os_escape_path"
name is a bit of a mistaken name as it is not really an OS specific
concept but may have been thought to have been at one point.

>>Kalle Olavi Niemitalo wrote:
>>
>>>Another option would be to call ap_escape_path_segment and append
>>>the slash only afterward, but then it would have to be appended
>>>separately to both name and href, leading to more complex code.
>>
>>As I said, I tried that - but ap_escape_path_segment() does not deal with
>>the ":" in any way.
>
> I'm sorry about the misinformation.

No problem - I thought it should have worked too, and to me it seems that it
would have been right "correct" choice since we know the name is just a single
segment. There is a very unlikely (not supported in any reasonable way) case
of a repository file/directory having a "/" in it that would be more correctly
addressed by using the ap_escape_path_segment form but there is no real benefit
to even thinking about that since I have no idea how that would be supported
elsewhere within the system and on the physical platforms. (How would one make
such a file?)

-- 
Michael Sinz                     Technology and Engineering Director/Consultant
"Starting Startups"                                mailto:michael.sinz@sinz.org
My place on the web                            http://www.sinz.org/Michael.Sinz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org