https://github.com/python/cpython/commit/1a530ac0c621fef453d35eece9fc057522058dea commit: 1a530ac0c621fef453d35eece9fc057522058dea branch: 3.15 author: Miss Islington (bot) <[email protected]> committer: orsenthil <[email protected]> date: 2026-11-08T16:06:06Z summary:
[3.15] gh-88037 - Move port validation logic to parsing time (GH-159122) (#161619) gh-88037 - Move port validation logic to parsing time (GH-159122) * Move port validation logic to parsing time * Fix CI. Handle the special case of file://C:\\ parsed by url2pathname function where ":" should not be treated as port separator. --------- (cherry picked from commit 403f6dd31a70613d4c49a4c9954aacb33a0637dc) Co-authored-by: Senthil Kumaran <[email protected]> Co-authored-by: Miguel Mendes <[email protected]> files: A Misc/NEWS.d/next/Library/2026-10-11-13-08-07.gh-issue-88037.CzKcxh.rst M Lib/test/test_urllib.py M Lib/test/test_urllib2.py M Lib/test/test_urlparse.py M Lib/urllib/parse.py M Lib/urllib/request.py diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index ab59727a8fd1820..3858b03546061e6 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -7,6 +7,7 @@ import email.message import io import unittest +import unittest.mock from test import support from test.support import os_helper from test.support import socket_helper @@ -400,11 +401,17 @@ def test_url_host_with_newline_header_injection_rejected(self): host = "localhost\r\nX-injected: header\r\n" schemeless_url = "//" + host + ":8080/test/?test=a" try: - InvalidURL = http.client.InvalidURL + # Once \r\n are stripped from the URL, the ':' in + # "X-injected:" is mistaken for the port separator, and + # urlsplit()/urlparse() now reject the resulting + # non-numeric "port" at parse time, before a Request is + # even built -- rejecting the injection earlier than + # http.client's own InvalidURL check ever gets a chance to. with self.assertRaisesRegex( - InvalidURL, r"contain control.*\\r"): + ValueError, "Port could not be cast to integer value"): urllib.request.urlopen(f"http:{schemeless_url}") - with self.assertRaisesRegex(InvalidURL, r"contain control.*\\n"): + with self.assertRaisesRegex( + ValueError, "Port could not be cast to integer value"): urllib.request.urlopen(f"https:{schemeless_url}") finally: self.unfakehttp() @@ -1672,6 +1679,17 @@ def test_url2pathname_resolve_host(self): self.assertEqual(fn(f'//{socket.gethostname()}/foo/bar'), f'{sep}foo{sep}bar') self.assertEqual(fn(f'//{socket.gethostname()}/foo/bar', resolve_host=True), f'{sep}foo{sep}bar') + def test_url2pathname_win_netloc_not_validated_as_port(self): + # The Windows branch of url2pathname() stuffs things like drive + # letters into the netloc (e.g. '//C:\\' -> authority 'C:\\'), + # which is not a real host[:port] pair. url2pathname() must not + # reject it as an invalid port; run the Windows branch directly + # so this is exercised on every platform, not just win32. + with unittest.mock.patch('os.name', 'nt'): + fn = urllib.request.url2pathname + self.assertEqual(fn('//C:\\'), 'C:\\') + self.assertEqual(fn('//C:80/'), 'C:80\\') + @unittest.skipUnless(sys.platform == 'win32', 'test specific to Windows pathnames.') def test_url2pathname_win(self): diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index eba4171969ad30d..045ef52c23bf84d 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -915,9 +915,9 @@ def test_file(self): parsed._replace(netloc='localhost:80').geturl(), "file:///file_does_not_exist.txt", "file://not-a-local-host.com//dir/file.txt", - "file://%s:80%s/%s" % (socket.gethostbyname('localhost'), + "file://%s:80/%s/%s" % (socket.gethostbyname('localhost'), os.getcwd(), TESTFN), - "file://somerandomhost.ontheinternet.com%s/%s" % + "file://somerandomhost.ontheinternet.com/%s/%s" % (os.getcwd(), TESTFN), ]: try: diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py index a5b7966c7780e9e..112cc139d597b6c 100644 --- a/Lib/test/test_urlparse.py +++ b/Lib/test/test_urlparse.py @@ -917,9 +917,8 @@ def test_urlsplit_attributes(self): # Verify an illegal port raises ValueError url = b"HTTP://WWW.PYTHON.ORG:65536/doc/#frag" - p = urllib.parse.urlsplit(url) with self.assertRaisesRegex(ValueError, "out of range"): - p.port + urllib.parse.urlsplit(url) def test_urlsplit_remove_unsafe_bytes(self): # Remove ASCII tabs and newlines from input @@ -1029,10 +1028,8 @@ def test_attributes_bad_port(self, bytes, parse, port): self.skipTest('non-ASCII bytes') netloc = str_encode(netloc) url = str_encode(url) - p = parse(url) - self.assertEqual(p.netloc, netloc) with self.assertRaises(ValueError): - p.port + parse(url) @support.subTests('bytes', (False, True)) @support.subTests('parse', (urllib.parse.urlsplit, urllib.parse.urlparse)) @@ -1670,13 +1667,11 @@ def test_splitting_bracketed_hosts(self): def test_port_casting_failure_message(self): message = "Port could not be cast to integer value as 'oracle'" - p1 = urllib.parse.urlparse('http://Server=sde; Service=sde:oracle') with self.assertRaisesRegex(ValueError, message): - p1.port + urllib.parse.urlparse('http://Server=sde; Service=sde:oracle') - p2 = urllib.parse.urlsplit('http://Server=sde; Service=sde:oracle') with self.assertRaisesRegex(ValueError, message): - p2.port + urllib.parse.urlsplit('http://Server=sde; Service=sde:oracle') def test_telurl_params(self): p1 = urllib.parse.urlparse('tel:123-4;phone-context=+1-650-516') diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py index 4247b9a4b07fa3f..ee0bceb2928c06f 100644 --- a/Lib/urllib/parse.py +++ b/Lib/urllib/parse.py @@ -140,6 +140,27 @@ def _coerce_args(*args): return args + (_noop,) return _decode_args(args) + (_encode_result,) +def _parse_hostinfo(netloc): + _, _, hostinfo = netloc.rpartition('@') + _, have_open_br, bracketed = hostinfo.partition('[') + if have_open_br: + hostname, _, port = bracketed.partition(']') + _, _, port = port.partition(':') + else: + hostname, _, port = hostinfo.partition(':') + if not port: + port = None + return hostname, port + +def _validate_port(port): + if port.isdigit() and port.isascii(): + port = int(port) + else: + raise ValueError(f"Port could not be cast to integer value as {port!r}") + if not (0 <= port <= 65535): + raise ValueError("Port out of range 0-65535") + return port + # Result objects are more helpful than simple tuples class _ResultMixinStr(object): """Standard approach to encoding parsed results from str to bytes""" @@ -198,12 +219,7 @@ def hostname(self): def port(self): port = self._hostinfo[1] if port is not None: - if port.isdigit() and port.isascii(): - port = int(port) - else: - raise ValueError(f"Port could not be cast to integer value as {port!r}") - if not (0 <= port <= 65535): - raise ValueError("Port out of range 0-65535") + port = _validate_port(port) return port __class_getitem__ = classmethod(types.GenericAlias) @@ -231,16 +247,7 @@ def _hostinfo(self): netloc = self.netloc if netloc is None: return None, None - _, _, hostinfo = netloc.rpartition('@') - _, have_open_br, bracketed = hostinfo.partition('[') - if have_open_br: - hostname, _, port = bracketed.partition(']') - _, _, port = port.partition(':') - else: - hostname, _, port = hostinfo.partition(':') - if not port: - port = None - return hostname, port + return _parse_hostinfo(netloc) class _NetlocResultMixinBytes(_NetlocResultMixinBase, _ResultMixinBytes): @@ -506,9 +513,7 @@ def _splitnetloc(url, start=0): delim = min(delim, wdelim) # use earliest delim position return url[start:delim], url[delim:] # return (domain, rest) -def _checknetloc(netloc): - if not netloc or netloc.isascii(): - return +def _checknetloc_nfkc(netloc): # looking for characters like \u2100 that expand to 'a/c' # IDNA uses NFKC equivalence, so normalize for this check import unicodedata @@ -524,6 +529,16 @@ def _checknetloc(netloc): raise ValueError("netloc '" + netloc + "' contains invalid " + "characters under NFKC normalization") +def _checknetloc(netloc): + if not netloc: + return + if not netloc.isascii(): + _checknetloc_nfkc(netloc) + + _, port = _parse_hostinfo(netloc) + if port is not None: + _validate_port(port) + def _check_bracketed_netloc(netloc): # Note that this function must mirror the splitting # done in NetlocResultMixins._hostinfo(). diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index 70d3afad446faff..1749cc07d29b05d 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -101,7 +101,8 @@ from urllib.parse import ( urlparse, urlsplit, urljoin, unwrap, quote, unquote, _splittype, _splithost, _splitport, _splituser, _splitpasswd, - _splitattr, _splitvalue, _splittag, + _splitattr, _splitvalue, _splittag, _splitnetloc, + _WHATWG_C0_CONTROL_OR_SPACE, _UNSAFE_URL_BYTES_TO_REMOVE, unquote_to_bytes, urlunparse) from urllib.response import addinfourl, addclosehook @@ -1753,7 +1754,22 @@ def url2pathname(url, *, require_scheme=False, resolve_host=False): """ if not require_scheme: url = 'file:' + url - scheme, authority, url = urlsplit(url)[:3] # Discard query and fragment. + # Match urlsplit()'s own control-character handling up front, so the + # except branch below can reuse this same cleaned-up string. + url = url.lstrip(_WHATWG_C0_CONTROL_OR_SPACE) + for b in _UNSAFE_URL_BYTES_TO_REMOVE: + url = url.replace(b, '') + try: + scheme, authority, url = urlsplit(url)[:3] # Discard query and fragment. + except ValueError: + # The authority here is not a real host[:port] pair -- it may + # hold a Windows drive letter (e.g. 'C:\\') -- so urlsplit()'s + # eager port check doesn't apply; split without it instead. + scheme, _, url = url.partition(':') + scheme = scheme.lower() + authority = '' + if url[:2] == '//': + authority, url = _splitnetloc(url, 2) if scheme != 'file': raise URLError("URL is missing a 'file:' scheme") if os.name == 'nt': diff --git a/Misc/NEWS.d/next/Library/2026-10-11-13-08-07.gh-issue-88037.CzKcxh.rst b/Misc/NEWS.d/next/Library/2026-10-11-13-08-07.gh-issue-88037.CzKcxh.rst new file mode 100644 index 000000000000000..697b44ba2985a0d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-11-13-08-07.gh-issue-88037.CzKcxh.rst @@ -0,0 +1,3 @@ +:func:`urllib.parse.urlsplit` and :func:`urllib.parse.urlparse` now raise +:exc:`ValueError` for an invalid port when the URL is parsed, instead of +when the :attr:`!port` attribute is read. Patch by Miguel Brito and Senthil Kumaran. _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
