> -----Original Message----- > From: Branko Čibej [mailto:br...@wandisco.com] > Sent: vrijdag 23 augustus 2013 06:55 > To: dev@subversion.apache.org > Subject: Re: Assertion in svn_uri_is_canonical > > On 23.08.2013 06:51, Branko Čibej wrote: > > On 23.08.2013 01:28, Branko Čibej wrote: > >> On 23.08.2013 01:04, Philip Martin wrote: > >>> Branko Čibej <br...@wandisco.com> writes: > >>> > >>>> On 22.08.2013 22:40, Branko Čibej wrote: > >>>>> On 22.08.2013 18:11, kmra...@rockwellcollins.com wrote: > >>>>>> Passing an invalid URL to svn co causes an abort and core dump. This > >>>>>> fails with all protocols > >>>>>> http, svn, file. It occurs with all versions I tested (1.7.5, 1.7.11, > >>>>>> 1.7.12, 1.8.1, and the now defunct 1.8.2) > >>>>>> It occurs with multiple subcommand (ls, info, etc.) It happens on > >>>>>> both unix and windows platforms. > >>>>>> The "abort" is especially bad on Windows since it will pop open a > >>>>>> dialog window due to the abort. > >>>>>> > >>>>>> It is expected that the command line would return an appropriate > user > >>>>>> friendly error message > >>>>>> instead of crashing when faced with invalid input. > >>>>>> > >>>>>> > >>>>>> ./svn co file://./test <file://test> > >>>>>> svn: subversion/libsvn_subr/dirent_uri.c:1315: svn_uri_basename: > >>>>>> Assertion `svn_uri_is_canonical(uri, ((void *)0))' failed. > >>>>>> Abort (core dumped) > >>>>> Interesting ... the assertion itself is fine, however, the command-line > >>>>> client should either reject invalid URL parameters, or canonicalize the > >>>>> input. Apparently we missed one. > >>>> Apparently all commands that accept an URL will abort in this way, > >>>> except for "svn relocate". So it looks like some kind of "policy" but I > >>>> think it's the wrong one. > >>> svn_uri_canonicalize allows any characters in hostname, A-Z is converted > >>> to a-z, other characters are simply copied: > >>> > >>> /* Found a hostname, convert to lowercase and copy to dst. */ > >>> if (*src == '[') > >>> { > >>> ... > >>> } > >>> else > >>> while (*src && (*src != '/') && (*src != ':')) > >>> *(dst++) = canonicalize_to_lower((*src++)); > >>> > >>> What is the canonical form of this? > >>> > >>> scheme://./ > >>> > >>> Should we drop '.' to give: > >>> > >>> scheme:/// > >>> > >>> or do we have to retain it as > >>> > >>> scheme://./ > >>> > >>> and change svn_uri_is_canonical to allow a hostname '.'? > >> I believe this is relevant:
One associated problem is that our code sometimes assumes that svn_uri_canonicalize(URL, ..) can canonicalize any string to a valid url, while this is probably not the case in this specific case. We inherited this assumption from the old svn_path_*() functions that always succeeded by chopping of components, which worked because "" is a valid relative path. (I don't know what the best solution in this case is for a generic scheme. file:// is different from svn:// and http:// so to what does this apply). And in file:// urls we only accept more than trivial hostnames on Windows. Bert