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

Re: crash with incomplete url

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-06-25 03:40:57 CEST

Stefan Küng wrote:
> Malcolm Rowe wrote:
>>
>> See e.g. this thread: http://svn.haxx.se/dev/archive-2005-12/0235.shtml
>> from the last time this was discussed.
>
[...]
> The problem I see here (with the issue I reported): Even if the path is
> canonicalized, the function crashes. So from my point of view, I did
> everything the docs asked me to, and still Subversion considers this
> ("svn+ssh://") an invalid url because it lacks the host part.
[...]

This thread turned to the question of what the policy should be, and died out.
  However, the crash has still not been fixed as far as I can see. I can see
at least two crash bugs:

1.
In subversion/libsvn_ra_svn/client.c, ra_svn_open() says:

> SVN_ERR(parse_url(url, &uri, sess_pool));

parse_url() can set uri.hostinfo to null (e.g. on url="svn+ssh://").

> SVN_ERR(find_tunnel_agent(tunnel, uri.hostinfo, &tunnel_argv, config,
> pool));

find_tunnel_agent() is not documented as accepting null, and crashes. In fact
it is not documented at all; that's doubtless part of the reason the bug came
to exist. Very naughty :-)

2.
svn_path_join(base, component, pool) says:

> * Note that the contents of @a base are not examined, so it is possible to
> * use this function for constructing URLs, or for relative URLs or
> * repository paths.

However, the implementation doesn't handle URLs very well. It starts off with:

> assert(is_canonical(base, blen));

but is_canonical() fails on some things that svn_path_canonicalize() returns,
such as "svn+ssh://". That's a definite bug in that pair of ...canonical...
functions.

With that fixed, svn_path_join() wouldn't crash, but I'm not convinced its
joining of "scheme://[host][/]" and "[/]path" would be well defined with
respect to how many slashes it puts in the result. But that's a separate issue.

These two are definite bugs and seem to have straightforward fixes. Anyone
care to write the patches?

Tests would also be very welcome. They probably need to call the APIs directly
as AFAIK the command-line client doesn't pass such arguments into these APIs.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jun 25 03:41:09 2006

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