Windows drive letter check fails on lower case cwd

[CC to subversion development list]

        Hi,

A few weeks ago a Subversion user reported that some of the subversion path tests failed for him. In particular our call to

apr_filepath_merge(&buffer, NULL, "C:hi", APR_FILEPATH_NOTRELATIVE, pool);

returned an error instead of an absolute path version of "hi" on drive C: for him.

List thread: http://svn.haxx.se/dev/archive-2009-10/0682.shtml

After some investigation I found that I can replicate this failure when I change the current directory to a path with a lower case drive letter.

E.g. by typing

C:\> cd c:\windows

I switch to "c:\windows" (and not C:\Windows as commonly expected).

In this specific case resolving the "C:hi" path style fails, because the check on line 543 of file_io/win32/filepath.c fails:

            if (addroot[0] == baseroot[0] && baseroot[1] == ':') {

As it assumes that the driveletters of the input and output paths always match.

After applying this simple patch:

Index: file_io/win32/filepath.c
===================================================================
--- file_io/win32/filepath.c (revision 832725)
+++ file_io/win32/filepath.c (working copy)
@@ -540,7 +540,8 @@
              * use the basepath _if_ it matches this drive letter!
              * Otherwise we must discard the basepath.
              */
- if (addroot[0] == baseroot[0] && baseroot[1] == ':') {
+ if (apr_toupper(addroot[0]) == apr_toupper(baseroot[0])
+ && baseroot[1] == ':') {
 #endif
                 /* Base the result path on the basepath
                  */

The call will return me "c:/windows/hi" and no error.

(For Subversion the answers "c:/windows/hi" and "C:/windows/hi" are both valid as we normalize the resulting drive letter to upper case directly after reading it. Normalizing to upper case might be preferred for apr itself)

Proposed log message:
[[
Use case insensitive drive letter comparison when making "C:hi"-like paths
absolute on Windows
]]

I'm not sure where/how I can add a test for this specific test in the apr test suite as I can't find a relevant test to add it too. But I did add a test in the Subversion project repository in r40370, which directly shows this behavior. (Currently the test is marked as XFail).

In the next Subversion release, this call to apr_filepath_merge() will be far more common than in all our previous releases as we switch to absolute paths for most processing, so it would be nice if we can get this issue fixed upstream well before we get to releasing Subversion 1.7 (and its planned betas).

Subversion tells its users it is compatible with apr 0.9 and 1.X, so I would also like to propose it for backport to older release's...

Thanks,

        Bert Huijben

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414408
Received on 2009-11-04 14:39:43 CET