[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: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2006-05-23 20:32:55 CEST

Malcolm Rowe wrote:
> On Tue, May 23, 2006 at 07:09:28PM +0200, Stefan Küng wrote:
>> then why don't you just do that for every public function yourself? I
>> mean I think it's better done in the library than force every client to
>> do it.
> See e.g. this thread: http://svn.haxx.se/dev/archive-2005-12/0235.shtml
> from the last time this was discussed.

Ok, the arguments in that thread were:
1) a lot of work, because it would have to be done in 'maaany' functions
2) some functions don't return an svn_error_t because they can't fail
with proper input

to 1): I agree it would be a lot of work. But as are other issues. And
they're still valid issues.
to 2): As long as 'proper input' is clearly defined and documented, I
don't mind if a function just returns garbage (garbage in, garbage out),
but even then it shouldn't crash.

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.
So, how could I have known what Subversion considers a valid url? That's
why I think Subversion should return an error for *every* input it
doesn't consider valid (if it can check it without crashing, e.g. when
an invalid pointer is passed). Only Subversion itself knows what's valid
here, so it should be responsible for doing the checking.
That also helps if sometime the underlying code is changed so that maybe
more urls are valid and can be handled - then only the library has to be
changed, not every client out there using it too.

I've stepped through the Subversion code when I first discovered this
issue. There's already a function which tokenizes the passed url, and
parts that are missing produce a NULL token (that's also why it crashes
- it accesses that NULL pointer later). A simple check for this would
suffice. A client would have to do the tokenizing first too and do the
checking, which then effectively means that tokenizing of the url is
done twice.


   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.tigris.org
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 23 20:35:18 2006

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