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