On Sat, 15 Jan 2005 kfogel@collab.net wrote:
> "Peter N. Lundblad" <peter@famlundblad.se> writes:
> > I think this is a classical example of "fixing symptoms". Note that I
> > already did the work in svnserve to canonicalize every (AFAICT) path
> > coming from the net. I actually don't see the problem with that approach.
> > OK, it might be more "academically correct" to check if paths are
> > canonical instead of just canonicalize them. In practice, if something
> > depends on a path being truly canonical, we still have possible crashes.
>
> Hey, this isn't "fixing symptoms".
>
> What we have here is a question of whether a protocol should be strict
> or tolerant. The JavaSVN client was sending non-canonical paths to
> the server. If our approach is to require a certain style of paths in
> that protocol, then rejecting non-conformant paths isn't "fixing
> symptoms", it's "enforcing protocol", a perfectly normal practice.
>
With "fixing symptoms" I was referring to the use of a function that
doesn't actually make sure that the paths are canonical. It fixes the
assertion failures (symptom), but doesn't fix the real problem (making
sure that paths are in canonical form). When you use a function, you
should make sure it does what you want. Since this function is internal,
and undocumented, this means reading the code:-)
And, ofcourse, I think *this commit* was "fixing symptoms". The approach
in itself is not. It is valid, but more work, IMO for no real benefit.
> > Instead of duplicating svn_path_canonicalize in is_canonical, we could
> > just be a little more permissive to clients.
> >
> > Also, I didn't see any objection to the svnserve changes when I made them.
> > It was even backported to 1.1.x.
>
> Thanks for pointing it out -- I think Mike just didn't remember it
> (neither did I, when he and I discussed the mod_dav_svn change).
>
That's understandable:-) I just didn't see any discussion on the list
about this.
> Technically, the two protocols do not have to behave the same way.
> One could be strict, the other permissive. But consistency would be
> nice.
>
Aggreed.
BTW, when I said we might still have other bugs in this fix, I menat that
something in svn_path might depend on other properties of a canonical path
than is_canonical enforces. These might be crashes as well. I don't
know, because I haven't checked.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jan 15 19:16:20 2005