Unable to branch from http server after ftp push

[Joris P]

Steps:
1) push over ftp to simple webserver
    bzr push ftp://username:password@server/project
2) branch over http
    bzr branch http://server/username/project my_project

The error:
bzr: ERROR: http://server/username/project/.bzr/checkout/format is redirected to http://I_don't_know_the_url

Extra details:
The format file is found in three directories in my working copy, which I pushed: branch, repository and checkout.
On the webserver I only find the branch and repository. It's normal that I don't need the checkout because that information is redundant, but the branch apparently can't live without it.

[Tim Hatch (timhatch)]

I can't reproduce this on my server w/ bzr 0.18. Was the "I don't know the url" something like a 404 page? What to you get if you request the ...checkout/format in a browser?

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Joris P wrote:

> The error:
> bzr: ERROR: http://server/username/project/.bzr/checkout/format is redirected to http://I_don't_know_the_url
>
> Extra details:
> The format file is found in three directories in my working copy, which I pushed: branch, repository and checkout.
> On the webserver I only find the branch and repository. It's normal that I don't need the checkout because that information is redundant, but the branch apparently can't live without it.

The checkout directory is not required, but if it's not present, your
web server should issue a 404 not found. Instead, it's issuing a redirect.

Is the redirect literally to "http://I_don't_know_the_url"? I can't
find that string anywhere in our source code, so it appears to be coming
from the server.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGrkzs0F+nu1YWqI0RAsjrAJ9fVTKRJon0bml/v72Abp8TkinkFACdHVjQ
NhG1hHhOyAsnhYsohDNtfiA=
=WR1i
-----END PGP SIGNATURE-----

[Joris P (joris-putcuyps)]

> I can't reproduce this on my server w/ bzr 0.18
I'm using gentoo with bzr 0.17.

> Was the "I don't know the url" something like a 404 page?
Try http://users.skynet.be/dummy/ as an example.

> What to you get if you request the ...checkout/format in a browser?
The same happens if I request this url "...checkout/format" in my browser.

> Is the redirect literally to "http://I_don't_know_the_url"?
No this is just a fictional url, I think only the fact of the redirection was important.

[James Westby (james-w)]

Hi,

A user just confirmed this was still present on IRC.

Thanks,

James

[Arvid Norlander (vorpalblade)]

The general problem of redirection affects other transports too for dumb servers such as sftp. I had this problem in the past. Very irritating.

[ChriS (christophe-troestler)]

Maybe a simple solution is to follow the redirection. For the example given (and similarly in my case), "w3m -dump_extra http://users.skynet.be/dummy/" reports that the page is redirected to http://hostingerrors.isp.belgacom.be/Errors.cgi?url=http://users.skynet.be/dummy/ but then that page has the proper head :

$ w3m -dump_head http://users.skynet.be/dummy/
HTTP/1.1 404 Not Found
Date: Sat, 29 Mar 2008 16:03:43 GMT
Server: Apache/1.3.33 (Debian GNU/Linux) mod_ssl/2.8.22 OpenSSL/0.9.7e
Connection: close
Content-Type: text/html

[ChriS (christophe-troestler)]

As a simple workaround (tested with 1.5dev), one can change line 282 of bzrlib/transport/http/_urllib2_wrappers.py to

  self.follow_redirections = True

