Patch #1496206: urllib2 PasswordMgr ./. default ports

3 files changed, 82 insertions, 18 deletions

diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 9203e37..32cc612 100644
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -76,10 +76,11 @@ def test_password_manager(self):
     >>> mgr.find_user_password("c", "http://example.com/bar")
     ('bar', 'nini')
 
-    Currently, we use the highest-level path where more than one match:
+    Actually, this is really undefined ATM
+##     Currently, we use the highest-level path where more than one match:
 
-    >>> mgr.find_user_password("Some Realm", "http://example.com/ni")
-    ('joe', 'password')
+##     >>> mgr.find_user_password("Some Realm", "http://example.com/ni")
+##     ('joe', 'password')
 
     Use latest add_password() in case of conflict:
 
@@ -110,6 +111,53 @@ def test_password_manager(self):
     pass
 
 
+def test_password_manager_default_port(self):
+    """
+    >>> mgr = urllib2.HTTPPasswordMgr()
+    >>> add = mgr.add_password
+
+    The point to note here is that we can't guess the default port if there's
+    no scheme.  This applies to both add_password and find_user_password.
+
+    >>> add("f", "http://g.example.com:80", "10", "j")
+    >>> add("g", "http://h.example.com", "11", "k")
+    >>> add("h", "i.example.com:80", "12", "l")
+    >>> add("i", "j.example.com", "13", "m")
+    >>> mgr.find_user_password("f", "g.example.com:100")
+    (None, None)
+    >>> mgr.find_user_password("f", "g.example.com:80")
+    ('10', 'j')
+    >>> mgr.find_user_password("f", "g.example.com")
+    (None, None)
+    >>> mgr.find_user_password("f", "http://g.example.com:100")
+    (None, None)
+    >>> mgr.find_user_password("f", "http://g.example.com:80")
+    ('10', 'j')
+    >>> mgr.find_user_password("f", "http://g.example.com")
+    ('10', 'j')
+    >>> mgr.find_user_password("g", "h.example.com")
+    ('11', 'k')
+    >>> mgr.find_user_password("g", "h.example.com:80")
+    ('11', 'k')
+    >>> mgr.find_user_password("g", "http://h.example.com:80")
+    ('11', 'k')
+    >>> mgr.find_user_password("h", "i.example.com")
+    (None, None)
+    >>> mgr.find_user_password("h", "i.example.com:80")
+    ('12', 'l')
+    >>> mgr.find_user_password("h", "http://i.example.com:80")
+    ('12', 'l')
+    >>> mgr.find_user_password("i", "j.example.com")
+    ('13', 'm')
+    >>> mgr.find_user_password("i", "j.example.com:80")
+    (None, None)
+    >>> mgr.find_user_password("i", "http://j.example.com")
+    ('13', 'm')
+    >>> mgr.find_user_password("i", "http://j.example.com:80")
+    (None, None)
+
+    """
+
 class MockOpener:
     addheaders = []
     def open(self, req, data=None):
diff --git a/Lib/urllib2.py b/Lib/urllib2.py
index cdb3a22..b2ff04f 100644
--- a/Lib/urllib2.py
+++ b/Lib/urllib2.py
@@ -695,32 +695,45 @@ class HTTPPasswordMgr:
         # uri could be a single URI or a sequence
         if isinstance(uri, basestring):
             uri = [uri]
-        uri = tuple(map(self.reduce_uri, uri))
         if not realm in self.passwd:
             self.passwd[realm] = {}
-        self.passwd[realm][uri] = (user, passwd)
+        for default_port in True, False:
+            reduced_uri = tuple(
+                [self.reduce_uri(u, default_port) for u in uri])
+            self.passwd[realm][reduced_uri] = (user, passwd)
 
     def find_user_password(self, realm, authuri):
         domains = self.passwd.get(realm, {})
-        authuri = self.reduce_uri(authuri)
-        for uris, authinfo in domains.iteritems():
-            for uri in uris:
-                if self.is_suburi(uri, authuri):
-                    return authinfo
+        for default_port in True, False:
+            reduced_authuri = self.reduce_uri(authuri, default_port)
+            for uris, authinfo in domains.iteritems():
+                for uri in uris:
+                    if self.is_suburi(uri, reduced_authuri):
+                        return authinfo
         return None, None
 
-    def reduce_uri(self, uri):
-        """Accept netloc or URI and extract only the netloc and path"""
+    def reduce_uri(self, uri, default_port=True):
+        """Accept authority or URI and extract only the authority and path."""
+        # note HTTP URLs do not have a userinfo component
         parts = urlparse.urlsplit(uri)
         if parts[1]:
             # URI
-            return parts[1], parts[2] or '/'
-        elif parts[0]:
-            # host:port
-            return uri, '/'
+            scheme = parts[0]
+            authority = parts[1]
+            path = parts[2] or '/'
         else:
-            # host
-            return parts[2], '/'
+            # host or host:port
+            scheme = None
+            authority = uri
+            path = '/'
+        host, port = splitport(authority)
+        if default_port and port is None and scheme is not None:
+            dport = {"http": 80,
+                     "https": 443,
+                     }.get(scheme)
+            if dport is not None:
+                authority = "%s:%d" % (host, dport)
+        return authority, path
 
     def is_suburi(self, base, test):
         """Check if test is below base in a URI tree
diff --git a/Misc/NEWS b/Misc/NEWS
index 31a5af9..90407d6 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -85,6 +85,9 @@ Extension Modules
 Library
 -------
 
+- Patch #1496206: improve urllib2 handling of passwords with respect to
+  default HTTP and HTTPS ports.
+
 - Patch #1080727: add "encoding" parameter to doctest.DocFileSuite.
 
 - Patch #1281707: speed up gzip.readline.