Smart server jail breaks bzr+http with shared repos

Note: I just reverted the copy of bzr used by my bzr+http server to
r4193, so http://bzr.mattnordhoff.com/ no longer exhibits this bug.

I confirm that upgrading to bzr 1.14 makes it impossible to use an existing bzr+http server. So it's kind of a "big" bug.

Workaround : downgrading to 1.13

It seems like the chroot information should be based on the configuration, and not the first URL passed.
I don't have specific information about how it is working, but that might be an avenue to look at for debugging this.

chroot information is dynamic based on the url and the root of the server;

it may be the wsgi glue.

If I was guessing, I would say the chroot is being set based on the '.bzr/smart' you are posting to. So if your layout is:

repo/branch

Then your initial POST goes to 'http://..../repo/branch/.bzr/smart'

At that point, we can see that we don't have a repository (or maybe we see the repository is 1 level up from a request, etc).

Anyway, we've done things 2 different ways in the past. One is to post:

1) POST http://../repo/branch/.bzr/smart open_repository ..
2) POST http://../repo/.bzr/smart open_repository .

The main difference is whether the "Transport.clone('..')" was changing the base URL that we POST to, or whether it was just something changing the paths transmitted.

My guess is the currentcode is doing (1), and that the chroot is being based on the POST url, which means that the jail is causing us to fail. So at this point we could:

1) Change the client to go back to the old way, and have 'clone(..)' change the POST url. I think Andrew didn't like that form. I *think* because there was no guarantee that both URLs were valid to POST to, and we know we have something valid where we are at now.
2) Change the jail code to only jail at the level of config, rather than making it dynamic based on the POST location. I personally would be fine with this, considering the WSGI codes says "expose paths under /srv/bzr/repo" it seems fine to use that as the chroot jail for all requests.

John A Meinel wrote:
[...]
> 2) Change the jail code to only jail at the level of config, rather than
> making it dynamic based on the POST location. I personally would be fine with
> this, considering the WSGI codes says "expose paths under /srv/bzr/repo" it
> seems fine to use that as the chroot jail for all requests.

I think this option makes the most sense. It's consistent with how the TCP and
inet (SSH) servers work.

I just added an evil little monkeypatch to my bzr-smart script that changes the jail to your configured root (e.g. /srv/bzr/repo):

from bzrlib.smart import request

def setup_jail(self):
    # The backing transport is a chroot on /srv/bzr/repo
    transport = self._backing_transport.clone('/')
    request.jail_info.transports = [transport, self._backing_transport]

request.SmartServerRequest.setup_jail = setup_jail

Monkeypatching is wrong, and might break, of course, but at least it works right now.

I have a fix for this, finally. See the linked branch. Will add test(s) and submit for review, hopefully will be part of 2.1.0.

With this plus a small bug fix for loggerhead, loggerhead's serve-branches script Just Works for me for serving a shared repo over bzr+http.

For posterity, I'm attaching a better (mostly just longer) version of the monkeypatch.

It verifies that the backing transport really is a chroot, so it should be a bit safer.

Credit goes to spiv, BTW. (Not blame, though!)

@Matt, please do a merge proposal instead of adding a patch to this bug, it's far easier to track, thanks in advance !

Vincent Ladeuil wrote:
> @Matt, please do a merge proposal instead of adding a patch to this bug,
> it's far easier to track, thanks in advance !

This bug has long been fixed. The monkeypatch is just a workaround
applications can use if they need to run with an older (< 2.1) bzr.

@Matt, ha ok, thanks for the explanation :D