(this also fixes bug #226303).

[Vincent Ladeuil (vila)]

As said in https://bugs.edge.launchpad.net/bzr/+bug/226303/comments/3, this is a bit more complicated.

Your work-around makes 7 tests fail (try it yourself with 'bzr selftest').

Moreover, you didn't mention for which command(s) in which contexts your workaround is useful.

You did find the right line. This was indeed left in place so that redirection can be used if needed *under control*, changing it blindly is not enough as our test suite demonstrates.

[ChriS (christophe-troestler)]

Upon more investigation, it turns out that the problem is as follows:

"bzr branch http://users.skynet.be/Pierre.Brukier/anum/projet" requests (POST)
    "/Pierre.Brukier/anum/projet/.bzr/smart" from "users.skynet.be"
which redirects to "http://hostingerrors.isp.belgacom.be/Errors.cgi?url=http://users.skynet.be/Pierre.Brukier/anum/projet/.bzr/smart"
The redirection is followed: "/Errors.cgi/.bzr/smart" is requested (POST) from "hostingerrors.isp.belgacom.be", which fails with 404 (this is ok).
Now, the problem is that "/Errors.cgi/.bzr/branch-format" is requested (GET) from "hostingerrors.isp.belgacom.be" instead of the original host! This is not fine as the redirect was meant only for ".bzr/smart" and not for all subsequent URLs.

I am not sure why this decision was taken but, at least in the case the redirection eventually returns a 404 error, the original transport should not be changed...

[ChriS (christophe-troestler)]

To make the handling of redirections separate for each format, I moved it from
BzrDir.open_from_transport to BzrDir.BzrDirFormat.find_transport_and_format (a new method not to perturb the find_format which is redefined in terms of the former). I also needed to modify workingtree.WorkingTreeFormat.find_format to include redirection handling. Possibly redirection handling should be more pervasive in "find_format" methods but I'll let you judge. With the attached path,

bzr.skynet branch http://users.skynet.be/Pierre.Brukier/anum/projet

(this bug) as well as

bzr merge ttp://users.skynet.be/Pierre.Brukier/anum/projet

(bug #226303) work properly.

[ChriS (christophe-troestler)]

Of course I meant "bzr branch http://users.skynet.be/Pierre.Brukier/anum/projet".

[Vincent Ladeuil (vila)]

Thanks for working on this, this approach if far better and far more mergeable than your previous one :)

The redirection handling in itself was designed as is because the idea is that the redirections must be followed only for the first query ever made for a branch. Either (as you found out) for a POST(.bzr/smart) or (as you can observe by prepending nosmart+ to your url) for GET(.bzr/branch-format) or GET(./bzr/checkout/format).

I think your diagnosis is sound about updating the transport only if no errors finally occurs. On the other hand, errors may also occur later so your approach may have to be deployed in other places. But I think your patch may not need to address these concerns now.

Your patch needs to be reviewed before inclusion though, so please send it to <email address hidden> (you may need to subscribe).

Before doing so though, a couple of remarks:

- you changed the bzr behavior, so you need tests to avoid future regressions,
- you fixed a bug, so we like at least one test failing before your modifications and succeeding after your modifications,
- you added a public method (find_transport_and_format), there is an API stability policy regarding public methods, it may be more appropriate to use a private method here (otherwise this method needs more tests),
- be aware that some plugins use or redefine this method (foreign VCS plugins as bzr-svn, bzr-hg, bzr-git, etc) so it will be nicer for them if you could find a way to stay compatible,
- it seems a bit strange that you need to modify workingtree at all, but other reviewers may know better than me.

Regarding the tests, have a look in bzrlib/tests at http_server.py and test_http.py on how to implement an http server with unusual behavior (like users.skynet.be).

[ChriS (christophe-troestler)]

Attached is a new bundle that differs a bit from the previous one in -- I believe -- a better code factorization.

> - you changed the bzr behavior, so you need tests to avoid future regressions,
> - you fixed a bug, so we like at least one test failing before your modifications and succeeding after your modifications,

I have started to do so in the attached bundle but I could really use some help for these.

> - be aware that some plugins use or redefine this method (foreign
> VCS plugins as bzr-svn, bzr-hg, bzr-git, etc) so it will be nicer
> for them if you could find a way to stay compatible,

If you are takling about find_format, I did not find any occurrence of
it in bzr-svn.

> - it seems a bit strange that you need to modify workingtree at all,
> but other reviewers may know better than me.

This is because .bzr/checkout/format is read (which may be redirected too).

BTW,

 bzr co http://users.skynet.be/Pierre.Brukier/anum/projet

also works with the patch.

[ChriS (christophe-troestler)]

Of course it was late and I forgot to commit... Here is the patch.

[ChriS (christophe-troestler)]

Patch submitted to the mailing list.

[ChriS (christophe-troestler)]

Patch available.

[John A Meinel (jameinel)]

I was going to try to just propose the patch, but it has long since bit-rotted, and doesn't apply cleanly. So I'm marking this back as patch-needsfixing

[John A Meinel (jameinel)]

Also note, Jelmer has mentioned that we should go ahead and probe for GET .bzr/branch-format before we use POST .bzr/smart.

We wanted to make getting to the smart server as fast as possible, but it turns out that it tends to interact poorly with foreign branch integration, etc. And I think changing the search order to GET + POST would also fix bugs like this. Servers tend to be configured very differently for POST than GET, so failures on POST are often not real failures.