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

Re: Bug: IP address bogusly interpreted as peg revision

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 28 May 2008 18:56:39 +0100

Julian Foad wrote:
> Karl Fogel wrote:
>
>> Julian Foad <julianfoad_at_btopenworld.com> writes:
>>
>>> It sounds to me like what's broken is the idea that we can validly
>>> stick the
>>> "@PEG" back on to a path or URL again after parsing and
>>> canonicalising it.
>>>
>>> Do you think we can get rid of all the code that relies on that, and
>>> deprecate the functions that do it and any functions that currently
>>> take "<canonical path or URL>@PEG" as input? Once we've parsed the
>>> syntax, keep the results separate. That sounds like the Right
>>> Thing. (The alternative, if we insist on glueing the two parts back
>>> together, is to define how we're going to do it in a way that doesn't
>>> lose information.)
>>
>> Public APIs would still take
>>
>> <canonical path or URL>@PEG
>>
>> right?
>
> No. I was suggesting deprecating all functions that currently do so.
> Haven't looked to see how feasible that would be, though. It might mean
> most of libsvn_client.

I took a look. This is completely localised within subversion/svn/; no public
library functions are affected. That makes it rather easy, I think.

Basically I'm talking about changing svn_client__args_to_target_array() to
return an array of (const char *path, svn_opt_revision_t peg) pairs.

Troy, I recommended against making this change while you were working on the
relative-URL patch, but now, as a separate improvement, it looks like it would
be great... Would you care to have a go at this?

- Julian

> Or, to develop the idea a bit further, the more precise problem is
> thinking that the "@PEG" is optional in such APIs: you can't pass an
> arbitrary <canonical path or URL> without a peg to such an API and
> guarantee that it will be interpreted as not having a peg. Therefore
> another solution is to deprecate those APIs that take
>
> <canonical path or URL>[@PEG]
>
> and replace them with ones that take
>
> <canonical path or URL>@[PEG]
>
> where the peg specifier field (starting with '@') has to be there even
> if there is no peg as such.
>
> But if we're considering doing that, I think we'd want to take the
> opportunity to separate the (optional) peg into a separate argument,
> because that's cleaner, given that it will already have been parsed once
> (before canonicalisation) anyway.
>
> (BTW, when I say "canonicalisation" in this thread, I really mean
> everything we do to arguments, which includes character-coding
> conversions and auto-escaping as well.)
>
>> Then we have the problem of one public API calling another one.
>
> Er... I don't follow. I guess you mean how would a deprecated function
> taking, say, "THING[@PEG]", convert its argument to a form suitable for
> passing to a different API taking, say, "THING@[PEG]". In the cases
> where such a conversion is defined, I don't know why it would be a
> problem. In the cases where it's ambiguous, like the one that started
> this thread ("http://name@10.0.0.1") then we won't be any worse off than
> we are now.
>
>> Or are you proposing that users of our APIs would call some public
>> function to separate THING[@PEG] into a { THING, [PEG] } structure,
>> and then all the other public APIs would take that structure?
>
> Yes, something more like that. This peg-separating operation needs to be
> done before or at the same time as "canonicalising" the raw arguments,
> so probably during the *_args_to_target_array*() functions.
>
> - Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-28 19:56:54 CEST

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