From 261e251df8ed5b588ecb2b9f722596c085c8c244 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 29 May 2006 20:52:54 +0000 Subject: [PATCH] Patches #1497027 and #972322: try HTTP digest auth first, and watch out for handler name collisions. --- Doc/lib/liburllib2.tex | 4 +-- Lib/test/test_urllib2.py | 61 ++++++++++++++++++++++++++++++++++++---- Lib/urllib2.py | 10 +++++++ 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/Doc/lib/liburllib2.tex b/Doc/lib/liburllib2.tex index 7c8ad5dce2..f77ed25353 100644 --- a/Doc/lib/liburllib2.tex +++ b/Doc/lib/liburllib2.tex @@ -65,9 +65,7 @@ exists), \class{HTTPSHandler} will also be added. Beginning in Python 2.3, a \class{BaseHandler} subclass may also change its \member{handler_order} member variable to modify its -position in the handlers list. Besides \class{ProxyHandler}, which has -\member{handler_order} of \code{100}, all handlers currently have it -set to \code{500}. +position in the handlers list. \end{funcdesc} diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 32cc6128e9..034b9d0316 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -318,6 +318,27 @@ class MockPasswordManager: class OpenerDirectorTests(unittest.TestCase): + def test_badly_named_methods(self): + # test work-around for three methods that accidentally follow the + # naming conventions for handler methods + # (*_open() / *_request() / *_response()) + + # These used to call the accidentally-named methods, causing a + # TypeError in real code; here, returning self from these mock + # methods would either cause no exception, or AttributeError. + + from urllib2 import URLError + + o = OpenerDirector() + meth_spec = [ + [("do_open", "return self"), ("proxy_open", "return self")], + [("redirect_request", "return self")], + ] + handlers = add_ordered_mock_handlers(o, meth_spec) + o.add_handler(urllib2.UnknownHandler()) + for scheme in "do", "proxy", "redirect": + self.assertRaises(URLError, o.open, scheme+"://example.com/") + def test_handled(self): # handler returning non-None means no more handlers will be called o = OpenerDirector() @@ -807,6 +828,8 @@ class HandlerTests(unittest.TestCase): realm = "ACME Widget Store" http_handler = MockHTTPHandler( 401, 'WWW-Authenticate: Basic realm="%s"\r\n\r\n' % realm) + opener.add_handler(auth_handler) + opener.add_handler(http_handler) self._test_basic_auth(opener, auth_handler, "Authorization", realm, http_handler, password_manager, "http://acme.example.com/protected", @@ -822,6 +845,8 @@ class HandlerTests(unittest.TestCase): realm = "ACME Networks" http_handler = MockHTTPHandler( 407, 'Proxy-Authenticate: Basic realm="%s"\r\n\r\n' % realm) + opener.add_handler(auth_handler) + opener.add_handler(http_handler) self._test_basic_auth(opener, auth_handler, "Proxy-authorization", realm, http_handler, password_manager, "http://acme.example.com:3128/protected", @@ -833,29 +858,53 @@ class HandlerTests(unittest.TestCase): # response (http://python.org/sf/1479302), where it should instead # return None to allow another handler (especially # HTTPBasicAuthHandler) to handle the response. + + # Also (http://python.org/sf/14797027, RFC 2617 section 1.2), we must + # try digest first (since it's the strongest auth scheme), so we record + # order of calls here to check digest comes first: + class RecordingOpenerDirector(OpenerDirector): + def __init__(self): + OpenerDirector.__init__(self) + self.recorded = [] + def record(self, info): + self.recorded.append(info) class TestDigestAuthHandler(urllib2.HTTPDigestAuthHandler): - handler_order = 400 # strictly before HTTPBasicAuthHandler - opener = OpenerDirector() + def http_error_401(self, *args, **kwds): + self.parent.record("digest") + urllib2.HTTPDigestAuthHandler.http_error_401(self, + *args, **kwds) + class TestBasicAuthHandler(urllib2.HTTPBasicAuthHandler): + def http_error_401(self, *args, **kwds): + self.parent.record("basic") + urllib2.HTTPBasicAuthHandler.http_error_401(self, + *args, **kwds) + + opener = RecordingOpenerDirector() password_manager = MockPasswordManager() digest_handler = TestDigestAuthHandler(password_manager) - basic_handler = urllib2.HTTPBasicAuthHandler(password_manager) - opener.add_handler(digest_handler) + basic_handler = TestBasicAuthHandler(password_manager) realm = "ACME Networks" http_handler = MockHTTPHandler( 401, 'WWW-Authenticate: Basic realm="%s"\r\n\r\n' % realm) + opener.add_handler(basic_handler) + opener.add_handler(digest_handler) + opener.add_handler(http_handler) + + # check basic auth isn't blocked by digest handler failing self._test_basic_auth(opener, basic_handler, "Authorization", realm, http_handler, password_manager, "http://acme.example.com/protected", "http://acme.example.com/protected", ) + # check digest was tried before basic (twice, because + # _test_basic_auth called .open() twice) + self.assertEqual(opener.recorded, ["digest", "basic"]*2) def _test_basic_auth(self, opener, auth_handler, auth_header, realm, http_handler, password_manager, request_url, protected_url): import base64, httplib user, password = "wile", "coyote" - opener.add_handler(auth_handler) - opener.add_handler(http_handler) # .add_password() fed through to password manager auth_handler.add_password(realm, request_url, user, password) diff --git a/Lib/urllib2.py b/Lib/urllib2.py index b2ff04fc6c..227311c312 100644 --- a/Lib/urllib2.py +++ b/Lib/urllib2.py @@ -297,6 +297,10 @@ class OpenerDirector: def add_handler(self, handler): added = False for meth in dir(handler): + if meth in ["redirect_request", "do_open", "proxy_open"]: + # oops, coincidental match + continue + i = meth.find("_") protocol = meth[:i] condition = meth[i+1:] @@ -768,6 +772,10 @@ class AbstractBasicAuthHandler: # www-authenticate header. should probably be a lot more careful # in parsing them to extract multiple alternatives + # XXX could pre-emptively send auth info already accepted (RFC 2617, + # end of section 2, and section 1.2 immediately after "credentials" + # production). + def __init__(self, password_mgr=None): if password_mgr is None: password_mgr = HTTPPasswordMgr() @@ -977,6 +985,7 @@ class HTTPDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler): """ auth_header = 'Authorization' + handler_order = 490 # before Basic auth def http_error_401(self, req, fp, code, msg, headers): host = urlparse.urlparse(req.get_full_url())[1] @@ -989,6 +998,7 @@ class HTTPDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler): class ProxyDigestAuthHandler(BaseHandler, AbstractDigestAuthHandler): auth_header = 'Proxy-Authorization' + handler_order = 490 # before Basic auth def http_error_407(self, req, fp, code, msg, headers): host = req.get_host() -- 2.40.0