[Python-Dev] [Python-checkins] cpython (2.7): Fix Issue #8797: Raise HTTPError on failed Basic Authentication immediately.

Senthil Kumaran senthil at uthcode.com
Sat Aug 16 12:40:02 CEST 2014


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 at python.org
> wrote:

> http://hg.python.org/cpython/rev/e0510a3bdf8f
> changeset:   92111:e0510a3bdf8f
> branch:      2.7
> parent:      92097:6d41f139709b
> user:        Senthil Kumaran <senthil at 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 at python.org
> https://mail.python.org/mailman/listinfo/python-checkins
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20140816/97005506/attachment.html>


More information about the Python-Dev mailing list