Rev 3903: Fix redirection related bugs: #245964, #265070, #270863 and #303959 in http://bazaar.launchpad.net/%7Evila/bzr/bzr.integration

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Dec 12 13:10:24 GMT 2008


At http://bazaar.launchpad.net/%7Evila/bzr/bzr.integration

------------------------------------------------------------
revno: 3903
revision-id: v.ladeuil+lp at free.fr-20081212130926-ov09evfb53npj0z0
parent: pqm at pqm.ubuntu.com-20081212125032-wboskm0umuvt44ft
parent: v.ladeuil+lp at free.fr-20081212122548-cd4vs5pp3k2i9e25
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: bzr.integration
timestamp: Fri 2008-12-12 14:09:26 +0100
message:
  Fix redirection related bugs: #245964, #265070, #270863 and #303959
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/push.py                 push.py-20080606021927-5fe39050e8xne9un-1
  bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
  bzrlib/tests/test_bzrdir.py    test_bzrdir.py-20060131065654-deba40eef51cf220
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
  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
  bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
  bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
    ------------------------------------------------------------
    revno: 3878.4.7
    revision-id: v.ladeuil+lp at free.fr-20081212122548-cd4vs5pp3k2i9e25
    parent: v.ladeuil+lp at free.fr-20081204171246-p28b3u0e2alz53iv
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Fri 2008-12-12 13:25:48 +0100
    message:
      Fixed as per Robert's review.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase._redirected_to): Better comment.
    modified:
      bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
    ------------------------------------------------------------
    revno: 3878.4.6
    revision-id: v.ladeuil+lp at free.fr-20081204171246-p28b3u0e2alz53iv
    parent: v.ladeuil+lp at free.fr-20081204160251-37f920alwn9t0i4p
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Thu 2008-12-04 18:12:46 +0100
    message:
      Fix bug #270863 by preserving 'bzr+http[s]' decorator.
      
      * bzrlib/transport/remote.py:
      (RemoteHTTPTransport._redirected_to): Specific implementation to
      handle the redirections.
      
      * bzrlib/transport/http/_urllib.py:
      (HttpTransport_urllib.__init__): Fix parameter order.
      
      * bzrlib/transport/http/_pycurl.py:
      (PyCurlTransport.__init__): Fix parameter order.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase.external_url): Semi drive-by fix, external_url
      shouldn't expose the implementation qualifier (it's private to bzr
      not externally usable).
      
      * bzrlib/transport/decorator.py:
      (TransportDecorator._redirected_to): Cleanup.
      
      * bzrlib/tests/test_smart_transport.py:
      (RemoteHTTPTransportTestCase): Add specific tests for
      _redirected_to.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
      bzrlib/transport/decorator.py  decorator.py-20060402223305-e913a0f25319ab42
      bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
      bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
      bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
      bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
    ------------------------------------------------------------
    revno: 3878.4.5
    revision-id: v.ladeuil+lp at free.fr-20081204160251-37f920alwn9t0i4p
    parent: v.ladeuil+lp at free.fr-20081204155340-atj2k9w0cbcuob2k
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Thu 2008-12-04 17:02:51 +0100
    message:
      Don't use the exception as a parameter for _redirected_to.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase._redirected_to): Update prototype.
      
      * bzrlib/transport/decorator.py:
      (TransportDecorator._redirected_to): Update prototype.
      
      * bzrlib/tests/blackbox/test_push.py:
      (RedirectingMemoryTransport): Update prototype.
    modified:
      bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
      bzrlib/push.py                 push.py-20080606021927-5fe39050e8xne9un-1
      bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
      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
    ------------------------------------------------------------
    revno: 3878.4.4
    revision-id: v.ladeuil+lp at free.fr-20081204155340-atj2k9w0cbcuob2k
    parent: v.ladeuil+lp at free.fr-20081204115030-4dytul8qv82viscm
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Thu 2008-12-04 16:53:40 +0100
    message:
      Cleanup.
      
      * bzrlib/transport/http/_urllib.py:
      (HttpTransport_urllib._perform): Update RedirectRequested creation.
      
      * bzrlib/transport/http/_pycurl.py:
      (PyCurlTransport._curl_perform): Update RedirectRequested creation.
      
      * bzrlib/tests/blackbox/test_push.py:
      (RedirectingMemoryTransport.mkdir): RedirectRequested wants urls
      not paths. Be more precise with the paths used too.
      (TestPushRedirect.test_push_redirects_on_mkdir,
      TestPushRedirect.test_push_gracefully_handles_too_many_redirects):
      Use '-d' instead of chdir().
      
      * bzrlib/push.py:
      (_show_push_branch.redirected): Updated and note() added (why
      wasn't it there in the first place ?).
      
      * bzrlib/errors.py:
      (RedirectRequested): Simplified now that the redirection stuff is
      handled by the transport itself.
    modified:
      bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
      bzrlib/push.py                 push.py-20080606021927-5fe39050e8xne9un-1
      bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
      bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
      bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
      bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
    ------------------------------------------------------------
    revno: 3878.4.3
    revision-id: v.ladeuil+lp at free.fr-20081204115030-4dytul8qv82viscm
    parent: v.ladeuil+lp at free.fr-20081204112556-g3i9tf396wygm1b7
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Thu 2008-12-04 12:50:30 +0100
    message:
      Fix bug #303959 by returning a transport based on the same url
      when redirected to an url with a slash appened.
      
      * bzrlib/tests/test_http.py:
      (Test_redirected_to.test_redirected_to_self_with_slash): Reproduce
      bug #303959.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
    ------------------------------------------------------------
    revno: 3878.4.2
    revision-id: v.ladeuil+lp at free.fr-20081204112556-g3i9tf396wygm1b7
    parent: v.ladeuil+lp at free.fr-20081203164038-5ghthsgdei3ank1j
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: 303959-redirection
    timestamp: Thu 2008-12-04 12:25:56 +0100
    message:
      Fix bug #265070 by providing a finer sieve for accepted redirections.
      
      * bzrlib/transport/http/_urllib.py:
      (HttpTransport_urllib.__init__): Provides the implementation name.
      
      * bzrlib/transport/http/_pycurl.py:
      (PyCurlTransport.__init__): Provides the implementation name.
      
      * bzrlib/transport/http/__init__.py:
      (HttpTransportBase.__init__): Make the implementation a mandatory
      parameter.
      (HttpTransportBase._redirected_to.relpath): We need a dedicated
      relpath method to be able to cope with slight differences between
      the url sent and the url received from the server.
      (HttpTransportBase._redirected_to): Fix bug #265070 by accepting
      more redirection kinds and handling them: http <-> https and user
      specified in the url.
      
      * bzrlib/tests/test_http.py:
      (Test_redirected_to): More complete tests for _redirected_to().
    modified:
      bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
      bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
      bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
      bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
    ------------------------------------------------------------
    revno: 3878.4.1
    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-12 12:14:01 +0000
+++ b/NEWS	2008-12-12 13:09:26 +0000
@@ -46,6 +46,12 @@
     * Give proper error message for diff with non-existent dotted revno.
       (Marius Kruger, #301969)
 
+    * Preserve transport decorators while following redirections.
+      (Vincent Ladeuil, #245964, #270863)
+
+    * Provides a finer and more robust filter for accepted redirections.
+      (Vincent Ladeuil, #303959, #265070)
+
     * ``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-05 17:21:36 +0000
+++ b/bzrlib/bzrdir.py	2008-12-12 13:09:26 +0000
@@ -782,29 +782,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.source, e.target)
+            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/errors.py'
--- a/bzrlib/errors.py	2008-12-11 16:25:46 +0000
+++ b/bzrlib/errors.py	2008-12-12 13:09:26 +0000
@@ -1661,49 +1661,15 @@
 
     _fmt = '%(source)s is%(permanently)s redirected to %(target)s'
 
-    def __init__(self, source, target, is_permanent=False, qual_proto=None):
+    def __init__(self, source, target, is_permanent=False):
         self.source = source
         self.target = target
         if is_permanent:
             self.permanently = ' permanently'
         else:
             self.permanently = ''
-        self._qualified_proto = qual_proto
         TransportError.__init__(self)
 
-    def _requalify_url(self, url):
-        """Restore the qualified proto in front of the url"""
-        # When this exception is raised, source and target are in
-        # user readable format. But some transports may use a
-        # different proto (http+urllib:// will present http:// to
-        # the user. If a qualified proto is specified, the code
-        # trapping the exception can get the qualified urls to
-        # properly handle the redirection themself (creating a
-        # new transport object from the target url for example).
-        # But checking that the scheme of the original and
-        # redirected urls are the same can be tricky. (see the
-        # FIXME in BzrDir.open_from_transport for the unique use
-        # case so far).
-        if self._qualified_proto is None:
-            return url
-
-        # The TODO related to NotBranchError mention that doing
-        # that kind of manipulation on the urls may not be the
-        # exception object job. On the other hand, this object is
-        # the interface between the code and the user so
-        # presenting the urls in different ways is indeed its
-        # job...
-        import urlparse
-        proto, netloc, path, query, fragment = urlparse.urlsplit(url)
-        return urlparse.urlunsplit((self._qualified_proto, netloc, path,
-                                   query, fragment))
-
-    def get_source_url(self):
-        return self._requalify_url(self.source)
-
-    def get_target_url(self):
-        return self._requalify_url(self.target)
-
 
 class TooManyRedirections(TransportError):
 

=== modified file 'bzrlib/push.py'
--- a/bzrlib/push.py	2008-08-28 19:35:29 +0000
+++ b/bzrlib/push.py	2008-12-04 16:02:51 +0000
@@ -74,8 +74,9 @@
             transport.mkdir('.')
             return transport
 
-        def redirected(redirected_transport, e, redirection_notice):
-            return transport.get_transport(e.get_target_url())
+        def redirected(transport, e, redirection_notice):
+            note(redirection_notice)
+            return transport._redirected_to(e.source, e.target)
 
         try:
             to_transport = transport.do_catching_redirections(

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- a/bzrlib/tests/blackbox/test_push.py	2008-09-25 22:25:09 +0000
+++ b/bzrlib/tests/blackbox/test_push.py	2008-12-04 16:02:51 +0000
@@ -22,6 +22,7 @@
 
 from bzrlib import (
     errors,
+    transport,
     urlutils,
     )
 from bzrlib.branch import Branch
@@ -30,7 +31,6 @@
 from bzrlib.repofmt.knitrepo import RepositoryFormatKnit1
 from bzrlib.tests.blackbox import ExternalBase
 from bzrlib.tests.http_server import HttpServer
-from bzrlib.transport import register_transport, unregister_transport
 from bzrlib.transport.memory import MemoryServer, MemoryTransport
 from bzrlib.uncommit import uncommit
 from bzrlib.urlutils import local_path_from_url
@@ -353,17 +353,24 @@
 
 class RedirectingMemoryTransport(MemoryTransport):
 
-    def mkdir(self, path, mode=None):
-        path = self.abspath(path)[len(self._scheme):]
-        if path == '/source':
-            raise errors.RedirectRequested(
-                path, self._scheme + '/target', is_permanent=True)
-        elif path == '/infinite-loop':
-            raise errors.RedirectRequested(
-                path, self._scheme + '/infinite-loop', is_permanent=True)
+    def mkdir(self, relpath, mode=None):
+        from bzrlib.trace import mutter
+        mutter('cwd: %r, rel: %r, abs: %r' % (self._cwd, relpath, abspath))
+        if self._cwd == '/source/':
+            raise errors.RedirectRequested(self.abspath(relpath),
+                                           self.abspath('../target'),
+                                           is_permanent=True)
+        elif self._cwd == '/infinite-loop/':
+            raise errors.RedirectRequested(self.abspath(relpath),
+                                           self.abspath('../infinite-loop'),
+                                           is_permanent=True)
         else:
             return super(RedirectingMemoryTransport, self).mkdir(
-                path, mode)
+                relpath, mode)
+
+    def _redirected_to(self, source, target):
+        # We do accept redirections
+        return transport.get_transport(target)
 
 
 class RedirectingMemoryServer(MemoryServer):
@@ -373,7 +380,7 @@
         self._files = {}
         self._locks = {}
         self._scheme = 'redirecting-memory+%s:///' % id(self)
-        register_transport(self._scheme, self._memory_factory)
+        transport.register_transport(self._scheme, self._memory_factory)
 
     def _memory_factory(self, url):
         result = RedirectingMemoryTransport(url)
@@ -383,7 +390,7 @@
         return result
 
     def tearDown(self):
-        unregister_transport(self._scheme, self._memory_factory)
+        transport.unregister_transport(self._scheme, self._memory_factory)
 
 
 class TestPushRedirect(ExternalBase):
@@ -406,10 +413,8 @@
         This is added primarily to handle lp:/ URI support, so that users can
         push to new branches by specifying lp:/ URIs.
         """
-        os.chdir('tree')
         destination_url = self.memory_server.get_url() + 'source'
-        self.run_bzr('push %s' % destination_url)
-        os.chdir('..')
+        self.run_bzr(['push', '-d', 'tree', destination_url])
 
         local_revision = Branch.open('tree').last_revision()
         remote_revision = Branch.open(
@@ -420,11 +425,9 @@
         """Push fails gracefully if the mkdir generates a large number of
         redirects.
         """
-        os.chdir('tree')
         destination_url = self.memory_server.get_url() + 'infinite-loop'
         out, err = self.run_bzr_error(
             ['Too many redirections trying to make %s\\.\n'
              % re.escape(destination_url)],
-            'push %s' % destination_url, retcode=3)
-        os.chdir('..')
+            ['push', '-d', 'tree', destination_url], retcode=3)
         self.assertEqual('', out)

=== 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-04 16:02:51 +0000
@@ -120,6 +120,7 @@
     t_adapter = TransportAdapter()
     t_classes= (TestHttpTransportRegistration,
                 TestHttpTransportUrls,
+                Test_redirected_to,
                 )
     is_testing_for_transports = tests.condition_isinstance(t_classes)
 
@@ -1263,7 +1264,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):
@@ -1743,3 +1744,47 @@
                           t.get_smart_medium().send_http_smart_request,
                           'whatever')
 
+class Test_redirected_to(tests.TestCase):
+
+    def test_redirected_to_subdir(self):
+        t = self._transport('http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'http://www.example.com/foo/subdir')
+        self.assertIsInstance(r, type(t))
+        # Both transports share the some connection
+        self.assertEquals(t._get_connection(), r._get_connection())
+
+    def test_redirected_to_self_with_slash(self):
+        t = self._transport('http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'http://www.example.com/foo/')
+        self.assertIsInstance(r, type(t))
+        # Both transports share the some connection (one can argue that we
+        # should return the exact same transport here, but that seems
+        # overkill).
+        self.assertEquals(t._get_connection(), r._get_connection())
+
+    def test_redirected_to_host(self):
+        t = self._transport('http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'http://foo.example.com/foo/subdir')
+        self.assertIsInstance(r, type(t))
+
+    def test_redirected_to_same_host_sibling_protocol(self):
+        t = self._transport('http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'https://www.example.com/foo')
+        self.assertIsInstance(r, type(t))
+
+    def test_redirected_to_same_host_different_protocol(self):
+        t = self._transport('http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'ftp://www.example.com/foo')
+        self.assertNotEquals(type(r), type(t))
+
+    def test_redirected_to_different_host_same_user(self):
+        t = self._transport('http://joe@www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'https://foo.example.com/foo')
+        self.assertIsInstance(r, type(t))
+        self.assertEquals(t._user, r._user)

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2008-10-17 06:01:10 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2008-12-04 17:12:46 +0000
@@ -3357,8 +3357,8 @@
         # requests for child URLs of that to the original URL.  i.e., we want to
         # POST to "bzr+http://host/foo/.bzr/smart" and never something like
         # "bzr+http://host/foo/.bzr/branch/.bzr/smart".  So, a cloned
-        # RemoteHTTPTransport remembers the initial URL, and adjusts the relpaths
-        # it sends in smart requests accordingly.
+        # RemoteHTTPTransport remembers the initial URL, and adjusts the
+        # relpaths it sends in smart requests accordingly.
         base_transport = remote.RemoteHTTPTransport('bzr+http://host/path')
         new_transport = base_transport.clone('child_dir')
         self.assertEqual(base_transport._http_transport,
@@ -3384,7 +3384,34 @@
             'c/',
             new_transport._client.remote_path_from_transport(new_transport))
 
-        
+    def test__redirect_to(self):
+        t = remote.RemoteHTTPTransport('bzr+http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'http://www.example.com/bar')
+        self.assertEquals(type(r), type(t))
+
+    def test__redirect_sibling_protocol(self):
+        t = remote.RemoteHTTPTransport('bzr+http://www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'https://www.example.com/bar')
+        self.assertEquals(type(r), type(t))
+        self.assertStartsWith(r.base, 'bzr+https')
+
+    def test__redirect_to_with_user(self):
+        t = remote.RemoteHTTPTransport('bzr+http://joe@www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'http://www.example.com/bar')
+        self.assertEquals(type(r), type(t))
+        self.assertEquals('joe', t._user)
+        self.assertEquals(t._user, r._user)
+
+    def test_redirected_to_same_host_different_protocol(self):
+        t = remote.RemoteHTTPTransport('bzr+http://joe@www.example.com/foo')
+        r = t._redirected_to('http://www.example.com/foo',
+                             'ftp://www.example.com/foo')
+        self.assertNotEquals(type(r), type(t))
+
+
 # TODO: Client feature that does get_bundle and then installs that into a
 # branch; this can be used in place of the regular pull/fetch operation when
 # coming from a smart server.

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2008-12-09 16:35:33 +0000
+++ b/bzrlib/transport/__init__.py	2008-12-12 13:09:26 +0000
@@ -1249,6 +1249,22 @@
         # should be asked to ConnectedTransport only.
         return None
 
+    def _redirected_to(self, source, target):
+        """Returns a transport suitable to re-issue a redirected request.
+
+        :param source: The source url as returned by the server.
+        :param target: The target url as returned by the server.
+
+        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-04 17:12:46 +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,14 @@
         """See Transport.lock_write."""
         return self._decorated.lock_write(relpath)
 
+    def _redirected_to(self, source, target):
+        redirected = self._decorated._redirected_to(source, target)
+        if redirected is not None:
+            return self.__class__(self._get_url_prefix() + redirected.base,
+                                  redirected)
+        else:
+            return None
+
 
 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-12 12:25:48 +0000
@@ -41,6 +41,7 @@
 from bzrlib.transport import (
     ConnectedTransport,
     _CoalescedOffset,
+    get_transport,
     Transport,
     )
 
@@ -92,16 +93,13 @@
     # _unqualified_scheme: "http" or "https"
     # _scheme: may have "+pycurl", etc
 
-    def __init__(self, base, _from_transport=None):
+    def __init__(self, base, _impl_name, _from_transport=None):
         """Set the base path where files will be stored."""
         proto_match = re.match(r'^(https?)(\+\w+)?://', base)
         if not proto_match:
             raise AssertionError("not a http url: %r" % base)
         self._unqualified_scheme = proto_match.group(1)
-        impl_name = proto_match.group(2)
-        if impl_name:
-            impl_name = impl_name[1:]
-        self._impl_name = impl_name
+        self._impl_name = _impl_name
         super(HttpTransportBase, self).__init__(base,
                                                 _from_transport=_from_transport)
         self._medium = None
@@ -414,8 +412,12 @@
 
     def external_url(self):
         """See bzrlib.transport.Transport.external_url."""
-        # HTTP URL's are externally usable.
-        return self.base
+        # HTTP URL's are externally usable as long as they don't mention their
+        # implementation qualifier
+        return self._unsplit_url(self._unqualified_scheme,
+                                 self._user, self._password,
+                                 self._host, self._port,
+                                 self._path)
 
     def is_readonly(self):
         """See Transport.is_readonly."""
@@ -451,17 +453,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 +508,80 @@
 
         return ','.join(strings)
 
+    def _redirected_to(self, source, target):
+        """Returns a transport suitable to re-issue a redirected request.
+
+        :param source: The source url as returned by the server.
+        :param target: The target url as returned by the server.
+
+        The redirection can be handled only if the relpath involved is not
+        renamed by the redirection.
+
+        :returns: A transport or None.
+        """
+        def relpath(abspath):
+            """Returns the path relative to our base.
+
+            The constraints are weaker than the real relpath method because the
+            abspath is coming from the server and may slightly differ from our
+            base. We don't check the scheme, host, port, user, password parts,
+            relying on the caller to give us a proper url (i.e. one returned by
+            the server mirroring the one we sent).
+            """
+            (scheme,
+             user, password,
+             host, port,
+             path) = self._split_url(abspath)
+            pl = len(self._path)
+            return path[pl:].strip('/')
+
+        relpath = relpath(source)
+        if not 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(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 in ('http', 'https'):
+            # Same protocol family (i.e. http[s]), we will preserve the same
+            # http client implementation when a redirection occurs from one to
+            # the other (otherwise users may be surprised that bzr switches
+            # from one implementation to the other, and devs may suffer
+            # debugging it).
+            if (scheme == self._unqualified_scheme
+                and 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 scheme qualification and the
+                # credentials (if they don't apply, the redirected to server
+                # will tell us, but if they do apply, we avoid prompting the
+                # user)
+                redir_scheme = scheme + '+' + self._impl_name
+                new_url = self._unsplit_url(redir_scheme,
+                                            self._user, self._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

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2008-08-29 07:40:35 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2008-12-04 17:12:46 +0000
@@ -105,9 +105,9 @@
     """
 
     def __init__(self, base, _from_transport=None):
-        super(PyCurlTransport, self).__init__(base,
+        super(PyCurlTransport, self).__init__(base, 'pycurl',
                                               _from_transport=_from_transport)
-        if base.startswith('https'):
+        if self._unqualified_scheme == 'https':
             # Check availability of https into pycurl supported
             # protocols
             supported = pycurl.version_info()[8]
@@ -366,8 +366,7 @@
             redirected_to = msg.getheader('location')
             raise errors.RedirectRequested(url,
                                            redirected_to,
-                                           is_permanent=(code == 301),
-                                           qual_proto=self._scheme)
+                                           is_permanent=(code == 301))
 
 
 def get_test_permutations():

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2008-05-29 06:43:43 +0000
+++ b/bzrlib/transport/http/_urllib.py	2008-12-04 17:12:46 +0000
@@ -43,7 +43,7 @@
 
     def __init__(self, base, _from_transport=None):
         super(HttpTransport_urllib, self).__init__(
-            base, _from_transport=_from_transport)
+            base, 'urllib', _from_transport=_from_transport)
         if _from_transport is not None:
             self._opener = _from_transport._opener
         else:
@@ -90,8 +90,7 @@
                 and code in (301, 302, 303, 307):
             raise errors.RedirectRequested(request.get_full_url(),
                                            request.redirected_to,
-                                           is_permanent=(code == 301),
-                                           qual_proto=self._scheme)
+                                           is_permanent=(code == 301))
 
         if request.redirected_to is not None:
             trace.mutter('redirected from: %s to: %s' % (request.get_full_url(),

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2008-12-01 23:20:22 +0000
+++ b/bzrlib/transport/remote.py	2008-12-04 17:12:46 +0000
@@ -568,6 +568,17 @@
                                    _from_transport=self,
                                    http_transport=self._http_transport)
 
+    def _redirected_to(self, source, target):
+        """See transport._redirected_to"""
+        redirected = self._http_transport._redirected_to(source, target)
+        if (redirected is not None
+            and isinstance(redirected, type(self._http_transport))):
+            return RemoteHTTPTransport('bzr+' + redirected.external_url(),
+                                       http_transport=redirected)
+        else:
+            # Either None or a transport for a different protocol
+            return redirected
+
 
 def get_test_permutations():
     """Return (transport, server) permutations for testing."""



More information about the bazaar-commits mailing list