[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH] fix for issue 2475: ignore case for hostnames (repost)

From: Lieven Govaerts <lgo_at_mobsol.be>
Date: 2006-03-30 02:38:57 CEST


attached is an updated patch, including the changes based on your remarks.

> -----Original Message-----
> From: Philip Martin [mailto:philip@codematters.co.uk]
> > + if (uri)
> > + {
> > + // convert hostname to lowercase
> What about the scheme, should that be converted as well?

Yes, the case of scheme schould be ignored too. I changed that in attached
patch. While rereading the patch, I saw that I'm using C++ style comments. I
replaced them by /* */

> Perhaps we could then remove the tolower hack in svn_ra_dav__open?

The question is, has the url that's used here always been canonicalized? My
tests seems to indicate that is the case, but I don't know if that's
guaranteed. I've removed that hack in the attached patch.

> > + apr_uri_t host_uri;
> > + apr_size_t offset;
> > + int i;
> > +
> > + apr_uri_parse(pool, path, &host_uri);
> The overlap between skip_uri_scheme and apr_uri_parse is a bit
> worrying, can we drop skip_uri_scheme just use apr_uri_parse
> everywhere?

I've dropped skip_uri_scheme in the svn_path_canonicalize function, which
made the code easier to understand as well. The function skip_uri_scheme is
still used in two other places (svn_path_is_url & svn_path_is_uri_safe).
Both can be replaced by something better, but I don't think that should be
part of this patch.

> > if (entry->repos)
> > {
> > + entry->repos = svn_path_canonicalize(entry->repos, pool);
> > if (entry->url && !
> svn_path_is_ancestor(entry->repos, entry->url))
> > return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
> > _("Entry for '%s' has
> invalid repository "
> I guess this is repairing old working copies that have stored
> non-canonical URLs, a repair that could be done manually using
> --relocate? I don't know whether our policy these days is to trust
> .svn data or not but we don't appear to force name to be canonical so
> forcing the URLs is inconsistent. I suppose we can justify it since
> we know that non-canonical URLs exist whereas the only known way to
> get non-canonical names is to modify the .svn data outside of
> Subversion.

I'm not exactly repairing old working copies here, just supporting them. I
don't think it's a good idea to ask people who have been using
'http://HOST/svn/repos' to relocate their working copies when they upgrade
to a new SVN version.

thanks for the review.


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.

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Received on Thu Mar 30 02:42:40 2006

This is an archived mail posted to the Subversion Dev mailing list.