Issue #20059: urllib.parse raises ValueError on all invalid ports.

Patch by Martin Panter.

diff --git a/Doc/library/urllib.parse.rst b/Doc/library/urllib.parse.rst
index 40098d04966f373cce37383cf2d7c6da97f6d112..7c075adf2afab361a393bbf04db94a64c21afc71 100644 (file)
--- a/Doc/library/urllib.parse.rst
+++ b/Doc/library/urllib.parse.rst
@@ -115,8 +115,9 @@ or on combining URL components into a URL string.
    |                  |       | if present               |                      |
    +------------------+-------+--------------------------+----------------------+
 
-   See section :ref:`urlparse-result-object` for more information on the result
-   object.
+   Reading the :attr:`port` attribute will raise a :exc:`ValueError` if
+   an invalid port is specified in the URL.  See section
+   :ref:`urlparse-result-object` for more information on the result object.
 
    .. versionchanged:: 3.2
       Added IPv6 URL parsing capabilities.
@@ -126,6 +127,10 @@ or on combining URL components into a URL string.
       false), in accordance with :rfc:`3986`.  Previously, a whitelist of
       schemes that support fragments existed.
 
+   .. versionchanged:: 3.6
+      Out-of-range port numbers now raise :exc:`ValueError`, instead of
+      returning :const:`None`.
+
 
 .. function:: parse_qs(qs, keep_blank_values=False, strict_parsing=False, encoding='utf-8', errors='replace')
 
@@ -228,8 +233,13 @@ or on combining URL components into a URL string.
    |                  |       | if present              |                      |
    +------------------+-------+-------------------------+----------------------+
 
-   See section :ref:`urlparse-result-object` for more information on the result
-   object.
+   Reading the :attr:`port` attribute will raise a :exc:`ValueError` if
+   an invalid port is specified in the URL.  See section
+   :ref:`urlparse-result-object` for more information on the result object.
+
+   .. versionchanged:: 3.6
+      Out-of-range port numbers now raise :exc:`ValueError`, instead of
+      returning :const:`None`.
 
 
 .. function:: urlunsplit(parts)

diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
index 4e686ad0b4dc02f950d35ab2103318fcbcb5ca39..e2aa1e2f02c5d67012cd490d0997ed5dc9b09c1a 100644 (file)
--- a/Doc/whatsnew/3.6.rst
+++ b/Doc/whatsnew/3.6.rst
@@ -162,7 +162,10 @@ that may require changes to your code.
 Changes in the Python API
 -------------------------
 
-* None yet.
+* Reading the :attr:`~urllib.parse.SplitResult.port` attribute of
+  :func:`urllib.parse.urlsplit` and :func:`~urllib.parse.urlparse` results
+  now raises :exc:`ValueError` for out-of-range values, rather than
+  returning :const:`None`.  See :issue:`20059`.
 
 
 Changes in the C API

diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py
index 0552f90594a3c3b29cc8e51a969398cc0e795293..fcf508259e743b9e9610b3379cc396ae540f69b3 100644 (file)
--- a/Lib/test/test_urlparse.py
+++ b/Lib/test/test_urlparse.py
@@ -554,29 +554,27 @@ class UrlParseTestCase(unittest.TestCase):
         self.assertEqual(p.port, 80)
         self.assertEqual(p.geturl(), url)
 
-        # Verify an illegal port is returned as None
+        # Verify an illegal port raises ValueError
         url = b"HTTP://WWW.PYTHON.ORG:65536/doc/#frag"
         p = urllib.parse.urlsplit(url)
-        self.assertEqual(p.port, None)
+        with self.assertRaisesRegex(ValueError, "out of range"):
+            p.port
 
     def test_attributes_bad_port(self):
-        """Check handling of non-integer ports."""
-        p = urllib.parse.urlsplit("http://www.example.net:foo")
-        self.assertEqual(p.netloc, "www.example.net:foo")
-        self.assertRaises(ValueError, lambda: p.port)
-
-        p = urllib.parse.urlparse("http://www.example.net:foo")
-        self.assertEqual(p.netloc, "www.example.net:foo")
-        self.assertRaises(ValueError, lambda: p.port)
-
-        # Once again, repeat ourselves to test bytes
-        p = urllib.parse.urlsplit(b"http://www.example.net:foo")
-        self.assertEqual(p.netloc, b"www.example.net:foo")
-        self.assertRaises(ValueError, lambda: p.port)
-
-        p = urllib.parse.urlparse(b"http://www.example.net:foo")
-        self.assertEqual(p.netloc, b"www.example.net:foo")
-        self.assertRaises(ValueError, lambda: p.port)
+        """Check handling of invalid ports."""
+        for bytes in (False, True):
+            for parse in (urllib.parse.urlsplit, urllib.parse.urlparse):
+                for port in ("foo", "1.5", "-1", "0x10"):
+                    with self.subTest(bytes=bytes, parse=parse, port=port):
+                        netloc = "www.example.net:" + port
+                        url = "http://" + netloc
+                        if bytes:
+                            netloc = netloc.encode("ascii")
+                            url = url.encode("ascii")
+                        p = parse(url)
+                        self.assertEqual(p.netloc, netloc)
+                        with self.assertRaises(ValueError):
+                            p.port
 
     def test_attributes_without_netloc(self):
         # This example is straight from RFC 3261.  It looks like it

diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py
index 01c9e587fbcaeee2334c11e44a47889f8c161102..5e2155ccafc595e57243a3c73d8741c9f2006a7c 100644 (file)
--- a/Lib/urllib/parse.py
+++ b/Lib/urllib/parse.py
@@ -156,9 +156,8 @@ class _NetlocResultMixinBase(object):
         port = self._hostinfo[1]
         if port is not None:
             port = int(port, 10)
-            # Return None on an illegal port
             if not ( 0 <= port <= 65535):
-                return None
+                raise ValueError("Port out of range 0-65535")
         return port
 
 

diff --git a/Misc/NEWS b/Misc/NEWS
index 8ec1f01462b780e3787270414c9a7a3e8dfc50a1..1aa6b09b7658c408879d57b5bd5e2cb78e9e74d8 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -15,6 +15,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #20059: urllib.parse raises ValueError on all invalid ports.
+  Patch by Martin Panter.
+
 - Issue #24824: Signatures of codecs.encode() and codecs.decode() now are
   compatible with pydoc.