Rev 4800: Make sure all redirection code paths can handle authentication. in file:///home/vila/src/bzr/bugs/395714-auth-redirect/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Dec 3 17:11:57 GMT 2009


At file:///home/vila/src/bzr/bugs/395714-auth-redirect/

------------------------------------------------------------
revno: 4800
revision-id: v.ladeuil+lp at free.fr-20091203171157-2lo1a9ej0fx3743b
parent: v.ladeuil+lp at free.fr-20091126143931-0frv0tk40gclv9yf
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 395714-auth-redirect
timestamp: Thu 2009-12-03 18:11:57 +0100
message:
  Make sure all redirection code paths can handle authentication.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (AbstractAuthHandler.auth_required): Provide the necessary
  attributes if the caller didn't.
  
  * bzrlib/tests/test_http.py:
  (install_redirected_request): Factor out the helper.
  (TestHTTPSilentRedirections.setUp): Use the above helper.
  (TestHTTPSilentRedirections.test_one_redirection,
  TestHTTPSilentRedirections.test_five_redirections): Delete the
  useless (hence misleading) code.
  (TestAuth.test_no_prompt_for_password_when_using_auth_config):
  Drive-by fix prompt go to stderr, stdout is not needed here.
  (TestAuthOnRedirected): Ensure we handle authentication on both
  code paths.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-11-25 15:02:09 +0000
