GH-127090: Fix `urllib.response.addinfourl.url` value for opened `file:` URIs (#127091)

The canonical `file:` URL (as generated by `pathname2url()`) is now used as the `url` attribute of the returned `addinfourl` object. The `addinfourl.url` attribute reflects the resolved URL for both `file:` or `http[s]:` URLs now.

diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
index 00e46990c406ac6d2d835dbfe87ec53ad41a7e86..042d3b35b770229b7170c3c65056a04f52342ffe 100644 (file)
--- a/Lib/test/test_urllib.py
+++ b/Lib/test/test_urllib.py
@@ -156,7 +156,7 @@ class urlopen_FileTests(unittest.TestCase):
         self.assertIsInstance(self.returned_obj.headers, email.message.Message)
 
     def test_url(self):
-        self.assertEqual(self.returned_obj.url, "file://" + self.quoted_pathname)
+        self.assertEqual(self.returned_obj.url, "file:" + self.quoted_pathname)
 
     def test_status(self):
         self.assertIsNone(self.returned_obj.status)
@@ -165,7 +165,7 @@ class urlopen_FileTests(unittest.TestCase):
         self.assertIsInstance(self.returned_obj.info(), email.message.Message)
 
     def test_geturl(self):
-        self.assertEqual(self.returned_obj.geturl(), "file://" + self.quoted_pathname)
+        self.assertEqual(self.returned_obj.geturl(), "file:" + self.quoted_pathname)
 
     def test_getcode(self):
         self.assertIsNone(self.returned_obj.getcode())
@@ -471,11 +471,14 @@ Connection: close
 
     def test_file_notexists(self):
         fd, tmp_file = tempfile.mkstemp()
-        tmp_fileurl = 'file://localhost/' + tmp_file.replace(os.path.sep, '/')
+        tmp_file_canon_url = 'file:' + urllib.request.pathname2url(tmp_file)
+        parsed = urllib.parse.urlsplit(tmp_file_canon_url)
+        tmp_fileurl = parsed._replace(netloc='localhost').geturl()
         try:
             self.assertTrue(os.path.exists(tmp_file))
             with urllib.request.urlopen(tmp_fileurl) as fobj:
                 self.assertTrue(fobj)
+                self.assertEqual(fobj.url, tmp_file_canon_url)
         finally:
             os.close(fd)
             os.unlink(tmp_file)
@@ -609,7 +612,7 @@ class urlretrieve_FileTests(unittest.TestCase):
 
     def constructLocalFileUrl(self, filePath):
         filePath = os.path.abspath(filePath)
-        return "file://%s" % urllib.request.pathname2url(filePath)
+        return "file:" + urllib.request.pathname2url(filePath)
 
     def createNewTempFile(self, data=b""):
         """Creates a new temporary file containing the specified data,

diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
index 99ad11cf0552ebd7802de8b6e7b628a66ace8f3c..4a9e653515be5b2c9fc3a50c184eb95bead4f0da 100644 (file)
--- a/Lib/test/test_urllib2.py
+++ b/Lib/test/test_urllib2.py
@@ -23,7 +23,7 @@ from urllib.request import (Request, OpenerDirector, HTTPBasicAuthHandler,
                             _proxy_bypass_winreg_override,
                             _proxy_bypass_macosx_sysconf,
                             AbstractDigestAuthHandler)
-from urllib.parse import urlparse
+from urllib.parse import urlsplit
 import urllib.error
 import http.client
 
@@ -717,14 +717,6 @@ class OpenerDirectorTests(unittest.TestCase):
                     self.assertIsInstance(args[1], MockResponse)
 
 
-def sanepathname2url(path):
-    urlpath = urllib.request.pathname2url(path)
-    if os.name == "nt" and urlpath.startswith("///"):
-        urlpath = urlpath[2:]
-    # XXX don't ask me about the mac...
-    return urlpath
-
-
 class HandlerTests(unittest.TestCase):
 
     def test_ftp(self):
@@ -818,19 +810,22 @@ class HandlerTests(unittest.TestCase):
         o = h.parent = MockOpener()
 
         TESTFN = os_helper.TESTFN
-        urlpath = sanepathname2url(os.path.abspath(TESTFN))
         towrite = b"hello, world\n"
+        canonurl = 'file:' + urllib.request.pathname2url(os.path.abspath(TESTFN))
+        parsed = urlsplit(canonurl)
+        if parsed.netloc:
+            raise unittest.SkipTest("non-local working directory")
         urls = [
-            "file://localhost%s" % urlpath,
-            "file://%s" % urlpath,
-            "file://%s%s" % (socket.gethostbyname('localhost'), urlpath),
+            canonurl,
+            parsed._replace(netloc='localhost').geturl(),
+            parsed._replace(netloc=socket.gethostbyname('localhost')).geturl(),
             ]
         try:
             localaddr = socket.gethostbyname(socket.gethostname())
         except socket.gaierror:
             localaddr = ''
         if localaddr:
-            urls.append("file://%s%s" % (localaddr, urlpath))
+            urls.append(parsed._replace(netloc=localaddr).geturl())
 
         for url in urls:
             f = open(TESTFN, "wb")
@@ -855,10 +850,10 @@ class HandlerTests(unittest.TestCase):
             self.assertEqual(headers["Content-type"], "text/plain")
             self.assertEqual(headers["Content-length"], "13")
             self.assertEqual(headers["Last-modified"], modified)
-            self.assertEqual(respurl, url)
+            self.assertEqual(respurl, canonurl)
 
         for url in [
-            "file://localhost:80%s" % urlpath,
+            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'),
@@ -1156,13 +1151,13 @@ class HandlerTests(unittest.TestCase):
         r = Request('http://example.com')
         for url in urls:
             r.full_url = url
-            parsed = urlparse(url)
+            parsed = urlsplit(url)
 
             self.assertEqual(r.get_full_url(), url)
             # full_url setter uses splittag to split into components.
             # splittag sets the fragment as None while urlparse sets it to ''
             self.assertEqual(r.fragment or '', parsed.fragment)
-            self.assertEqual(urlparse(r.get_full_url()).query, parsed.query)
+            self.assertEqual(urlsplit(r.get_full_url()).query, parsed.query)
 
     def test_full_url_deleter(self):
         r = Request('http://www.example.com')

diff --git a/Lib/test/test_urllib2net.py b/Lib/test/test_urllib2net.py
index f0874d8d3ce46347a09a5a47fecb67b6c3f6e651..b84290a7368c29ea6f53780e5ce9f25a82e272d8 100644 (file)
--- a/Lib/test/test_urllib2net.py
+++ b/Lib/test/test_urllib2net.py
@@ -4,7 +4,6 @@ from test import support
 from test.support import os_helper
 from test.support import socket_helper
 from test.support import ResourceDenied
-from test.test_urllib2 import sanepathname2url
 
 import os
 import socket
@@ -151,7 +150,7 @@ class OtherNetworkTests(unittest.TestCase):
             f.write('hi there\n')
             f.close()
             urls = [
-                'file:' + sanepathname2url(os.path.abspath(TESTFN)),
+                'file:' + urllib.request.pathname2url(os.path.abspath(TESTFN)),
                 ('file:///nonsensename/etc/passwd', None,
                  urllib.error.URLError),
                 ]

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index 1fcaa89188188dd177aa9e20c8dfb882e9476515..7ef85431b718ad7c319cbfb939d27a1902570000 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -1488,10 +1488,7 @@ class FileHandler(BaseHandler):
                 host, port = _splitport(host)
             if not host or \
                 (not port and _safe_gethostbyname(host) in self.get_names()):
-                if host:
-                    origurl = 'file://' + host + filename
-                else:
-                    origurl = 'file://' + filename
+                origurl = 'file:' + pathname2url(localfile)
                 return addinfourl(open(localfile, 'rb'), headers, origurl)
         except OSError as exp:
             raise URLError(exp, exp.filename)

diff --git a/Misc/NEWS.d/next/Library/2024-11-21-06-03-46.gh-issue-127090.yUYwdh.rst b/Misc/NEWS.d/next/Library/2024-11-21-06-03-46.gh-issue-127090.yUYwdh.rst
new file mode 100644 (file)
index 0000000..8efe563
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-11-21-06-03-46.gh-issue-127090.yUYwdh.rst
@@ -0,0 +1,3 @@
+Fix value of :attr:`urllib.response.addinfourl.url` for ``file:`` URLs that
+express relative paths and absolute Windows paths. The canonical URL generated
+by :func:`urllib.request.pathname2url` is now used.