You need to be aware that APR_FILEPATH_TRUENAME must construct
a canonical path name. That is why you are having so many problems.
It seems that you are asking for ../ to be treated as a 'relative root'. That
means that only foo/bar should be normalized.
I'd be happy to look at your issues and patch later this weekend. Just
wanted you to know that the note wasn't ignored before then :-)
Bill
At 12:42 PM 8/8/2003, cmpilato@collab.net wrote:
>Subversion users have been noticing some annoying behavior in our use
>of apr_filepath_merge() that only happens on Windows platforms. We
>call this function with the APR_FILEPATH_TRUENAME flag to get the case
>and segment delimiters all in shape. We always pass "" for the
>basepath. The addpath comes straight from whatever targets the user
>passes on the commandline, so it can be absolute, relative, etc.
>
>The specific problem we are seeing on Windows is that if you call
>apr_filepath_merge("", "..\foo\bar") it returns "foo/bar", which is,
>of course, *not* the same path that was provided to the function. If
>you call apr_filepath_merge(NULL, "..\foo\bar"), you get a correct
>response, but it's an absolute path -- not what we want.
>
>I spent a few hours debugging this function, and found three concerns:
>
> 1. There was a flag test that used "&&" instead of "&" -- obvious
> bug.
>
> 2. There was a check for "is this path trying to go above the
> root" which is not conditional upon whether or not a root has
> even been calculated.
>
>I fixed these two things, cross my fingers, and re-ran my tests. This
>time, however, apr_filepath_merge("", "..\foo\bar") returned
>"baz/foo/bar", where "baz" is the name of the "foo"'s parent
>directory. So:
>
> 3. There was a loop which is calling apr_lstat() to find the true
> names of each path segment, but doesn't ignore "." and "..".
>
>I have a patch which I'd like someone with the k-nowledge to review.
>Also, I'm unfamiliar with the apr and httpd testing setups -- anyone
>who can assist me with self-verification of this change would be
>greatly appreciated.
>
>-------------------------------------------------------------------
>Log Message:
>
>* file_io/win32/filepath.c
> (apr_filepath_merge): Fix a flag test to use '&' instead of '&&'.
> Don't count it as "going above the root" when no root is known.
> Skip the apr_lstat() calls on "." and ".." path segments.
>
>Index: filepath.c
>===================================================================
>RCS file: /home/cvspublic/apr/file_io/win32/filepath.c,v
>retrieving revision 1.41
>diff -u -r1.41 filepath.c
>--- filepath.c 16 Feb 2003 21:59:08 -0000 1.41
>+++ filepath.c 8 Aug 2003 17:23:04 -0000
>@@ -704,7 +704,7 @@
> #endif
>
> /* backpath (../) */
>- if (pathlen <= rootlen)
>+ if (rootlen && (pathlen <= rootlen))
> {
> /* Attempt to move above root. Always die if the
> * APR_FILEPATH_SECUREROOTTEST flag is specified.
>@@ -733,7 +733,7 @@
> */
> if (pathlen + 3 >= sizeof(path))
> return APR_ENAMETOOLONG;
>- memcpy(path + pathlen, ((flags && APR_FILEPATH_NATIVE)
>+ memcpy(path + pathlen, ((flags & APR_FILEPATH_NATIVE)
> ? "..\\" : "../"), 3);
> pathlen += 3;
> }
>@@ -869,6 +869,24 @@
> break;
> }
> }
>+
>+ /* Skip over the '.' and '..' segments. */
>+ if ((seglen == 1)
>+ && (path[keptlen + seglen - 1] == '.')) {
>+ keptlen += seglen;
>+ if (saveslash)
>+ keptlen++;
>+ continue;
>+ }
>+ if ((seglen == 2)
>+ && (path[keptlen + seglen - 1] == '.')
>+ && (path[keptlen + seglen - 2] == '.')) {
>+ keptlen += seglen;
>+ if (saveslash)
>+ keptlen++;
>+ continue;
>+ }
>+
> /* Null term for stat! */
> path[keptlen + seglen] = '\0';
> if ((rv = apr_lstat(&finfo, path,
|