Lieven Govaerts wrote:
[...]
Lieven, I don't have time to do a full review but it looks like Philip is doing
that. I just want to say that all your answers in that post sound good to me,
and point out a few things in the patch.
>
> [[[
> Fix for issue #2475: Make sure scheme and hostname in URI's are handled in a
> case insenitive manner.
>
> Patch by: Lieven Govaerts <lgo@mobsol.be>
>
> Found by: Andy Somerville
>
> * subversion/libsvn_subr/path.c:
> (svn_path_canonicalize): convert scheme and hostname to lowercase.
>
> * subversion/libsvn_wc/entries.c:
> (svn_wc__atts_to_entry): while reading from the entries file,
> canonicalize url and repos.
>
> * subversion/include/svn_path.h:
> (svn_path_canonicalize): add comment
>
> * subversion/tests/libsvn_subr/path-test.c:
> (test_canonicalize): add test for uppercase hostnames
>
> * subversion/tests/cmdline/path_tests.py:
> new file, contains tests for issue #2475.
Also mention the change you are making to "session.c".
> ]]]
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/include/svn_path.h
> ===================================================================
> --- subversion/include/svn_path.h (revision 19084)
> +++ subversion/include/svn_path.h (working copy)
> @@ -185,6 +185,9 @@
> * separator characters, and possibly other semantically inoperative
> * transformations.
> *
> + * Convert the scheme and hostname to lowercase case-insensitive handling
> + * of hostnames ( see issue 2475 )
That comment doesn't make very much sense to me; are there some words missing
or something?
> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/path.c (revision 19084)
> +++ subversion/libsvn_subr/path.c (working copy)
> + uri = host_uri.scheme ? TRUE : FALSE;
>
> /* If this is an absolute path, then just copy over the initial
> separator character. */
> @@ -1139,6 +1132,33 @@
>
> }
>
> + if (uri)
> + {
> + /* convert scheme and hostname to lowercase */
> + apr_size_t offset;
> + int i;
> +
> + for(i = 0; host_uri.scheme[i]; i++)
> + host_uri.scheme[i] = tolower(host_uri.scheme[i]);
> + for(i = 0; host_uri.hostname[i]; i++)
> + host_uri.hostname[i] = tolower(host_uri.hostname[i]);
> +
> + /* path will be pointing to a new memory location, so update src to
> + * point to the new location too. */
> + offset = strlen(host_uri.scheme)+3; // "(scheme)://"
You've still got a C++/C99-style comment there. (Also we like spaces around
operators like "+".)
> + path = apr_uri_unparse(pool, &host_uri, APR_URI_UNP_REVEALPASSWORD);
> +
> + /* skip 3rd '/' in file:/// uri */
> + if (path[offset] == '/')
> + offset++;
> +
> + /* copy src to dst */
> + memcpy(dst, path, offset);
> + dst += offset;
> +
> + src = path + offset;
> + }
> +
> Index: subversion/tests/cmdline/path_tests.py
> ===================================================================
> --- subversion/tests/cmdline/path_tests.py (revision 0)
> +++ subversion/tests/cmdline/path_tests.py (revision 0)
> @@ -0,0 +1,561 @@
> +#!/usr/bin/env python
> +#
> +# path_tests.py: testing case insensitive hostname handling.
> +#
> +# Subversion is a tool for revision control.
> +# See http://subversion.tigris.org for more information.
> +#
> +# ====================================================================
> +# Copyright (c) 2000-2006 CollabNet. All rights reserved.
[...]
> +# list all tests here, starting with None:
> +test_list = [ None,
> + path_move_between_wcs,
> + path_copy_between_wcs,
> + path_copy_in_repo,
> + path_checkout
> + ]
> +
> +if __name__ == '__main__':
> + svntest.main.run_tests(test_list)
> + # NOTREACHED
> +
> +
> +### End of file.
> +#!/usr/bin/env python
Oops! You seem to have concatenated two files here.
> +#
> +# path_tests.py: testing case insensitive hostname handling.
> +#
> +# Subversion is a tool for revision control.
> +# See http://subversion.tigris.org for more information.
> +#
> +# ====================================================================
> +# Copyright (c) 2000-2006 CollabNet. All rights reserved.
[...]
> +if __name__ == '__main__':
> + svntest.main.run_tests(test_list)
> + # NOTREACHED
> +
> +
> +### End of file.
> +#!/usr/bin/env python
Er, three.
> +#
> +# path_tests.py: testing case insensitive hostname handling.
> +#
> +# Subversion is a tool for revision control.
> +# See http://subversion.tigris.org for more information.
> +#
> +# ====================================================================
> +# Copyright (c) 2000-2006 CollabNet. All rights reserved.
[...]
> +if __name__ == '__main__':
> + svntest.main.run_tests(test_list)
> + # NOTREACHED
> +
> +
> +### End of file.
> Index: subversion/tests/libsvn_subr/path-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/path-test.c (revision 19084)
> +++ subversion/tests/libsvn_subr/path-test.c (working copy)
> @@ -670,6 +670,12 @@
> { "http://hst", "http://hst" },
> { "http://hst/foo/../bar","http://hst/foo/../bar" },
> { "http://hst/", "http://hst" },
> + { "http://HST/", "http://hst" },
> + { "http://HST/FOO/BaR", "http://hst/FOO/BaR" },
> + { "svn+ssh://j.random@HST/HST/FOO/BaR", "svn+ssh://j.random@hst/HST/FOO/BaR" },
> + { "svn+ssh://j.random:jray_at_HST/HST/FOO/BaR", "svn+ssh://j.random:jray_at_hst/HST/FOO/BaR" },
> + { "SVN+ssh://j.random:jray_at_HST/HST/FOO/BaR", "svn+ssh://j.random:jray_at_hst/HST/FOO/BaR" },
I suggest using at least some upper-case letters in every part of the URL
(incl. user name and password), to ensure that they don't accidentally get
lower-cased. (Assuming that's the correct behaviour.)
> + { "file:///Users/jrandom/wc", "file:///Users/jrandom/wc" },
> #if defined(WIN32) || defined(__CYGWIN__)
> /* We permit UNC paths on Windows. By definition UNC
Regards,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 30 03:53:30 2006