-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
GH-123599: url2pathname()
: don't call gethostbyname()
by default
#132610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
0630a2d
26c8688
1bff8b9
4ec391f
8df17b4
9e75357
873c19a
4bbeef6
2a42d88
e94ad6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1466,7 +1466,7 @@ def get_names(self): | |
def open_local_file(self, req): | ||
import email.utils | ||
import mimetypes | ||
localfile = url2pathname(req.full_url, require_scheme=True) | ||
localfile = url2pathname(req.full_url, require_scheme=True, resolve_host=True) | ||
try: | ||
stats = os.stat(localfile) | ||
size = stats.st_size | ||
|
@@ -1482,7 +1482,7 @@ def open_local_file(self, req): | |
|
||
file_open = open_local_file | ||
|
||
def _is_local_authority(authority): | ||
def _is_local_authority(authority, resolve): | ||
# Compare hostnames | ||
if not authority or authority == 'localhost': | ||
return True | ||
|
@@ -1494,6 +1494,8 @@ def _is_local_authority(authority): | |
if authority == hostname: | ||
return True | ||
# Compare IP addresses | ||
if not resolve: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder, should we check this before calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm struggling to come up with firm info on when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not know. But I was thinking about this case: processing the "file:" URL on different computer. We cannot use I do not know how common this case, and how common the opposite case, when we only use |
||
return False | ||
try: | ||
address = socket.gethostbyname(authority) | ||
except (socket.gaierror, AttributeError): | ||
barneygale marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -1641,21 +1643,24 @@ def data_open(self, req): | |
return addinfourl(io.BytesIO(data), headers, url) | ||
|
||
|
||
# Code move from the old urllib module | ||
# Code moved from the old urllib module | ||
|
||
def url2pathname(url, *, require_scheme=False): | ||
def url2pathname(url, *, require_scheme=False, resolve_host=False): | ||
"""Convert the given file URL to a local file system path. | ||
|
||
The 'file:' scheme prefix must be omitted unless *require_scheme* | ||
is set to true. | ||
|
||
The URL authority may be resolved with gethostbyname() if | ||
*resolve_host* is set to true. | ||
""" | ||
if require_scheme: | ||
scheme, url = _splittype(url) | ||
if scheme != 'file': | ||
raise URLError("URL is missing a 'file:' scheme") | ||
authority, url = _splithost(url) | ||
if os.name == 'nt': | ||
if not _is_local_authority(authority): | ||
if not _is_local_authority(authority, resolve_host): | ||
# e.g. file://server/share/file.txt | ||
url = '//' + authority + url | ||
elif url[:3] == '///': | ||
|
@@ -1669,7 +1674,7 @@ def url2pathname(url, *, require_scheme=False): | |
# Older URLs use a pipe after a drive letter | ||
url = url[:1] + ':' + url[2:] | ||
url = url.replace('/', '\\') | ||
elif not _is_local_authority(authority): | ||
elif not _is_local_authority(authority, resolve_host): | ||
raise URLError("file:// scheme is supported only on localhost") | ||
encoding = sys.getfilesystemencoding() | ||
errors = sys.getfilesystemencodeerrors() | ||
|
8073
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
Fix issue where :func:`urllib.request.url2pathname` mishandled file URLs with | ||
authorities. If an authority is present and resolves to ``localhost``, it is | ||
now discarded. If an authority is present but *doesn't* resolve to | ||
``localhost``, then on Windows a UNC path is returned (as before), and on | ||
other platforms a :exc:`urllib.error.URLError` is now raised. | ||
Add *resolve_host* keyword-only argument to | ||
:func:`urllib.request.url2pathname`, and fix handling of file URLs with | ||
barneygale marked this conversation as resolved.
Show resolved
Hide resolved
|
||
authorities. |
Uh oh!
There was an error while loading. Please reload this page.