I added some extra coverage for basic auth in the tests and I notice that in buildbots, some of them are throwing "error: [Errno 32] Broken pipe" error. I am looking into this and will fix this. Thanks, Senthil On Sat, Aug 16, 2014 at 2:19 PM, senthil.kumaran <python-checkins@python.org
wrote:
http://hg.python.org/cpython/rev/e0510a3bdf8f changeset: 92111:e0510a3bdf8f branch: 2.7 parent: 92097:6d41f139709b user: Senthil Kumaran <senthil@uthcode.com> date: Sat Aug 16 14:16:14 2014 +0530 summary: Fix Issue #8797: Raise HTTPError on failed Basic Authentication immediately. Initial patch by Sam Bull.
files: Lib/test/test_urllib2_localnet.py | 86 ++++++++++++++++++- Lib/urllib2.py | 19 +--- Misc/NEWS | 3 + 3 files changed, 90 insertions(+), 18 deletions(-)
diff --git a/Lib/test/test_urllib2_localnet.py b/Lib/test/test_urllib2_localnet.py --- a/Lib/test/test_urllib2_localnet.py +++ b/Lib/test/test_urllib2_localnet.py @@ -1,6 +1,8 @@ +import base64 import urlparse import urllib2 import BaseHTTPServer +import SimpleHTTPServer import unittest import hashlib
@@ -66,6 +68,48 @@
# Authentication infrastructure
+ +class BasicAuthHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): + """Handler for performing Basic Authentication.""" + # Server side values + USER = "testUser" + PASSWD = "testPass" + REALM = "Test" + USER_PASSWD = "%s:%s" % (USER, PASSWD) + ENCODED_AUTH = base64.b64encode(USER_PASSWD) + + def __init__(self, *args, **kwargs): + SimpleHTTPServer.SimpleHTTPRequestHandler.__init__(self, *args, + **kwargs) + + def log_message(self, format, *args): + # Supress the HTTP Console log output + pass + + def do_HEAD(self): + self.send_response(200) + self.send_header("Content-type", "text/html") + self.end_headers() + + def do_AUTHHEAD(self): + self.send_response(401) + self.send_header("WWW-Authenticate", "Basic realm=\"%s\"" % self.REALM) + self.send_header("Content-type", "text/html") + self.end_headers() + + def do_GET(self): + if self.headers.getheader("Authorization") == None: + self.do_AUTHHEAD() + self.wfile.write("No Auth Header Received") + elif self.headers.getheader( + "Authorization") == "Basic " + self.ENCODED_AUTH: + SimpleHTTPServer.SimpleHTTPRequestHandler.do_GET(self) + else: + self.do_AUTHHEAD() + self.wfile.write(self.headers.getheader("Authorization")) + self.wfile.write("Not Authenticated") + + class DigestAuthHandler: """Handler for performing digest authentication."""
@@ -228,6 +272,45 @@ test_support.threading_cleanup(*self._threads)
+class BasicAuthTests(BaseTestCase): + USER = "testUser" + PASSWD = "testPass" + INCORRECT_PASSWD = "Incorrect" + REALM = "Test" + + def setUp(self): + super(BasicAuthTests, self).setUp() + # With Basic Authentication + def http_server_with_basic_auth_handler(*args, **kwargs): + return BasicAuthHandler(*args, **kwargs) + self.server = LoopbackHttpServerThread(http_server_with_basic_auth_handler) + self.server_url = 'http://127.0.0.1:%s' % self.server.port + self.server.start() + self.server.ready.wait() + + def tearDown(self): + self.server.stop() + super(BasicAuthTests, self).tearDown() + + def test_basic_auth_success(self): + ah = urllib2.HTTPBasicAuthHandler() + ah.add_password(self.REALM, self.server_url, self.USER, self.PASSWD) + urllib2.install_opener(urllib2.build_opener(ah)) + try: + self.assertTrue(urllib2.urlopen(self.server_url)) + except urllib2.HTTPError: + self.fail("Basic Auth Failed for url: %s" % self.server_url) + except Exception as e: + raise e + + def test_basic_auth_httperror(self): + ah = urllib2.HTTPBasicAuthHandler() + ah.add_password(self.REALM, self.server_url, self.USER, + self.INCORRECT_PASSWD) + urllib2.install_opener(urllib2.build_opener(ah)) + self.assertRaises(urllib2.HTTPError, urllib2.urlopen, self.server_url) + + class ProxyAuthTests(BaseTestCase): URL = "http://localhost"
@@ -240,6 +323,7 @@ self.digest_auth_handler = DigestAuthHandler() self.digest_auth_handler.set_users({self.USER: self.PASSWD}) self.digest_auth_handler.set_realm(self.REALM) + # With Digest Authentication def create_fake_proxy_handler(*args, **kwargs): return FakeProxyHandler(self.digest_auth_handler, *args, **kwargs)
@@ -544,7 +628,7 @@ # the next line. #test_support.requires("network")
- test_support.run_unittest(ProxyAuthTests, TestUrlopen) + test_support.run_unittest(BasicAuthTests, ProxyAuthTests, TestUrlopen)
if __name__ == "__main__": test_main() diff --git a/Lib/urllib2.py b/Lib/urllib2.py --- a/Lib/urllib2.py +++ b/Lib/urllib2.py @@ -843,10 +843,7 @@ password_mgr = HTTPPasswordMgr() self.passwd = password_mgr self.add_password = self.passwd.add_password - self.retried = 0
- def reset_retry_count(self): - self.retried = 0
def http_error_auth_reqed(self, authreq, host, req, headers): # host may be an authority (without userinfo) or a URL with an @@ -854,13 +851,6 @@ # XXX could be multiple headers authreq = headers.get(authreq, None)
- if self.retried > 5: - # retry sending the username:password 5 times before failing. - raise HTTPError(req.get_full_url(), 401, "basic auth failed", - headers, None) - else: - self.retried += 1 - if authreq: mo = AbstractBasicAuthHandler.rx.search(authreq) if mo: @@ -869,17 +859,14 @@ warnings.warn("Basic Auth Realm was unquoted", UserWarning, 2) if scheme.lower() == 'basic': - response = self.retry_http_basic_auth(host, req, realm) - if response and response.code != 401: - self.retried = 0 - return response + return self.retry_http_basic_auth(host, req, realm)
def retry_http_basic_auth(self, host, req, realm): user, pw = self.passwd.find_user_password(realm, host) if pw is not None: raw = "%s:%s" % (user, pw) auth = 'Basic %s' % base64.b64encode(raw).strip() - if req.headers.get(self.auth_header, None) == auth: + if req.get_header(self.auth_header, None) == auth: return None req.add_unredirected_header(self.auth_header, auth) return self.parent.open(req, timeout=req.timeout) @@ -895,7 +882,6 @@ url = req.get_full_url() response = self.http_error_auth_reqed('www-authenticate', url, req, headers) - self.reset_retry_count() return response
@@ -911,7 +897,6 @@ authority = req.get_host() response = self.http_error_auth_reqed('proxy-authenticate', authority, req, headers) - self.reset_retry_count() return response
diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -19,6 +19,9 @@ Library -------
+- Issue #8797: Raise HTTPError on failed Basic Authentication immediately. + Initial patch by Sam Bull. + - Issue #21448: Changed FeedParser feed() to avoid O(N**2) behavior when parsing long line. Original patch by Raymond Hettinger.
-- Repository URL: http://hg.python.org/cpython
_______________________________________________ Python-checkins mailing list Python-checkins@python.org https://mail.python.org/mailman/listinfo/python-checkins