Skip to content

Commit

Permalink
pythonGH-127090: Fix urllib.response.addinfourl.url value for opene…
Browse files Browse the repository at this point in the history
…d `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.
  • Loading branch information
barneygale authored Dec 7, 2024
1 parent 27d0d21 commit 79b7cab
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 28 deletions.
11 changes: 7 additions & 4 deletions Lib/test/test_urllib.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def test_headers(self):
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)
Expand All @@ -165,7 +165,7 @@ def test_info(self):
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())
Expand Down Expand Up @@ -471,11 +471,14 @@ def test_missing_localfile(self):

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)
Expand Down Expand Up @@ -609,7 +612,7 @@ def tearDown(self):

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,
Expand Down
31 changes: 13 additions & 18 deletions Lib/test/test_urllib2.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
_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

Expand Down Expand Up @@ -717,14 +717,6 @@ def test_processors(self):
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):
Expand Down Expand Up @@ -818,19 +810,22 @@ def test_file(self):
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")
Expand All @@ -855,10 +850,10 @@ def test_file(self):
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'),
Expand Down Expand Up @@ -1156,13 +1151,13 @@ def test_full_url_setter(self):
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')
Expand Down
3 changes: 1 addition & 2 deletions Lib/test/test_urllib2net.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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
Expand Down Expand Up @@ -151,7 +150,7 @@ def test_file(self):
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),
]
Expand Down
5 changes: 1 addition & 4 deletions Lib/urllib/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -1488,10 +1488,7 @@ def open_local_file(self, req):
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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 79b7cab

Please sign in to comment.