Patch #1470846: fix urllib2 ProxyBasicAuthHandler.

Doc/lib/liburllib2.tex

@@ -621,14 +621,20 @@ user/password.
 \subsection{AbstractBasicAuthHandler Objects
             \label{abstract-basic-auth-handler}}
 
-\begin{methoddesc}[AbstractBasicAuthHandler]{handle_authentication_request}
+\begin{methoddesc}[AbstractBasicAuthHandler]{http_error_auth_reqed}
                                             {authreq, host, req, headers}
 Handle an authentication request by getting a user/password pair, and
 re-trying the request.  \var{authreq} should be the name of the header
 where the information about the realm is included in the request,
-\var{host} is the host to authenticate to, \var{req} should be the
-(failed) \class{Request} object, and \var{headers} should be the error
-headers.
+\var{host} specifies the URL and path to authenticate for, \var{req}
+should be the (failed) \class{Request} object, and \var{headers}
+should be the error headers.
+
+\var{host} is either an authority (e.g. \code{"python.org"}) or a URL
+containing an authority component (e.g. \code{"http://python.org/"}).
+In either case, the authority must not contain a userinfo component
+(so, \code{"python.org"} and \code{"python.org:80"} are fine,
+\code{"joe:password@python.org"} is not).
 \end{methoddesc}
 
 
@@ -653,7 +659,7 @@ Retry the request with authentication information, if available.
 \subsection{AbstractDigestAuthHandler Objects
             \label{abstract-digest-auth-handler}}
 
-\begin{methoddesc}[AbstractDigestAuthHandler]{handle_authentication_request}
+\begin{methoddesc}[AbstractDigestAuthHandler]{http_error_auth_reqed}
                                             {authreq, host, req, headers}
 \var{authreq} should be the name of the header where the information about
 the realm is included in the request, \var{host} should be the host to

Lib/test/test_urllib2net.py

@@ -23,6 +23,46 @@ class URLTimeoutTest(unittest.TestCase):
         f = urllib2.urlopen("http://www.python.org/")
         x = f.read()
 
+
+class AuthTests(unittest.TestCase):
+    """Tests urllib2 authentication features."""
+
+## Disabled at the moment since there is no page under python.org which
+## could be used to HTTP authentication.
+#
+#    def test_basic_auth(self):
+#        import httplib
+#
+#        test_url = "http://www.python.org/test/test_urllib2/basic_auth"
+#        test_hostport = "www.python.org"
+#        test_realm = 'Test Realm'
+#        test_user = 'test.test_urllib2net'
+#        test_password = 'blah'
+#
+#        # failure
+#        try:
+#            urllib2.urlopen(test_url)
+#        except urllib2.HTTPError, exc:
+#            self.assertEqual(exc.code, 401)
+#        else:
+#            self.fail("urlopen() should have failed with 401")
+#
+#        # success
+#        auth_handler = urllib2.HTTPBasicAuthHandler()
+#        auth_handler.add_password(test_realm, test_hostport,
+#                                  test_user, test_password)
+#        opener = urllib2.build_opener(auth_handler)
+#        f = opener.open('http://localhost/')
+#        response = urllib2.urlopen("http://www.python.org/")
+#
+#        # The 'userinfo' URL component is deprecated by RFC 3986 for security
+#        # reasons, let's not implement it!  (it's already implemented for proxy
+#        # specification strings (that is, URLs or authorities specifying a
+#        # proxy), so we must keep that)
+#        self.assertRaises(httplib.InvalidURL,
+#                          urllib2.urlopen, "http://evil:thing@example.com")
+
+
 class urlopenNetworkTests(unittest.TestCase):
     """Tests urllib2.urlopen using the network.
 
@@ -86,7 +126,8 @@ class urlopenNetworkTests(unittest.TestCase):
 
 def test_main():
     test_support.requires("network")
-    test_support.run_unittest(URLTimeoutTest, urlopenNetworkTests)
+    test_support.run_unittest(URLTimeoutTest, urlopenNetworkTests,
+                              AuthTests)
 
 if __name__ == "__main__":
     test_main()

Lib/urllib2.py

@@ -612,7 +612,6 @@ def _parse_proxy(proxy):
     ('http', 'joe', 'password', 'proxy.example.com')
 
     """
