You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexander Sinyushkin <Al...@svnkit.com> on 2006/11/23 09:07:38 UTC
reparent bug in svnserve
Hello, guys, seems that there's a bug in svnserve's 'reparent'
behaviour. The point is that svnserve sends 'success' on a try to
reparent to a different repository although documentation says the
opposite. The problem appears because the 'get_fs_path' function in
'serve.c' checks only the first n bytes of two urls, where n - is
the length of the old repos_root, but a new url may contain all the
previous bytes as the old one does, however at the same time it may
be not the same repository. For example:
old_repos_url = "svn://localhost/svn-test-work/repositories/externals_tests-1"
new_url = "svn://localhost/svn-test-work/repositories/externals_tests-1.other"
This is what I noticed while running 'externals' python tests.
P.S.: sorry if this letter has come to you twice. I wasn't sure if the
first address was right.
--
Alexander Sinyushkin,
TMate Software,
http://svnkit.com/ - Java [Sub]Versioning Library!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: reparent bug in svnserve
Posted by Peter Lundblad <pl...@google.com>.
David Glasser writes:
> On 11/26/06, David Glasser <gl...@mit.edu> wrote:
> > On 11/23/06, Alexander Sinyushkin <Al...@svnkit.com> wrote:
> > > This is what I noticed while running 'externals' python tests.
> >
What do you mean? Can you trigger this using the python bindings, or are you
speaking the svn protocol directly?
> Actually, hmm, I think I see what the real issue is. Turns out that
> unlike most of the stubs in libsvn_ra/ra_loader.c, svn_ra_reparent
> actually does some common work --- it calls svn_ra_get_repos_root and
> does the appropriate check with svn_path_is_ancestor. So what you
> describe should not be a problem for anything that actually uses the C
> client implementation of svn_ra_reparent.
Or doesn't abuse the (underdocumented) svn protocol.
Note also that some other commands (diff, switch and link-path in the reporter)
also suffer from this bug.
>
> I'd recommend that SVNKit make the same client-side check as the C
> libraries do, but we should still probably fix this (using
> svn_path_is_ancestor).
+1.
(Plus, there could hypothetically be a race
> condition or something with the client-side check.)
>
I don't think we support relocating the repository while it is online, if
that's what you're referring to...
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re[2]: reparent bug in svnserve
Posted by David Glasser <gl...@mit.edu>.
On 11/26/06, Alexander Sinyushkin <Al...@svnkit.com> wrote:
> Hello, David.
>
> > I'd recommend that SVNKit make the same client-side check as the C
> > libraries do, but we should still probably fix this (using
>
> Yes, we added this kind of check. It seems to work fine, but I also
> thought it might appear useful for you to know of such a problem.
Absolutely. I can't fix it this minute since I'm in a rush, but I
opened issue 2665.
--dave
--
David Glasser | glasser@mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: reparent bug in svnserve
Posted by David Glasser <gl...@mit.edu>.
On 11/26/06, David Glasser <gl...@mit.edu> wrote:
> On 11/23/06, Alexander Sinyushkin <Al...@svnkit.com> wrote:
> > Hello, guys, seems that there's a bug in svnserve's 'reparent'
> > behaviour. The point is that svnserve sends 'success' on a try to
> > reparent to a different repository although documentation says the
> > opposite. The problem appears because the 'get_fs_path' function in
> > 'serve.c' checks only the first n bytes of two urls, where n - is
> > the length of the old repos_root, but a new url may contain all the
> > previous bytes as the old one does, however at the same time it may
> > be not the same repository. For example:
> > old_repos_url = "svn://localhost/svn-test-work/repositories/externals_tests-1"
> > new_url = "svn://localhost/svn-test-work/repositories/externals_tests-1.other"
> >
> > This is what I noticed while running 'externals' python tests.
>
> Good point, Alexander. What do you think the fix is -- checking to
> see if the next character after the matching bit is '\0' or '/'? (Are
> we guaranteed that it uses forward slashes in this part of the code?)
>
> On the other hand, the situation in the other ra modules seems to be
> even worse: none of the other three do any checks, and it looks like
> the ra_local one could even manage to pass around pointers to
> unallocated memory.
Actually, hmm, I think I see what the real issue is. Turns out that
unlike most of the stubs in libsvn_ra/ra_loader.c, svn_ra_reparent
actually does some common work --- it calls svn_ra_get_repos_root and
does the appropriate check with svn_path_is_ancestor. So what you
describe should not be a problem for anything that actually uses the C
client implementation of svn_ra_reparent. On the other hand, if there
in theory (wink wink) existed, say, a Java reimplementation that tried
to speak the same protocol without making that client-side check
first, then sure, I see that there could be trouble.
I'd recommend that SVNKit make the same client-side check as the C
libraries do, but we should still probably fix this (using
svn_path_is_ancestor). (Plus, there could hypothetically be a race
condition or something with the client-side check.)
--dave
--
David Glasser | glasser@mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: reparent bug in svnserve
Posted by David Glasser <gl...@mit.edu>.
On 11/23/06, Alexander Sinyushkin <Al...@svnkit.com> wrote:
> Hello, guys, seems that there's a bug in svnserve's 'reparent'
> behaviour. The point is that svnserve sends 'success' on a try to
> reparent to a different repository although documentation says the
> opposite. The problem appears because the 'get_fs_path' function in
> 'serve.c' checks only the first n bytes of two urls, where n - is
> the length of the old repos_root, but a new url may contain all the
> previous bytes as the old one does, however at the same time it may
> be not the same repository. For example:
> old_repos_url = "svn://localhost/svn-test-work/repositories/externals_tests-1"
> new_url = "svn://localhost/svn-test-work/repositories/externals_tests-1.other"
>
> This is what I noticed while running 'externals' python tests.
Good point, Alexander. What do you think the fix is -- checking to
see if the next character after the matching bit is '\0' or '/'? (Are
we guaranteed that it uses forward slashes in this part of the code?)
On the other hand, the situation in the other ra modules seems to be
even worse: none of the other three do any checks, and it looks like
the ra_local one could even manage to pass around pointers to
unallocated memory.
--dave
--
David Glasser | glasser@mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org