[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-08-14 23:27:09 CEST

Ping!

Julian Foad wrote:
> 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 Mon Aug 14 23:27:14 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.