[PATCH] streamclone: partially encode filename over the wire, not for local read

Matt Mackall mpm at selenic.com
Wed Sep 30 16:38:38 CDT 2009


On Wed, 2009-09-30 at 17:01 -0400, Greg Ward wrote:
> On Wed, Sep 30, 2009 at 4:22 PM, Matt Mackall <mpm at selenic.com> wrote:
> > On Wed, 2009-09-30 at 16:08 -0400, Greg Ward wrote:
> >> # HG changeset patch
> >> # User Greg Ward <greg-hg at gerg.ca>
> >> # Date 1254341291 14400
> >> # Node ID 0411fe6a60722535cf44df4ea89b42c82ef96821
> >> # Parent  b2d65ee49a72f374fd6831f229e03ca723c29c77
> >> streamclone: partially encode filename over the wire, not for local read.
> >>
> >> (Fixes issue1847, which was introduced by 810387f59696: stream clone
> >> of a repo with directory named *.d failed: server raises exception
> >> "IOError: [Errno 2] No such file or directory:
> >> /tmp/test/.hg/store/data/foo.d.hg.hg/foo".)
> >
> > Not entirely clear to me what's going on in your fix. What happens in
> > this matrix?
> 
> Quoting my patch:
> 
> -                # for backwards compat, name was partially encoded
> -                entries.append((store.encodedir(name), size))
> 
> Dir-encoding the name at this point has two side-effects:
> 
> 1) send dir-encoded name over the wire (presumably correct and intended)

In line with historical accident, at least. We should instead have been
passing raw filenames with no encoding at all.

> 2) pass dir-encoded name to repo.sopener() (bad: that's where the
> double escaping comes from, i.e. "foo.d" escaped once to "foo.d.hg"
> [for wire protocol] and again to "foo.d.hg.hg" by repo.sopener())
> 
> So my patch delays the encodedir() a bit:
> 
> -        yield '%s\0%d\n' % (name, size)
> +        # partially encode name over the wire for backwards compat
> +        yield '%s\0%d\n' % (store.encodedir(name), size)
> 
> I am *assuming* that dir encoding the name over the wire is the
> correct thing to do, because that's what it was doing before.  I
> believe that the bug is not in the wire protocol, but in passing an
> already-direncoded name to repo.sopener().

Yes, I think you've nailed it.

> >            old client  new client
> > old server   breaks          ?
> > new server      ?         works
> >
> > If both '?' aren't 'works', then is there anything we can do to fix
> > that?
> 
> Good question!  Old server (that is, hg 1.3 or 1.3.1) will always
> break, because the bug is in the server code.  Stream cloning a repo
> with directories named like *.d does not work with 1.3 on the server,
> regardless of the client version.
> 
> I just tested (new server, old client) for various values of "old"
> (1.2.1, 1.3.1, current crew).  All work, where "work" means "can
> stream clone repo with *.d directory".  So the matrix is
> 
>              old client   new client
> old server   breaks       breaks
> new server   works        works

Ok, queued.

-- 
http://selenic.com : development and support for Mercurial and Linux




More information about the Mercurial-devel mailing list