Rev 3879: Fix bug #245964 by preserving decorators during redirections (when in lp:~vila/bzr/303959-redirection
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Dec 3 16:40:42 GMT 2008
At lp:~vila/bzr/303959-redirection
------------------------------------------------------------
revno: 3879
revision-id: v.ladeuil+lp at free.fr-20081203164038-5ghthsgdei3ank1j
parent: pqm at pqm.ubuntu.com-20081202015700-3mc9dola31w7h5h4
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 303959-redirection
timestamp: Wed 2008-12-03 17:40:38 +0100
message:
Fix bug #245964 by preserving decorators during redirections (when
appropriate).
* bzrlib/transport/decorator.py:
(TransportDecorator._redirected_to): Generic implementation
delegating to the _decorated transport.
* bzrlib/transport/http/__init__.py:
(HttpTransportBase.clone): Drive-by fix, deleted, base
implementation was strictly identical.
(HttpTransportBase._redirected_to): Narrow the possible
redirections to those where both urls have the same ending
relpath (to cope with known usages that doesn't respect the 'base'
paradigm (i.e. using '.bzr/smart' instead of 'smart' on a cloned
transport for example). Preserve qualifiers when creating the
redirected transport. Also preserve user if used against the same
host:port (further authentication can invalidate the user if it's
wrong, but if it's required, we'd better propagate it).
* bzrlib/transport/decorator.py:
(TransportDecorator.__init__): Drive-by fix, the url is indeed not
decorated anymore, name the variable accordingly.
* bzrlib/transport/__init__.py:
(Transport._redirected_to): Default do nothing implementation.
* bzrlib/tests/test_bzrdir.py:
(TestHTTPRedirections.test_qualifier_preserved): Ensures that
qualifiers are preserved across redirections.
* bzrlib/bzrdir.py:
(BzrDir.open_from_transport.redirected): Delegate redirection
handling to the transport.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/bzrdir.py bzrdir.py-20060131065624-156dfea39c4387cb
bzrlib/tests/test_bzrdir.py test_bzrdir.py-20060131065654-deba40eef51cf220
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/__init__.py transport.py-20050711165921-4978aa7ce1285ad5
bzrlib/transport/decorator.py decorator.py-20060402223305-e913a0f25319ab42
bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2008-12-01 23:20:22 +0000
+++ b/NEWS 2008-12-03 16:40:38 +0000
@@ -23,6 +23,9 @@
* Give proper error message for diff with non-existent dotted revno.
(Marius Kruger, #301969)
+ * Preserve transport decorators while following redirections.
+ (Vincent Ladeuil, #245964)
+
* ``Transport.readv()`` defaults to not reading more than 100MB in a
single array. Further ``RemoteTransport.readv`` sets this to 5MB to
work better with how it splits its requests.
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py 2008-12-01 19:07:21 +0000
+++ b/bzrlib/bzrdir.py 2008-12-03 16:40:38 +0000
@@ -773,29 +773,20 @@
:param transport: Transport containing the bzrdir.
:param _unsupported: private.
"""
+ # Keep initial base since 'transport' may be modified while following
+ # the redirections.
base = transport.base
-
def find_format(transport):
return transport, BzrDirFormat.find_format(
transport, _server_formats=_server_formats)
def redirected(transport, e, redirection_notice):
- qualified_source = e.get_source_url()
- relpath = transport.relpath(qualified_source)
- if not e.target.endswith(relpath):
- # Not redirected to a branch-format, not a branch
- raise errors.NotBranchError(path=e.target)
- target = e.target[:-len(relpath)]
+ redirected_transport = transport._redirected_to(e)
+ if redirected_transport is None:
+ raise errors.NotBranchError(base)
note('%s is%s redirected to %s',
- transport.base, e.permanently, target)
- # Let's try with a new transport
- # FIXME: If 'transport' has a qualifier, this should
- # be applied again to the new transport *iff* the
- # schemes used are the same. Uncomment this code
- # once the function (and tests) exist.
- # -- vila20070212
- #target = urlutils.copy_url_qualifiers(original, target)
- return get_transport(target)
+ transport.base, e.permanently, redirected_transport.base)
+ return redirected_transport
try:
transport, format = do_catching_redirections(find_format,
=== modified file 'bzrlib/tests/test_bzrdir.py'
--- a/bzrlib/tests/test_bzrdir.py 2008-09-25 22:25:09 +0000
+++ b/bzrlib/tests/test_bzrdir.py 2008-12-03 16:40:38 +0000
@@ -48,15 +48,16 @@
TestSkipped,
test_sftp_transport
)
-from bzrlib.tests.http_server import HttpServer
-from bzrlib.tests.http_utils import (
- TestCaseWithTwoWebservers,
- HTTPServerRedirecting,
+from bzrlib.tests import(
+ http_server,
+ http_utils,
)
from bzrlib.tests.test_http import TestWithTransport_pycurl
from bzrlib.transport import get_transport
from bzrlib.transport.http._urllib import HttpTransport_urllib
from bzrlib.transport.memory import MemoryServer
+from bzrlib.transport.nosmart import NoSmartTransportDecorator
+from bzrlib.transport.readonly import ReadonlyTransportDecorator
from bzrlib.repofmt import knitrepo, weaverepo
@@ -547,7 +548,7 @@
def setUp(self):
super(ChrootedTests, self).setUp()
if not self.vfs_transport_factory == MemoryServer:
- self.transport_readonly_server = HttpServer
+ self.transport_readonly_server = http_server.HttpServer
def local_branch_path(self, branch):
return os.path.realpath(urlutils.local_path_from_url(branch.base))
@@ -1064,8 +1065,8 @@
workingtree.WorkingTreeFormat3)
-class TestHTTPRedirectionLoop(object):
- """Test redirection loop between two http servers.
+class TestHTTPRedirections(object):
+ """Test redirection between two http servers.
This MUST be used by daughter classes that also inherit from
TestCaseWithTwoWebservers.
@@ -1075,62 +1076,85 @@
run, its implementation being incomplete.
"""
- # Should be defined by daughter classes to ensure redirection
- # still use the same transport implementation (not currently
- # enforced as it's a bit tricky to get right (see the FIXME
- # in BzrDir.open_from_transport for the unique use case so
- # far)
- _qualifier = None
-
def create_transport_readonly_server(self):
- return HTTPServerRedirecting()
+ return http_utils.HTTPServerRedirecting()
def create_transport_secondary_server(self):
- return HTTPServerRedirecting()
+ return http_utils.HTTPServerRedirecting()
def setUp(self):
- # Both servers redirect to each server creating a loop
- super(TestHTTPRedirectionLoop, self).setUp()
+ super(TestHTTPRedirections, self).setUp()
# The redirections will point to the new server
self.new_server = self.get_readonly_server()
# The requests to the old server will be redirected
self.old_server = self.get_secondary_server()
# Configure the redirections
self.old_server.redirect_to(self.new_server.host, self.new_server.port)
+
+ def test_loop(self):
+ # Both servers redirect to each other creating a loop
self.new_server.redirect_to(self.old_server.host, self.old_server.port)
-
- def _qualified_url(self, host, port):
- return 'http+%s://%s:%s' % (self._qualifier, host, port)
-
- def test_loop(self):
# Starting from either server should loop
- old_url = self._qualified_url(self.old_server.host,
+ old_url = self._qualified_url(self.old_server.host,
self.old_server.port)
oldt = self._transport(old_url)
self.assertRaises(errors.NotBranchError,
bzrdir.BzrDir.open_from_transport, oldt)
- new_url = self._qualified_url(self.new_server.host,
+ new_url = self._qualified_url(self.new_server.host,
self.new_server.port)
newt = self._transport(new_url)
self.assertRaises(errors.NotBranchError,
bzrdir.BzrDir.open_from_transport, newt)
-
-class TestHTTPRedirections_urllib(TestHTTPRedirectionLoop,
- TestCaseWithTwoWebservers):
+ def test_qualifier_preserved(self):
+ wt = self.make_branch_and_tree('branch')
+ old_url = self._qualified_url(self.old_server.host,
+ self.old_server.port)
+ start = self._transport(old_url).clone('branch')
+ bdir = bzrdir.BzrDir.open_from_transport(start)
+ # Redirection should preserve the qualifier, hence the transport class
+ # itself.
+ self.assertIsInstance(bdir.root_transport, type(start))
+
+
+class TestHTTPRedirections_urllib(TestHTTPRedirections,
+ http_utils.TestCaseWithTwoWebservers):
"""Tests redirections for urllib implementation"""
- _qualifier = 'urllib'
_transport = HttpTransport_urllib
+ def _qualified_url(self, host, port):
+ return 'http+urllib://%s:%s' % (host, port)
+
class TestHTTPRedirections_pycurl(TestWithTransport_pycurl,
- TestHTTPRedirectionLoop,
- TestCaseWithTwoWebservers):
+ TestHTTPRedirections,
+ http_utils.TestCaseWithTwoWebservers):
"""Tests redirections for pycurl implementation"""
- _qualifier = 'pycurl'
+ def _qualified_url(self, host, port):
+ return 'http+pycurl://%s:%s' % (host, port)
+
+
+class TestHTTPRedirections_nosmart(TestHTTPRedirections,
+ http_utils.TestCaseWithTwoWebservers):
+ """Tests redirections for the nosmart decorator"""
+
+ _transport = NoSmartTransportDecorator
+
+ def _qualified_url(self, host, port):
+ return 'nosmart+http://%s:%s' % (host, port)
+
+
+class TestHTTPRedirections_readonly(TestHTTPRedirections,
+ http_utils.TestCaseWithTwoWebservers):
+ """Tests redirections for readonly decoratror"""
+
+ _transport = ReadonlyTransportDecorator
+
+ def _qualified_url(self, host, port):
+ return 'readonly+http://%s:%s' % (host, port)
class TestDotBzrHidden(TestCaseWithTransport):
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2008-09-26 15:28:29 +0000
+++ b/bzrlib/tests/test_http.py 2008-12-03 16:40:38 +0000
@@ -1263,7 +1263,7 @@
('bundle',
'# Bazaar revision bundle v0.9\n#\n')
],)
-
+ # The requests to the old server will be redirected to the new server
self.old_transport = self._transport(self.old_server.get_url())
def test_redirected(self):
=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py 2008-12-01 23:20:22 +0000
+++ b/bzrlib/transport/__init__.py 2008-12-03 16:40:38 +0000
@@ -1249,6 +1249,21 @@
# should be asked to ConnectedTransport only.
return None
+ def _redirected_to(self, exception):
+ """Returns a transport suitable to re-issue a redirected request.
+
+ :param exception: A RedirectRequested exception.
+
+ The redirection can be handled only if the relpath involved is not
+ renamed by the redirection.
+
+ :returns: A transport or None.
+ """
+ # This returns None by default, meaning the transport can't handle the
+ # redirection.
+ return None
+
+
class _SharedConnection(object):
"""A connection shared between several transports."""
=== modified file 'bzrlib/transport/decorator.py'
--- a/bzrlib/transport/decorator.py 2008-04-24 07:22:53 +0000
+++ b/bzrlib/transport/decorator.py 2008-12-03 16:40:38 +0000
@@ -46,16 +46,14 @@
"""
prefix = self._get_url_prefix()
if not url.startswith(prefix):
- raise ValueError(
- "url %r doesn't start with decorator prefix %r" % \
- (url, prefix))
- decorated_url = url[len(prefix):]
+ raise ValueError("url %r doesn't start with decorator prefix %r" %
+ (url, prefix))
+ not_decorated_url = url[len(prefix):]
if _decorated is None:
- self._decorated = get_transport(decorated_url)
+ self._decorated = get_transport(not_decorated_url)
else:
self._decorated = _decorated
- super(TransportDecorator, self).__init__(
- prefix + self._decorated.base)
+ super(TransportDecorator, self).__init__(prefix + self._decorated.base)
def abspath(self, relpath):
"""See Transport.abspath()."""
@@ -170,6 +168,12 @@
"""See Transport.lock_write."""
return self._decorated.lock_write(relpath)
+ def _redirected_to(self, exception):
+ redirected = self._decorated._redirected_to(exception)
+ if redirected is not None:
+ return self.__class__(self._get_url_prefix() + redirected.base,
+ redirected)
+
class DecoratorServer(Server):
"""Server for the TransportDecorator for testing with.
=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py 2008-09-26 14:28:56 +0000
+++ b/bzrlib/transport/http/__init__.py 2008-12-03 16:40:38 +0000
@@ -41,6 +41,7 @@
from bzrlib.transport import (
ConnectedTransport,
_CoalescedOffset,
+ get_transport,
Transport,
)
@@ -451,17 +452,6 @@
"""
raise errors.TransportNotPossible('http does not support lock_write()')
- def clone(self, offset=None):
- """Return a new HttpTransportBase with root at self.base + offset
-
- We leave the daughter classes take advantage of the hint
- that it's a cloning not a raw creation.
- """
- if offset is None:
- return self.__class__(self.base, self)
- else:
- return self.__class__(self.abspath(offset), self)
-
def _attempted_range_header(self, offsets, tail_amount):
"""Prepare a HTTP Range header at a level the server should accept.
@@ -517,6 +507,54 @@
return ','.join(strings)
+ def _redirected_to(self, exception):
+ """Returns a transport suitable to re-issue a redirected request.
+
+ :param exception: A RedirectRequested exception.
+
+ The redirection can be handled only if the relpath involved is not
+ renamed by the redirection.
+
+ :returns: A transport or None.
+ """
+ relpath = self.relpath(exception.get_source_url())
+ if not exception.target.endswith(relpath):
+ # The final part of the url has been renamed, we can't handle the
+ # redirection.
+ return None
+ new_transport = None
+ (scheme,
+ user, password,
+ host, port,
+ path) = self._split_url(exception.target)
+ # Recalculate base path. This is needed to ensure that when the
+ # redirected tranport will be used to re-try whatever request was
+ # redirected, we end up with the same url
+ base_path = path[:-len(relpath)]
+ if scheme == self._unqualified_scheme:
+ # Same protocol (i.e. http[s])
+ if (host == self._host
+ and port == self._port
+ and (user is None or user == self._user)):
+ # If a user is specified, it should match, we don't care about
+ # passwords, wrong passwords will be rejected anyway.
+ new_transport = self.clone(base_path)
+ else:
+ # Rebuild the url preserving the qualified scheme
+ new_url = self._unsplit_url(self._scheme,
+ user, password,
+ host, port,
+ base_path)
+ new_transport = get_transport(new_url)
+ else:
+ # Redirected to a different protocol
+ new_url = self._unsplit_url(scheme,
+ user, password,
+ host, port,
+ base_path)
+ new_transport = get_transport(new_url)
+ return new_transport
+
# TODO: May be better located in smart/medium.py with the other
# SmartMedium classes
More information about the bazaar-commits
mailing list