Skip to content

Commit db1f75c

Browse files
barneygalesrinivasreddy
authored andcommitted
pythonGH-127090: Fix urllib.response.addinfourl.url value for opened file: URIs (python#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.
1 parent 4263465 commit db1f75c

File tree

5 files changed

+25
-28
lines changed

5 files changed

+25
-28
lines changed

Lib/test/test_urllib.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def test_headers(self):
156156
self.assertIsInstance(self.returned_obj.headers, email.message.Message)
157157

158158
def test_url(self):
159-
self.assertEqual(self.returned_obj.url, "file://" + self.quoted_pathname)
159+
self.assertEqual(self.returned_obj.url, "file:" + self.quoted_pathname)
160160

161161
def test_status(self):
162162
self.assertIsNone(self.returned_obj.status)
@@ -165,7 +165,7 @@ def test_info(self):
165165
self.assertIsInstance(self.returned_obj.info(), email.message.Message)
166166

167167
def test_geturl(self):
168-
self.assertEqual(self.returned_obj.geturl(), "file://" + self.quoted_pathname)
168+
self.assertEqual(self.returned_obj.geturl(), "file:" + self.quoted_pathname)
169169

170170
def test_getcode(self):
171171
self.assertIsNone(self.returned_obj.getcode())
@@ -471,11 +471,14 @@ def test_missing_localfile(self):
471471

472472
def test_file_notexists(self):
473473
fd, tmp_file = tempfile.mkstemp()
474-
tmp_fileurl = 'file://localhost/' + tmp_file.replace(os.path.sep, '/')
474+
tmp_file_canon_url = 'file:' + urllib.request.pathname2url(tmp_file)
475+
parsed = urllib.parse.urlsplit(tmp_file_canon_url)
476+
tmp_fileurl = parsed._replace(netloc='localhost').geturl()
475477
try:
476478
self.assertTrue(os.path.exists(tmp_file))
477479
with urllib.request.urlopen(tmp_fileurl) as fobj:
478480
self.assertTrue(fobj)
481+
self.assertEqual(fobj.url, tmp_file_canon_url)
479482
finally:
480483
os.close(fd)
481484
os.unlink(tmp_file)
@@ -609,7 +612,7 @@ def tearDown(self):
609612

610613
def constructLocalFileUrl(self, filePath):
611614
filePath = os.path.abspath(filePath)
612-
return "file://%s" % urllib.request.pathname2url(filePath)
615+
return "file:" + urllib.request.pathname2url(filePath)
613616

614617
def createNewTempFile(self, data=b""):
615618
"""Creates a new temporary file containing the specified data,

Lib/test/test_urllib2.py

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
_proxy_bypass_winreg_override,
2424
_proxy_bypass_macosx_sysconf,
2525
AbstractDigestAuthHandler)
26-
from urllib.parse import urlparse
26+
from urllib.parse import urlsplit
2727
import urllib.error
2828
import http.client
2929

@@ -717,14 +717,6 @@ def test_processors(self):
717717
self.assertIsInstance(args[1], MockResponse)
718718

719719

720-
def sanepathname2url(path):
721-
urlpath = urllib.request.pathname2url(path)
722-
if os.name == "nt" and urlpath.startswith("///"):
723-
urlpath = urlpath[2:]
724-
# XXX don't ask me about the mac...
725-
return urlpath
726-
727-
728720
class HandlerTests(unittest.TestCase):
729721

730722
def test_ftp(self):
@@ -818,19 +810,22 @@ def test_file(self):
818810
o = h.parent = MockOpener()
819811

820812
TESTFN = os_helper.TESTFN
821-
urlpath = sanepathname2url(os.path.abspath(TESTFN))
822813
towrite = b"hello, world\n"
814+
canonurl = 'file:' + urllib.request.pathname2url(os.path.abspath(TESTFN))
815+
parsed = urlsplit(canonurl)
816+
if parsed.netloc:
817+
raise unittest.SkipTest("non-local working directory")
823818
urls = [
824-
"file://localhost%s" % urlpath,
825-
"file://%s" % urlpath,
826-
"file://%s%s" % (socket.gethostbyname('localhost'), urlpath),
819+
canonurl,
820+
parsed._replace(netloc='localhost').geturl(),
821+
parsed._replace(netloc=socket.gethostbyname('localhost')).geturl(),
827822
]
828823
try:
829824
localaddr = socket.gethostbyname(socket.gethostname())
830825
except socket.gaierror:
831826
localaddr = ''
832827
if localaddr:
833-
urls.append("file://%s%s" % (localaddr, urlpath))
828+
urls.append(parsed._replace(netloc=localaddr).geturl())
834829

835830
for url in urls:
836831
f = open(TESTFN, "wb")
@@ -855,10 +850,10 @@ def test_file(self):
855850
self.assertEqual(headers["Content-type"], "text/plain")
856851
self.assertEqual(headers["Content-length"], "13")
857852
self.assertEqual(headers["Last-modified"], modified)
858-
self.assertEqual(respurl, url)
853+
self.assertEqual(respurl, canonurl)
859854

860855
for url in [
861-
"file://localhost:80%s" % urlpath,
856+
parsed._replace(netloc='localhost:80').geturl(),
862857
"file:///file_does_not_exist.txt",
863858
"file://not-a-local-host.com//dir/file.txt",
864859
"file://%s:80%s/%s" % (socket.gethostbyname('localhost'),
@@ -1156,13 +1151,13 @@ def test_full_url_setter(self):
11561151
r = Request('http://example.com')
11571152
for url in urls:
11581153
r.full_url = url
1159-
parsed = urlparse(url)
1154+
parsed = urlsplit(url)
11601155

11611156
self.assertEqual(r.get_full_url(), url)
11621157
# full_url setter uses splittag to split into components.
11631158
# splittag sets the fragment as None while urlparse sets it to ''
11641159
self.assertEqual(r.fragment or '', parsed.fragment)
1165-
self.assertEqual(urlparse(r.get_full_url()).query, parsed.query)
1160+
self.assertEqual(urlsplit(r.get_full_url()).query, parsed.query)
11661161

11671162
def test_full_url_deleter(self):
11681163
r = Request('http://www.example.com')

Lib/test/test_urllib2net.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
from test.support import os_helper
55
from test.support import socket_helper
66
from test.support import ResourceDenied
7-
from test.test_urllib2 import sanepathname2url
87

98
import os
109
import socket
@@ -151,7 +150,7 @@ def test_file(self):
151150
f.write('hi there\n')
152151
f.close()
153152
urls = [
154-
'file:' + sanepathname2url(os.path.abspath(TESTFN)),
153+
'file:' + urllib.request.pathname2url(os.path.abspath(TESTFN)),
155154
('file:///nonsensename/etc/passwd', None,
156155
urllib.error.URLError),
157156
]

Lib/urllib/request.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,10 +1488,7 @@ def open_local_file(self, req):
14881488
host, port = _splitport(host)
14891489
if not host or \
14901490
(not port and _safe_gethostbyname(host) in self.get_names()):
1491-
if host:
1492-
origurl = 'file://' + host + filename
1493-
else:
1494-
origurl = 'file://' + filename
1491+
origurl = 'file:' + pathname2url(localfile)
14951492
return addinfourl(open(localfile, 'rb'), headers, origurl)
14961493
except OSError as exp:
14971494
raise URLError(exp, exp.filename)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix value of :attr:`urllib.response.addinfourl.url` for ``file:`` URLs that
2+
express relative paths and absolute Windows paths. The canonical URL generated
3+
by :func:`urllib.request.pathname2url` is now used.

0 commit comments

Comments
 (0)