Thanks for the review. Attached a new patch with the changes based on your
remarks.
> -----Original Message-----
> From: Julian Foad [mailto:julianfoad@btopenworld.com]
> >
[..]
> > *
> > + * 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?
Fixed.
>
> > Index: subversion/libsvn_subr/path.c
> > ===================================================================
> > --- subversion/libsvn_subr/path.c (revision 19084)
> > +++ subversion/libsvn_subr/path.c (working copy)
>
[..]
> > + /* 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 "+".)
Fixed.
[..]
> > +
> > +### End of file.
> > +#!/usr/bin/env python
>
> Er, three.
That happens when you revert a patch to test the unit tests, and then
reapply it. Fixed.
> > +
> > +### 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.)
Good point, it provides a better test (with minimal effort :). Fixed.
> > + { "file:///Users/jrandom/wc", "file:///Users/jrandom/wc" },
> > #if defined(WIN32) || defined(__CYGWIN__)
> > /* We permit UNC paths on Windows. By definition UNC
>
> Regards,
>
> - Julian
[[[
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/libsvn_ra_dav/session.c:
(svn_ra_dav__open): cleaned up now obsolete tolower hack.
* subversion/tests/cmdline/path_tests.py:
new file, contains tests for issue #2475.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 30 08:26:21 2006