apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bert Huijben <B.Huij...@competence.biz>
Subject Windows drive letter check fails on lower case cwd
Date Wed, 04 Nov 2009 13:39:15 GMT
[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


Mime
View raw message