Rev 4866: (vila) Start fixing bug #395714 when authentication is required after in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Fri Dec 4 17:06:23 GMT 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4866 [merge]
revision-id: pqm at pqm.ubuntu.com-20091204170617-8llyv1nv7g9cmiv7
parent: pqm at pqm.ubuntu.com-20091204153754-fxhnwlf52kbxxczk
parent: v.ladeuil+lp at free.fr-20091204152223-04genfpg607iayix
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2009-12-04 17:06:17 +0000
message:
(vila) Start fixing bug #395714 when authentication is required after
an http redirection
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/tests/test_http.py testhttp.py-20051018020158-b2eef6e867c514d9
bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
=== modified file 'NEWS'
--- a/NEWS 2009-12-04 10:16:00 +0000
+++ b/NEWS 2009-12-04 17:06:17 +0000
@@ -47,6 +47,9 @@
* ``bzr serve`` is more clear about the risk of supplying --allow-writes.
(Robert Collins, #84659)
+* Fix bug with redirected URLs over authenticated HTTP.
+ (Glen Mailer, Neil Martinsen-Burrell, Vincent Ladeuil, #395714)
+
* ``bzr serve --quiet`` really is quiet now. (Gordon Tyler, #252834)
* Lots of bugfixes for the test suite on Windows. We should once again
@@ -210,7 +213,6 @@
``repo.add_revision`` ignores the supplied inventory sha1 and recomputes
the sha1 from the repo directly. (John Arbash Meinel)
-
* Shelve command refuse to run if there is no real terminal.
(Alexander Belchenko)
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py 2009-11-18 16:02:53 +0000
+++ b/bzrlib/tests/test_http.py 2009-12-04 15:22:23 +0000
@@ -269,19 +269,19 @@
def test_empty_header(self):
scheme, remainder = self.parse_header('')
- self.assertEquals('', scheme)
+ self.assertEqual('', scheme)
self.assertIs(None, remainder)
def test_negotiate_header(self):
scheme, remainder = self.parse_header('Negotiate')
- self.assertEquals('negotiate', scheme)
+ self.assertEqual('negotiate', scheme)
self.assertIs(None, remainder)
def test_basic_header(self):
scheme, remainder = self.parse_header(
'Basic realm="Thou should not pass"')
- self.assertEquals('basic', scheme)
- self.assertEquals('realm="Thou should not pass"', remainder)
+ self.assertEqual('basic', scheme)
+ self.assertEqual('realm="Thou should not pass"', remainder)
def test_basic_extract_realm(self):
scheme, remainder = self.parse_header(
@@ -289,13 +289,13 @@
_urllib2_wrappers.BasicAuthHandler)
match, realm = self.auth_handler.extract_realm(remainder)
self.assertTrue(match is not None)
- self.assertEquals('Thou should not pass', realm)
+ self.assertEqual('Thou should not pass', realm)
def test_digest_header(self):
scheme, remainder = self.parse_header(
'Digest realm="Thou should not pass"')
- self.assertEquals('digest', scheme)
- self.assertEquals('realm="Thou should not pass"', remainder)
+ self.assertEqual('digest', scheme)
+ self.assertEqual('realm="Thou should not pass"', remainder)
class TestHTTPServer(tests.TestCase):
@@ -393,14 +393,14 @@
def test_url_parsing(self):
f = FakeManager()
url = http.extract_auth('http://example.com', f)
- self.assertEquals('http://example.com', url)
- self.assertEquals(0, len(f.credentials))
+ self.assertEqual('http://example.com', url)
+ self.assertEqual(0, len(f.credentials))
url = http.extract_auth(
'http://user:pass@www.bazaar-vcs.org/bzr/bzr.dev', f)
- self.assertEquals('http://www.bazaar-vcs.org/bzr/bzr.dev', url)
- self.assertEquals(1, len(f.credentials))
- self.assertEquals([None, 'www.bazaar-vcs.org', 'user', 'pass'],
- f.credentials[0])
+ self.assertEqual('http://www.bazaar-vcs.org/bzr/bzr.dev', url)
+ self.assertEqual(1, len(f.credentials))
+ self.assertEqual([None, 'www.bazaar-vcs.org', 'user', 'pass'],
+ f.credentials[0])
class TestHttpTransportUrls(tests.TestCase):
@@ -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,18 +1418,16 @@
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 = \
[('(.*)', r'%s/1\1' % (new_prefix), 301),]
- self.assertEquals('redirected once',t._perform(req).read())
+ self.assertEqual('redirected once',t._perform(req).read())
def test_five_redirections(self):
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,
@@ -1441,7 +1439,7 @@
('/4(.*)', r'%s/5\1' % (new_prefix), 301),
('(/[^/]+)', r'%s/1\1' % (old_prefix), 301),
]
- self.assertEquals('redirected 5 times',t._perform(req).read())
+ self.assertEqual('redirected 5 times',t._perform(req).read())
class TestDoCatchRedirections(http_utils.TestCaseWithRedirectedWebserver):
@@ -1460,8 +1458,8 @@
t = self._transport(self.new_server.get_url())
# We use None for redirected so that we fail if redirected
- self.assertEquals('0123456789',
- transport.do_catching_redirections(
+ self.assertEqual('0123456789',
+ transport.do_catching_redirections(
self.get_a, t, None).read())
def test_one_redirection(self):
@@ -1472,10 +1470,10 @@
dir, file = urlutils.split(exception.target)
return self._transport(dir)
- self.assertEquals('0123456789',
- transport.do_catching_redirections(
+ self.assertEqual('0123456789',
+ transport.do_catching_redirections(
self.get_a, self.old_transport, redirected).read())
- self.assertEquals(1, self.redirections)
+ self.assertEqual(1, self.redirections)
def test_redirection_loop(self):
@@ -1580,8 +1578,8 @@
self.assertEqual('', ui.ui_factory.stdin.readline())
stderr.seek(0)
expected_prompt = self._expected_username_prompt(t._unqualified_scheme)
- self.assertEquals(expected_prompt, stderr.read(len(expected_prompt)))
- self.assertEquals('', stdout.getvalue())
+ self.assertEqual(expected_prompt, stderr.read(len(expected_prompt)))
+ self.assertEqual('', stdout.getvalue())
self._check_password_prompt(t._unqualified_scheme, 'joe',
stderr.readline())
@@ -1602,7 +1600,7 @@
self.assertEqual('', ui.ui_factory.stdin.readline())
self._check_password_prompt(t._unqualified_scheme, 'joe',
stderr.getvalue())
- self.assertEquals('', stdout.getvalue())
+ self.assertEqual('', stdout.getvalue())
# And we shouldn't prompt again for a different request
# against the same transport.
self.assertEqual('contents of b\n',t.get('b').read())
@@ -1618,7 +1616,7 @@
% (scheme.upper(),
user, self.server.host, self.server.port,
self.server.auth_realm)))
- self.assertEquals(expected_prompt, actual_prompt)
+ self.assertEqual(expected_prompt, actual_prompt)
def _expected_username_prompt(self, scheme):
return (self._username_prompt_prefix
@@ -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(
@@ -1856,7 +1854,7 @@
'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())
+ self.assertEqual(t._get_connection(), r._get_connection())
def test_redirected_to_self_with_slash(self):
t = self._transport('http://www.example.com/foo')
@@ -1866,7 +1864,7 @@
# 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())
+ self.assertEqual(t._get_connection(), r._get_connection())
def test_redirected_to_host(self):
t = self._transport('http://www.example.com/foo')
@@ -1891,7 +1889,7 @@
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)
+ self.assertEqual(t._user, r._user)
class PredefinedRequestHandler(http_server.TestingHTTPRequestHandler):
@@ -2154,3 +2152,81 @@
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.assertEqual('redirected once',
+ transport.do_catching_redirections(
+ self.get_a, self.old_transport, redirected).read())
+ self.assertEqual(1, self.redirections)
+ # stdin should be empty
+ self.assertEqual('', ui.ui_factory.stdin.readline())
+ # stdout should be empty, stderr will contains the prompts
+ self.assertEqual('', 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.assertEqual('redirected once',t._perform(req).read())
+ # stdin should be empty
+ self.assertEqual('', ui.ui_factory.stdin.readline())
+ # stdout should be empty, stderr will contains the prompts
+ self.assertEqual('', stdout.getvalue())
+
+
=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py 2009-08-19 16:33:39 +0000
+++ b/bzrlib/transport/http/__init__.py 2009-11-26 14:39:31 +0000
@@ -154,7 +154,7 @@
None, None, self._host, self._port, path)
def _create_auth(self):
- """Returns a dict returning the credentials provided at build time."""
+ """Returns a dict containing the credentials provided at build time."""
auth = dict(host=self._host, port=self._port,
user=self._user, password=self._password,
protocol=self._unqualified_scheme,
=== 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-03 13:46:16 +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.
@@ -1167,8 +1176,8 @@
and then during dialog with the server).
"""
auth_conf = config.AuthenticationConfig()
- user = auth['user']
- password = auth['password']
+ user = auth.get('user', None)
+ password = auth.get('password', None)
realm = auth['realm']
if user is None:
@@ -1308,7 +1317,8 @@
# Put useful info into auth
self.update_auth(auth, 'scheme', scheme)
self.update_auth(auth, 'realm', realm)
- if auth['user'] is None or auth['password'] is None:
+ if (auth.get('user', None) is None
+ or auth.get('password', None) is None):
user, password = self.get_user_password(auth)
self.update_auth(auth, 'user', user)
self.update_auth(auth, 'password', password)
@@ -1373,7 +1383,7 @@
# Put useful info into auth
self.update_auth(auth, 'scheme', scheme)
self.update_auth(auth, 'realm', realm)
- if auth['user'] is None or auth['password'] is None:
+ if auth.get('user', None) is None or auth.get('password', None) is None:
user, password = self.get_user_password(auth)
self.update_auth(auth, 'user', user)
self.update_auth(auth, 'password', password)
More information about the bazaar-commits
mailing list