+++ b/NEWS	2009-12-03 17:11:57 +0000
@@ -46,7 +46,7 @@
   2.4.  (Vincent Ladeuil, #475585)
 
 * Fixed bug with redirected URLs over authenticated HTTP.
-  (Glen Mailer, Neil Martinsen-Burrell, #395714)
+  (Glen Mailer, Neil Martinsen-Burrell, Vincent Ladeuil, #395714)
 
 
 Improvements

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-10-30 09:34:50 +0000
+++ b/bzrlib/tests/test_http.py	2009-12-03 17:11:57 +0000
@@ -1366,6 +1366,14 @@
         self.follow_redirections = True
 
 
+def install_redirected_request(test):
+    test.original_class = _urllib2_wrappers.Request
+    def restore():
+        _urllib2_wrappers.Request = test.original_class
+    _urllib2_wrappers.Request = RedirectedRequest
+    test.addCleanup(restore)
+
+
 class TestHTTPSilentRedirections(http_utils.TestCaseWithRedirectedWebserver):
     """Test redirections.
 
@@ -1385,8 +1393,7 @@
             raise tests.TestNotApplicable(
                 "pycurl doesn't redirect silently annymore")
         super(TestHTTPSilentRedirections, self).setUp()
-        self.setup_redirected_request()
-        self.addCleanup(self.cleanup_redirected_request)
+        install_redirected_request(self)
         self.build_tree_contents([('a','a'),
                                   ('1/',),
                                   ('1/a', 'redirected once'),
@@ -1402,13 +1409,6 @@
 
         self.old_transport = self._transport(self.old_server.get_url())
 
-    def setup_redirected_request(self):
-        self.original_class = _urllib2_wrappers.Request
-        _urllib2_wrappers.Request = RedirectedRequest
-
-    def cleanup_redirected_request(self):
-        _urllib2_wrappers.Request = self.original_class
-
     def create_transport_secondary_server(self):
         """Create the secondary server, redirections are defined in the tests"""
         return http_utils.HTTPServerRedirecting(
@@ -1418,7 +1418,6 @@
         t = self.old_transport
 
         req = RedirectedRequest('GET', t.abspath('a'))
-        req.follow_redirections = True
         new_prefix = 'http://%s:%s' % (self.new_server.host,
                                        self.new_server.port)
         self.old_server.redirections = \
@@ -1429,7 +1428,6 @@
         t = self.old_transport
 
         req = RedirectedRequest('GET', t.abspath('a'))
-        req.follow_redirections = True
         old_prefix = 'http://%s:%s' % (self.old_server.host,
                                        self.old_server.port)
         new_prefix = 'http://%s:%s' % (self.new_server.host,
@@ -1638,7 +1636,7 @@
         self.server.add_user(user, password)
         t = self.get_user_transport(user, None)
         ui.ui_factory = tests.TestUIFactory(stdin=stdin_content,
-                                            stdout=tests.StringIOWrapper())
+                                            stderr=tests.StringIOWrapper())
         # Create a minimal config file with the right password
         conf = config.AuthenticationConfig()
         conf._get_config().update(
@@ -2154,3 +2152,79 @@
     def assertActivitiesMatch(self):
         # Nothing to check here
         pass
+
+
+class TestAuthOnRedirected(http_utils.TestCaseWithRedirectedWebserver):
+    """Test authentication on the redirected http server."""
+
+    _auth_header = 'Authorization'
+    _password_prompt_prefix = ''
+    _username_prompt_prefix = ''
+    _auth_server = http_utils.HTTPBasicAuthServer
+    _transport = _urllib.HttpTransport_urllib
+
+    def create_transport_readonly_server(self):
+        return self._auth_server()
+
+    def create_transport_secondary_server(self):
+        """Create the secondary server redirecting to the primary server"""
+        new = self.get_readonly_server()
+
+        redirecting = http_utils.HTTPServerRedirecting()
+        redirecting.redirect_to(new.host, new.port)
+        return redirecting
+
+    def setUp(self):
+        super(TestAuthOnRedirected, self).setUp()
+        self.build_tree_contents([('a','a'),
+                                  ('1/',),
+                                  ('1/a', 'redirected once'),
+                                  ],)
+        new_prefix = 'http://%s:%s' % (self.new_server.host,
+                                       self.new_server.port)
+        self.old_server.redirections = [
+            ('(.*)', r'%s/1\1' % (new_prefix), 301),]
+        self.old_transport = self._transport(self.old_server.get_url())
+        self.new_server.add_user('joe', 'foo')
+
+    def get_a(self, transport):
+        return transport.get('a')
+
+    def test_auth_on_redirected_via_do_catching_redirections(self):
+        self.redirections = 0
+
+        def redirected(transport, exception, redirection_notice):
+            self.redirections += 1
+            dir, file = urlutils.split(exception.target)
+            return self._transport(dir)
+
+        stdout = tests.StringIOWrapper()
+        stderr = tests.StringIOWrapper()
+        ui.ui_factory = tests.TestUIFactory(stdin='joe\nfoo\n',
+                                            stdout=stdout, stderr=stderr)
+        self.assertEquals('redirected once',
+                          transport.do_catching_redirections(
+                self.get_a, self.old_transport, redirected).read())
+        self.assertEquals(1, self.redirections)
+        # stdin should be empty
+        self.assertEqual('', ui.ui_factory.stdin.readline())
+        self.assertEquals('', stdout.getvalue())
+
+    def test_auth_on_redirected_via_following_redirections(self):
+        self.new_server.add_user('joe', 'foo')
+        stdout = tests.StringIOWrapper()
+        stderr = tests.StringIOWrapper()
+        ui.ui_factory = tests.TestUIFactory(stdin='joe\nfoo\n',
+                                            stdout=stdout, stderr=stderr)
+        t = self.old_transport
+        req = RedirectedRequest('GET', t.abspath('a'))
+        new_prefix = 'http://%s:%s' % (self.new_server.host,
+                                       self.new_server.port)
+        self.old_server.redirections = [
+            ('(.*)', r'%s/1\1' % (new_prefix), 301),]
+        self.assertEquals('redirected once',t._perform(req).read())
+        # stdin should be empty
+        self.assertEqual('', ui.ui_factory.stdin.readline())
+        self.assertEquals('', stdout.getvalue())
+
+

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/transport/http/_urllib.py	2009-12-03 17:11:57 +0000
@@ -87,8 +87,8 @@
             self._update_credentials((request.auth, request.proxy_auth))
 
         code = response.code
-        if request.follow_redirections is False \
-                and code in (301, 302, 303, 307):
+        if (request.follow_redirections is False
+            and code in (301, 302, 303, 307)):
             raise errors.RedirectRequested(request.get_full_url(),
                                            request.redirected_to,
                                            is_permanent=(code == 301))

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2009-11-26 14:39:31 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2009-12-03 17:11:57 +0000
@@ -71,6 +71,7 @@
     trace,
     transport,
     ui,
+    urlutils,
     )
 
 
@@ -1066,6 +1067,14 @@
 
         auth = self.get_auth(request)
         auth['modified'] = False
+        # Put some common info in auth if the caller didn't
+        if auth.get('path', None) is None:
+            (protocol, _, _,
+             host, port, path) = urlutils.parse_url(request.get_full_url())
+            self.update_auth(auth, 'protocol', protocol)
+            self.update_auth(auth, 'host', host)
+            self.update_auth(auth, 'port', port)
+            self.update_auth(auth, 'path', path)
         # FIXME: the auth handler should be selected at a single place instead
         # of letting all handlers try to match all headers, but the current
         # design doesn't allow a simple implementation.



More information about the bazaar-commits mailing list