You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2004/07/04 15:13:01 UTC
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
On Sun, 2004-07-04 at 09:01, jpieper@tigris.org wrote:
> Issue #1779: Improve svn_path_canonicalize to make more
> transformations aimed at removing path specification redundancies.
I am concerned about the case where one runs something like "svn st
foo/../bar", where foo is a symlink with multiple path elements.
(Symlinks within working dirs around allowed, but I see no reason why
symlinks should be disallowed in the portion of a path leading up to the
working copy.)
You did not break this case; it was already broken due to the call to
apr_filepath_merge() in libsvn_subr/opt.c:svn_opt_args_to_target_array.
But it seems like the logic of both apr_filepath_merge() and the new
logic of svn_path_canonicalize() are fundamentally flawed, unless they
are only applied within a playground where directory symlinks are
disallowed.
> (svn_path_canonicalize): Replaced old implementation with new that
> uses nearly identical logic to large portions of
> apr_filepath_merge. It condenses redundant "/./", "/../", and
> "//" portions of the path, as well as removing any trailing
> separator characters.
When you say in a log message that a function is "nearly identical" to a
preexisting function, you might say how it's different. As it stands,
I'm not sure why you didn't use apr_filepath_merge to implement
svn_path_canonicalize.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [OT] file:// vs. file:///
Posted by Klaus Rennecke <kr...@tigris.org>.
Branko Čibej wrote:
> Klaus Rennecke wrote:
>
>> [...] See Section 3.10 in http://www.faqs.org/rfcs/rfc1738.html where
>> the file:/// URL is the special case with host name omitted because
>> it is localhost.
>>
>> So it's file:/// and *not* file://.
>
>
> That paragraph says:
>
> As a special case, <host> can be the string "localhost" or the empty
> string; this is interpreted as `the machine from which the URL is
> being interpreted'.
>
> Note, "can be", not "must be". So to be consistent, it's "http:// and
> file://", not "http:// and file:///". because the second form implies
> a certain value for the scheme-specific data, and the first doesn't.
True. I give, you win :-)
It just happened to me that in some completely unrelated case people
were appending the absolute file name to file:// so that it resulted in
OK URLs on unix, but invalid (host name "C:") on weirder OSes.
> In fact, when we talk just about the RA-specific URL scheme, we should
> be using only "http:", "svn:" and "file:" -- the double slashes aren't
> part of the scheme, according to that RFC. [...]
Yes that sounds like a good suggestion. It is even easier on the eyes
because the email client does not highlight it as a link.
/Klaus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [OT] file:// vs. file:///
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> In fact, when we talk just about the RA-specific URL scheme, we should
> be using only "http:", "svn:" and "file:" -- the double slashes aren't
> part of the scheme, according to that RFC.
Maybe we should, but since it's *vastly* more recognizable to humans
with the slashes, I plan to continue using them anyway. Hope y'all
will too :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[OT] file:// vs. file:///
Posted by Branko Čibej <br...@xbc.nu>.
Klaus Rennecke wrote:
> Branko Čibej wrote:
>
>> [...]
>> And please, for the last time, it's file://, not file:/// -- two
>> slashes, not three, if you want to be consistent with the others.
>> [...]
>
>
>
> I beg to differ. See Section 3.10 in
> http://www.faqs.org/rfcs/rfc1738.html where the file:/// URL is the
> special case with host name omitted because it is localhost.
>
> So it's file:/// and *not* file://.
That paragraph says:
As a special case, <host> can be the string "localhost" or the empty
string; this is interpreted as `the machine from which the URL is
being interpreted'.
Note, "can be", not "must be". So to be consistent, it's "http:// and
file://", not "http:// and file:///". because the second form implies a
certain value for the scheme-specific data, and the first doesn't.
In fact, when we talk just about the RA-specific URL scheme, we should
be using only "http:", "svn:" and "file:" -- the double slashes aren't
part of the scheme, according to that RFC.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Klaus Rennecke <kr...@tigris.org>.
Branko Čibej wrote:
> [...]
> And please, for the last time, it's file://, not file:/// -- two
> slashes, not three, if you want to be consistent with the others.
> [...]
I beg to differ. See Section 3.10 in
http://www.faqs.org/rfcs/rfc1738.html where the file:/// URL is the
special case with host name omitted because it is localhost.
So it's file:/// and *not* file://.
For a more loose behavior that's end user oriented see
http://www.mozilla.org/quality/networking/testing/filetests.html
/Klaus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Josh Pieper wrote:
>Branko ??ibej wrote:
>
>
>>Josh Pieper wrote:
>>
>>
>>
>>>I don't understand. Are you saying that some portions of file:// URLs
>>>are not given directly to the filesystem to resolve? Or that we can
>>>safely collapse "/../" paths from them? Or something else?
>>>
>>>
>>>
>>>
>>Both, but the second is more important. I think we should consider
>>removing the '/../' portions from file:// URLs. Whilst we can't do that
>>safely in paths -- because of the order in which symlinks get resolved,
>>as I now understand -- I think we can safely forbid this kind of symlink
>>magic for canonical repository URLs and simply declare that these don't
>>contain /../ and /./ sequences. That would let us handle all URLs the same.
>>
>>
>
>Brane,
>
>ghudson, breser and myself talked a little on IRC and think that just
>generating a sane error if any URLs contain "/../", and not collapsing
>".." in paths would be an acceptable solution for now. What do you
>think?
>
>
I'm happy with that. The result is that we don't get any "non-canonical"
URLs into the libraries, which is basically what I was aiming for. Thanks.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Branko ??ibej wrote:
> Josh Pieper wrote:
>
> >I don't understand. Are you saying that some portions of file:// URLs
> >are not given directly to the filesystem to resolve? Or that we can
> >safely collapse "/../" paths from them? Or something else?
> >
> >
> Both, but the second is more important. I think we should consider
> removing the '/../' portions from file:// URLs. Whilst we can't do that
> safely in paths -- because of the order in which symlinks get resolved,
> as I now understand -- I think we can safely forbid this kind of symlink
> magic for canonical repository URLs and simply declare that these don't
> contain /../ and /./ sequences. That would let us handle all URLs the same.
Brane,
ghudson, breser and myself talked a little on IRC and think that just
generating a sane error if any URLs contain "/../", and not collapsing
".." in paths would be an acceptable solution for now. What do you
think?
-Josh
--
Decaffeinated coffee? Just Say No.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Josh Pieper wrote:
>Branko ??ibej wrote:
>
>
>>Josh Pieper wrote:
>>
>>
>>>file:/// URLs would have to be special cased then to leave the ".."
>>>path elements in since a portion is directly translated to a
>>>filesystem path.
>>>
>>No they do not. They're still URLs, not paths.
>>
>>
>I don't understand. Are you saying that some portions of file:// URLs
>are not given directly to the filesystem to resolve? Or that we can
>safely collapse "/../" paths from them? Or something else?
>
>
Both, but the second is more important. I think we should consider
removing the '/../' portions from file:// URLs. Whilst we can't do that
safely in paths -- because of the order in which symlinks get resolved,
as I now understand -- I think we can safely forbid this kind of symlink
magic for canonical repository URLs and simply declare that these don't
contain /../ and /./ sequences. That would let us handle all URLs the same.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Branko ??ibej wrote:
> Josh Pieper wrote:
>
> >Ben Reser wrote:
> >
> >
> >>On Sun, Jul 04, 2004 at 11:57:24AM -0400, Josh Pieper wrote:
> >>
> >>
> >>>Ok, in IRC ghudson proposed:
> >>>
> >>><ghudson> Well, I'd agree with your initial judgement that we
> >>>shouldn't be collapsing .. segments, and then if we changed the
> >>>apr_filepath_merge() call in opt.c to an svn_path_canonicalize, we'd
> >>>fix the case I mentioned was broken.
> >>>
> >>>I don't believe we considered the symlink case when breser and I
> >>>initially talked on IRC about supporting collapsing "/../" elements.
> >>>Does anyone have objections to just removing the ".." collapsing
> >>>support from svn_path_canonicalize? That would also resolve the issue
> >>>of implementing svn_path_canonicalize in terms of apr_filepath_merge.
> >>>
> >>>
> >>No we sure didn't. But if we remove the colapsing of .. in paths we
> >>still need it in URLs. See the comment by Klaus Rennecke about the
> >>weirdness that occurs with URLS with /../ in them.
> >>
> >>
> >
> >file:/// URLs would have to be special cased then to leave the ".."
> >path elements in since a portion is directly translated to a
> >filesystem path.
> >
> >
> No they do not. They're still URLs, not paths.
I don't understand. Are you saying that some portions of file:// URLs
are not given directly to the filesystem to resolve? Or that we can
safely collapse "/../" paths from them? Or something else?
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Josh Pieper wrote:
>Ben Reser wrote:
>
>
>>On Sun, Jul 04, 2004 at 11:57:24AM -0400, Josh Pieper wrote:
>>
>>
>>>Ok, in IRC ghudson proposed:
>>>
>>><ghudson> Well, I'd agree with your initial judgement that we
>>>shouldn't be collapsing .. segments, and then if we changed the
>>>apr_filepath_merge() call in opt.c to an svn_path_canonicalize, we'd
>>>fix the case I mentioned was broken.
>>>
>>>I don't believe we considered the symlink case when breser and I
>>>initially talked on IRC about supporting collapsing "/../" elements.
>>>Does anyone have objections to just removing the ".." collapsing
>>>support from svn_path_canonicalize? That would also resolve the issue
>>>of implementing svn_path_canonicalize in terms of apr_filepath_merge.
>>>
>>>
>>No we sure didn't. But if we remove the colapsing of .. in paths we
>>still need it in URLs. See the comment by Klaus Rennecke about the
>>weirdness that occurs with URLS with /../ in them.
>>
>>
>
>file:/// URLs would have to be special cased then to leave the ".."
>path elements in since a portion is directly translated to a
>filesystem path.
>
>
No they do not. They're still URLs, not paths.
And please, for the last time, it's file://, not file:/// -- two
slashes, not three, if you want to be consistent with the others.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Ben Reser wrote:
> On Sun, Jul 04, 2004 at 01:26:01PM -0400, Josh Pieper wrote:
> > file:/// URLs would have to be special cased then to leave the ".."
> > path elements in since a portion is directly translated to a
> > filesystem path.
>
> True.
>
> > That seems like an unnecessary restriction, especially for file:///
> > URLs.
>
> I wasn't too fond of it. But obviously we have to either make /../
> behave the way people expect or not at all. I prefer making them work.
Well, I think we have to allow it for filesystem paths, which would be
any relative path and the root portion of the file:/// URL. Since we
don't know what portion of the file:/// URL is the root at
canonicalization time, I don't think it feasible to treat the root
differently from the rest of the path.
> > Are there other cases where this problem can arise besides a
> > repos->repos copy? If not, then maybe we could just not collapse the
> > ".." anywhere and give better diagnostics there.
>
> Not sure. I haven't done extensive testing looking for problems...
Therefore, I think this above suggestion is the best alternative. We
would treat all URLs and relative paths the same with regard to
"/../", namely that they would not be collapsed. Then our job is just
to make "svn copy" and "svn move" return sensible errors when those
elements are present in the wrong place.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 01:26:01PM -0400, Josh Pieper wrote:
> file:/// URLs would have to be special cased then to leave the ".."
> path elements in since a portion is directly translated to a
> filesystem path.
True.
> That seems like an unnecessary restriction, especially for file:///
> URLs.
I wasn't too fond of it. But obviously we have to either make /../
behave the way people expect or not at all. I prefer making them work.
> Are there other cases where this problem can arise besides a
> repos->repos copy? If not, then maybe we could just not collapse the
> ".." anywhere and give better diagnostics there.
Not sure. I haven't done extensive testing looking for problems...
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Ben Reser wrote:
> On Sun, Jul 04, 2004 at 11:57:24AM -0400, Josh Pieper wrote:
> > Ok, in IRC ghudson proposed:
> >
> > <ghudson> Well, I'd agree with your initial judgement that we
> > shouldn't be collapsing .. segments, and then if we changed the
> > apr_filepath_merge() call in opt.c to an svn_path_canonicalize, we'd
> > fix the case I mentioned was broken.
> >
> > I don't believe we considered the symlink case when breser and I
> > initially talked on IRC about supporting collapsing "/../" elements.
> > Does anyone have objections to just removing the ".." collapsing
> > support from svn_path_canonicalize? That would also resolve the issue
> > of implementing svn_path_canonicalize in terms of apr_filepath_merge.
>
> No we sure didn't. But if we remove the colapsing of .. in paths we
> still need it in URLs. See the comment by Klaus Rennecke about the
> weirdness that occurs with URLS with /../ in them.
file:/// URLs would have to be special cased then to leave the ".."
path elements in since a portion is directly translated to a
filesystem path.
> The other option is to simply reject URLs with /../ in them. But
> whatever we do we need to make sure people don't get unexpected behavior
> with them.
That seems like an unnecessary restriction, especially for file:///
URLs. Are there other cases where this problem can arise besides a
repos->repos copy? If not, then maybe we could just not collapse the
".." anywhere and give better diagnostics there.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 11:57:24AM -0400, Josh Pieper wrote:
> Ok, in IRC ghudson proposed:
>
> <ghudson> Well, I'd agree with your initial judgement that we
> shouldn't be collapsing .. segments, and then if we changed the
> apr_filepath_merge() call in opt.c to an svn_path_canonicalize, we'd
> fix the case I mentioned was broken.
>
> I don't believe we considered the symlink case when breser and I
> initially talked on IRC about supporting collapsing "/../" elements.
> Does anyone have objections to just removing the ".." collapsing
> support from svn_path_canonicalize? That would also resolve the issue
> of implementing svn_path_canonicalize in terms of apr_filepath_merge.
No we sure didn't. But if we remove the colapsing of .. in paths we
still need it in URLs. See the comment by Klaus Rennecke about the
weirdness that occurs with URLS with /../ in them.
The other option is to simply reject URLs with /../ in them. But
whatever we do we need to make sure people don't get unexpected behavior
with them.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Klaus Rennecke wrote:
> Josh Pieper wrote:
>
>> [...]
>>
>> Does anyone have objections to just removing the ".." collapsing
>> support from svn_path_canonicalize?
>> [...]
>>
>
> This is why I was asking for it in the first place (example from
> 2004-06-20):
>
> tukan:[kre]> svn --version
> svn, Version 1.1.0 (dev build)
> übersetzt Jun 20 2004, 18:52:50
> [...snip...]
> tukan:[kre]> svnadmin create /tmp/1901
> tukan:[kre]> svn mkdir -m "" file:///tmp/1901/branches
> Revision 1 übertragen.
> tukan:[kre]> svn mkdir -m "" file:///tmp/1901/branches/branch1
> Revision 2 übertragen.
> tukan:[kre]> svn copy -m "" file:///tmp/1901/branches/branch1
> file:///tmp/1901/branches/branch1/../branch2
> subversion/libsvn_repos/commit.c:114: (apr_err=160028)
> svn: Out of date: '/branches/branch1/..' in transaction '3'
But paths and URLs behave differently, don't they?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Klaus Rennecke <kr...@tigris.org>.
Josh Pieper wrote:
> [...]
>
>Does anyone have objections to just removing the ".." collapsing
>support from svn_path_canonicalize?
>[...]
>
This is why I was asking for it in the first place (example from
2004-06-20):
tukan:[kre]> svn --version
svn, Version 1.1.0 (dev build)
übersetzt Jun 20 2004, 18:52:50
[...snip...]
tukan:[kre]> svnadmin create /tmp/1901
tukan:[kre]> svn mkdir -m "" file:///tmp/1901/branches
Revision 1 übertragen.
tukan:[kre]> svn mkdir -m "" file:///tmp/1901/branches/branch1
Revision 2 übertragen.
tukan:[kre]> svn copy -m "" file:///tmp/1901/branches/branch1
file:///tmp/1901/branches/branch1/../branch2
subversion/libsvn_repos/commit.c:114: (apr_err=160028)
svn: Out of date: '/branches/branch1/..' in transaction '3'
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-07-04 at 12:13, Branko Čibej wrote:
> $ mkdir -p A/B/C/D
> $ mkdir D
> $ ln -s A/B/C/D d
> $ cd d/../D
Not only is this specific to the shell cd command, it's also
shell-specific. tcsh puts you in A/B/C/D, as does Solaris /bin/sh, but
bash and Solaris /usr/xpg4/bin/sh put you in D.
The shell cd command is a little weird, I think mainly because it's
trying to paper over automounter symlinks. It's not a particularly good
precedent to follow. It's also a bit more complicated than what we're
currently doing in apr_filepath_merge and the new
svn_path_canonicalize. Observe:
$ cd /tmp/d
$ pwd
/tmp/d
$ /bin/pwd
/tmp/A/B/C/D
$ cd ..
$ pwd
/tmp
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Josh Pieper wrote:
>Branko ??ibej wrote:
>
>
>>Josh Pieper wrote:
>>
>>
>>
>>>I don't believe we considered the symlink case when breser and I
>>>initially talked on IRC about supporting collapsing "/../" elements.
>>>Does anyone have objections to just removing the ".." collapsing
>>>support from svn_path_canonicalize? That would also resolve the issue
>>>of implementing svn_path_canonicalize in terms of apr_filepath_merge.
>>>
>>>
>>>
>>>
>>The Unix filesystem interprets /../ sequences in paths _before_
>>expanding symbolic links. Try this:
>>
>> $ mkdir -p A/B/C/D
>> $ mkdir D
>> $ ln -s A/B/C/D d
>> $ cd d/../D
>>
>>Guess where you'll end up?
>>
>>So, this whole discussion seems pointless.
>>
>>
>
>
>Actually my linux system lets you resolve files in either parent.
>There are certainly cases when it works.
>
>Try this recipe:
>
>mkdir -p A/B/C/D
>echo "foo" > A/B/C/foo
>ln -s A/B/C/D d1
>cat d/../foo
>
>
So. Looks like the shell and the filesystem behave differently. This is
really boring, because it means we'd have to canonicalise paths
differently on Unix and Windows, which again points at apr_filepath_merge.
Hm. Weird.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Branko Čibej <br...@xbc.nu>.
Josh Pieper wrote:
>I don't believe we considered the symlink case when breser and I
>initially talked on IRC about supporting collapsing "/../" elements.
>Does anyone have objections to just removing the ".." collapsing
>support from svn_path_canonicalize? That would also resolve the issue
>of implementing svn_path_canonicalize in terms of apr_filepath_merge.
>
>
The Unix filesystem interprets /../ sequences in paths _before_
expanding symbolic links. Try this:
$ mkdir -p A/B/C/D
$ mkdir D
$ ln -s A/B/C/D d
$ cd d/../D
Guess where you'll end up?
So, this whole discussion seems pointless.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Greg Hudson wrote:
> On Sun, 2004-07-04 at 11:21, Josh Pieper wrote:
> > Yes, I considered that case; this also stopped generating an error
> > when you try "invalid_path/../real_path." Apache does this when
> > resolving URLs to filesystem paths, so at least one other application
> > handles paths the same way. Of course that doesn't mean it is a valid
> > method, but collapsing the ".." makes the copy code more robust to
> > differing path specifications in the common cases.
>
> Apache can dictate that directory symlinks are disallowed in document
> roots. I don't think it's fair for us to dictate that about all paths
> on the command line, only about the area within working copies (and, of
> course, the virtual area within repositories).
>
> I followed the pre-commit discussion about this change, but the
> discussion of "differing path specifications" never seemed to become
> concrete. Where is it necessary to ensure that canon(p1) == canon(p2)
> if p1 and p2 are two different paths expressing the same object?
Ok, in IRC ghudson proposed:
<ghudson> Well, I'd agree with your initial judgement that we
shouldn't be collapsing .. segments, and then if we changed the
apr_filepath_merge() call in opt.c to an svn_path_canonicalize, we'd
fix the case I mentioned was broken.
I don't believe we considered the symlink case when breser and I
initially talked on IRC about supporting collapsing "/../" elements.
Does anyone have objections to just removing the ".." collapsing
support from svn_path_canonicalize? That would also resolve the issue
of implementing svn_path_canonicalize in terms of apr_filepath_merge.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-07-04 at 11:21, Josh Pieper wrote:
> Yes, I considered that case; this also stopped generating an error
> when you try "invalid_path/../real_path." Apache does this when
> resolving URLs to filesystem paths, so at least one other application
> handles paths the same way. Of course that doesn't mean it is a valid
> method, but collapsing the ".." makes the copy code more robust to
> differing path specifications in the common cases.
Apache can dictate that directory symlinks are disallowed in document
roots. I don't think it's fair for us to dictate that about all paths
on the command line, only about the area within working copies (and, of
course, the virtual area within repositories).
I followed the pre-commit discussion about this change, but the
discussion of "differing path specifications" never seemed to become
concrete. Where is it necessary to ensure that canon(p1) == canon(p2)
if p1 and p2 are two different paths expressing the same object?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr tests/clients/cmdline tests/libsvn_subr
Posted by Josh Pieper <jj...@pobox.com>.
Greg Hudson wrote:
> On Sun, 2004-07-04 at 09:01, jpieper@tigris.org wrote:
> > Issue #1779: Improve svn_path_canonicalize to make more
> > transformations aimed at removing path specification redundancies.
>
> I am concerned about the case where one runs something like "svn st
> foo/../bar", where foo is a symlink with multiple path elements.
> (Symlinks within working dirs around allowed, but I see no reason why
> symlinks should be disallowed in the portion of a path leading up to the
> working copy.)
>
> You did not break this case; it was already broken due to the call to
> apr_filepath_merge() in libsvn_subr/opt.c:svn_opt_args_to_target_array.
> But it seems like the logic of both apr_filepath_merge() and the new
> logic of svn_path_canonicalize() are fundamentally flawed, unless they
> are only applied within a playground where directory symlinks are
> disallowed.
Yes, I considered that case; this also stopped generating an error
when you try "invalid_path/../real_path." Apache does this when
resolving URLs to filesystem paths, so at least one other application
handles paths the same way. Of course that doesn't mean it is a valid
method, but collapsing the ".." makes the copy code more robust to
differing path specifications in the common cases.
> > (svn_path_canonicalize): Replaced old implementation with new that
> > uses nearly identical logic to large portions of
> > apr_filepath_merge. It condenses redundant "/./", "/../", and
> > "//" portions of the path, as well as removing any trailing
> > separator characters.
>
> When you say in a log message that a function is "nearly identical" to a
> preexisting function, you might say how it's different. As it stands,
> I'm not sure why you didn't use apr_filepath_merge to implement
> svn_path_canonicalize.
Yes, good point. I will document the reasons why in the log message.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10133 - in trunk/subversion: include libsvn_subr
tests/clients/cmdline tests/libsvn_subr
Posted by Klaus Rennecke <kr...@tigris.org>.
Greg Hudson wrote:
> [...]
>
>I am concerned about the case where one runs something like "svn st
>foo/../bar", where foo is a symlink with multiple path elements.
>[...]
>
You might get away with backpathing over symlinks if you URL-encode the
.. segments, like foo/%2E%2E/bar. If I recall correctly, the URL
decoding is done *after* the canonicalisation, at least in some cases
(checkout-cmd). Well, OK, this is probably a bug, but I don't see a way
to fix it safely with the current flow control.
That aside, it's very risky to try backpathing over symlinks. Several
levels may shortcut out there, the bash does it for instance in a very
elaborate way so that even "cd .." most often backtracks to the symlink
source.
/Klaus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org