-    from urlparse import _splitnetloc
     scheme, r_scheme = splittype(proxy)
     if not r_scheme.startswith("/"):
         # authority
@@ -673,6 +672,7 @@ class ProxyHandler(BaseHandler):
             return self.parent.open(req)
 
 class HTTPPasswordMgr:
+
     def __init__(self):
         self.passwd = {}
 
@@ -696,10 +696,15 @@ class HTTPPasswordMgr:
 
     def reduce_uri(self, uri):
         """Accept netloc or URI and extract only the netloc and path"""
-        parts = urlparse.urlparse(uri)
+        parts = urlparse.urlsplit(uri)
         if parts[1]:
+            # URI
             return parts[1], parts[2] or '/'
+        elif parts[0]:
+            # host:port
+            return uri, '/'
         else:
+            # host
             return parts[2], '/'
 
     def is_suburi(self, base, test):
@@ -742,6 +747,8 @@ class AbstractBasicAuthHandler:
         self.add_password = self.passwd.add_password
 
     def http_error_auth_reqed(self, authreq, host, req, headers):
+        # host may be an authority (without userinfo) or a URL with an
+        # authority
         # XXX could be multiple headers
         authreq = headers.get(authreq, None)
         if authreq:
@@ -752,10 +759,7 @@ class AbstractBasicAuthHandler:
                     return self.retry_http_basic_auth(host, req, realm)
 
     def retry_http_basic_auth(self, host, req, realm):
-        # TODO(jhylton): Remove the host argument? It depends on whether
-        # retry_http_basic_auth() is consider part of the public API.
-        # It probably is.
-        user, pw = self.passwd.find_user_password(realm, req.get_full_url())
+        user, pw = self.passwd.find_user_password(realm, host)
         if pw is not None:
             raw = "%s:%s" % (user, pw)
             auth = 'Basic %s' % base64.encodestring(raw).strip()
@@ -766,14 +770,15 @@ class AbstractBasicAuthHandler:
         else:
             return None
 
+
 class HTTPBasicAuthHandler(AbstractBasicAuthHandler, BaseHandler):
 
     auth_header = 'Authorization'
 
     def http_error_401(self, req, fp, code, msg, headers):
-        host = urlparse.urlparse(req.get_full_url())[1]
+        url = req.get_full_url()
         return self.http_error_auth_reqed('www-authenticate',
-                                          host, req, headers)
+                                          url, req, headers)
 
 
 class ProxyBasicAuthHandler(AbstractBasicAuthHandler, BaseHandler):
@@ -781,9 +786,13 @@ class ProxyBasicAuthHandler(AbstractBasicAuthHandler, BaseHandler):
     auth_header = 'Proxy-authorization'
 
     def http_error_407(self, req, fp, code, msg, headers):
-        host = req.get_host()
+        # http_error_auth_reqed requires that there is no userinfo component in
+        # authority.  Assume there isn't one, since urllib2 does not (and
+        # should not, RFC 3986 s. 3.2.1) support requests for URLs containing
+        # userinfo.
+        authority = req.get_host()
         return self.http_error_auth_reqed('proxy-authenticate',
-                                          host, req, headers)
+                                          authority, req, headers)
 
 
 def randombytes(n):

Misc/NEWS

@@ -86,6 +86,8 @@ Extension Modules
 Library
 -------
 
+- Patch #1470846: fix urllib2 ProxyBasicAuthHandler.
+
 - Patch #1475231: ``doctest`` has a new ``SKIP`` option, which causes
   a doctest to be skipped (the code is not run, and the expected output
   or exception is ignored).