FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-44672: Stop pre-resolving the FTP host in urllib.request by soreavis · Pull Request #154192 · python/cpython · GitHub

/ cpython Public
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .py  (2) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
41 changes: 40 additions & 1 deletion Lib/test/test_urllib2.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ def connect_ftp(self, user, passwd, host, port, dirs,
# ftp authentication not yet implemented by FTPHandler
self.assertEqual(h.user, user)
self.assertEqual(h.passwd, passwd)
self.assertEqual(h.host, socket.gethostbyname(host))
self.assertEqual(h.host, host)
self.assertEqual(h.port, port)
self.assertEqual(h.dirs, dirs)
self.assertEqual(h.ftpwrapper.filename, filename)
Expand All @@ -797,6 +797,45 @@ def connect_ftp(self, user, passwd, host, port, dirs,
self.assertEqual(int(headers["Content-length"]), len(data))
r.close()

def test_ftp_no_hostname_resolution(self):
# gh-44672: the host must reach connect_ftp unresolved so that
# ftplib resolves it via getaddrinfo(), which supports IPv6.
class MockFTPWrapper:
def retrfile(self, filename, filetype):
return io.StringIO(""), 0

def close(self):
pass

class NullFTPHandler(urllib.request.FTPHandler):
def connect_ftp(self, user, passwd, host, port, dirs, timeout):
self.host = host
return MockFTPWrapper()

h = NullFTPHandler()
req = Request("ftp://localhost/foo/bar.html")
req.timeout = None
with mock.patch.object(socket, "gethostbyname",
side_effect=AssertionError(
"ftp_open must not pre-resolve the host")):
r = h.ftp_open(req)
r.close()
self.assertEqual(h.host, "localhost")

def test_ftp_gaierror(self):
# gh-44672: without pre-resolution the lookup failure now surfaces
# from ftplib as socket.gaierror; it must still become a URLError.
class GaiErrorFTPHandler(urllib.request.FTPHandler):
def connect_ftp(self, user, passwd, host, port, dirs, timeout):
raise socket.gaierror(-2, "Name or service not known")

h = GaiErrorFTPHandler()
req = Request("ftp://nonexistent.invalid/")
req.timeout = None
with self.assertRaises(urllib.error.URLError) as cm:
h.ftp_open(req)
self.assertIsInstance(cm.exception.__cause__, socket.gaierror)

@support.requires_resource("network")
def test_ftp_error(self):
class ErrorFTPHandler(urllib.request.FTPHandler):
Expand Down
4 changes: 0 additions & 4 deletions Lib/urllib/request.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -1526,10 +1526,6 @@ def ftp_open(self, req):
user = user or ''
passwd = passwd or ''

try:
host = socket.gethostbyname(host)
except OSError as msg:
raise URLError(msg)
path, attrs = _splitattr(req.selector)
dirs = path.split('/')
dirs = list(map(unquote, dirs))
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:func:`urllib.request.urlopen` can now fetch ``ftp://`` URLs whose host is
IPv6-only. :class:`~urllib.request.FTPHandler` no longer pre-resolves the host
with the IPv4-only :func:`socket.gethostbyname`; the unresolved host is passed to
:mod:`ftplib`, which resolves it with :func:`socket.getaddrinfo`.
Loading

Back | FazBrowse Home | New Git URL