Stefan Sperling wrote on Sun, 8 Jun 2008 at 19:24 +0200:
> Hi,
> 
> attached is a series of patches that I believe fix issue #2116
> ('svn log file:///' results in a failed assertion).
> 
> Does anyone object to any of these?
> 
> Stefan
> 
> Index: subversion/tests/libsvn_subr/path-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/path-test.c	(revision 31628)
> +++ subversion/tests/libsvn_subr/path-test.c	(working copy)
> @@ -736,6 +736,12 @@ test_canonicalize(const char **msg,
>      { "http://hst",           "http://hst" },
>      { "http://hst/foo/../bar","http://hst/foo/../bar" },
>      { "http://hst/",          "http://hst" },
> +    { "http:///",             "http://" },
> +    { "https://",             "https://" },
> +    { "file:///",             "file://" },
> +    { "file://",              "file://" },
> +    { "svn:///",              "svn://" },
> +    { "svn+ssh:///",          "svn+ssh://" },
I added
  +    { "foo//",                "foo" },
  +    { "foo///",               "foo" },
in r31634.
> 
> [[[
> 
> * subversion/libsvn_ra_local/split_url.c
>   (svn_ra_local__split_URL): Treat file:// equivalent to file:///.
>    This function used to complain about a missing hostname when
>    just passed "file://". But "file://" is the canonical version
>    of "file:///", which is equivalent to "file://localhost/".
>    skip_uri_scheme() (in subversion/libsvn_subr/path.c) and
>    therefore svn_path_is_url() have been considering "file://"
>    a valid URL since r14445.
> 
> ]]]
> 
For anyone interested, here is the 'diff -w' version of this patch:
> Index: subversion/libsvn_ra_local/split_url.c
> ===================================================================
> --- subversion/libsvn_ra_local/split_url.c	(revision 31632)
> +++ subversion/libsvn_ra_local/split_url.c	(working copy)
>    /* Then, skip what's between the "file://" prefix and the next
>       occurance of '/' -- this is the hostname, and we are considering
>       everything from that '/' until the end of the URL to be the
> -     absolute path portion of the URL. */
> +     absolute path portion of the URL.
> +     If we got just "file://", treat it the same as "file:///". */
>    hostname = URL + 7;
> +  if (*hostname == '\0')
> +    {
> +      path = "/";
> +      hostname = NULL;
> +    }
> +  else
> +    {
>    path = strchr(hostname, '/');
>    if (! path)
>      return svn_error_createf
>        (SVN_ERR_RA_ILLEGAL_URL, NULL,
>         _("Local URL '%s' contains only a hostname, no path"), URL);
>  
>    /* Treat localhost as an empty hostname. */
>    if (hostname != path)
>      {
>        hostname = svn_path_uri_decode(apr_pstrmemdup(pool, hostname,
>                                                      path - hostname), pool);
>        if (strncmp(hostname, "localhost", 9) == 0)
>          hostname = NULL;
>      }
>    else
>      hostname = NULL;
> +    }
>  
>    /* Duplicate the URL, starting at the top of the path.
>       At the same time, we URI-decode the path. */
>  #if defined(WIN32) || defined(__CYGWIN__)
>    /* On Windows, we'll typically have to skip the leading / if the
>       path starts with a drive letter.  Like most Web browsers, We
>       support two variants of this scheme:
> [[[
> 
> * subversion/libsvn_ra_serf/serf.c
>   (svn_ra_serf__open,
>    svn_ra_serf__reparent): Don't accept an empty hostname.
> 
> ]]]
> 
> Index: subversion/libsvn_ra_serf/serf.c
> ===================================================================
> --- subversion/libsvn_ra_serf/serf.c	(revision 31628)
> +++ subversion/libsvn_ra_serf/serf.c	(working copy)
> @@ -500,7 +500,7 @@ svn_ra_serf__open(svn_ra_session_t *session,
>    serf_sess->context = serf_context_create(serf_sess->pool);
>  
>    status = apr_uri_parse(serf_sess->pool, repos_URL, &url);
> -  if (status)
> +  if (status || strlen(url.hostname) == 0)
url.hostname may be NULL, see r31223 (including epg's comment there).
Actually, r31223 makes it unnecessary to check here for NULL, but for
the svn_ra_serf__reparent hunk it's still an issue.  And I don't know
about the neon patch, which doesn't use apr_uri_parse().
>      {
>        return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
>                                 _("Illegal repository URL '%s'"),
> [[[
> 
> * subversion/libsvn_ra_svn/client.c
>   (parse_url): Don't accept an empty hostname.
> 
(I said that on IRC; repeating for the list)
This, I think, would break tunnels that hard-code the hostname, such as
svn+tau:///bionets/trunk which I currently have.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-08 20:48:05 CEST