Calling read() on HTTPError may cause KeyError in tempfile

[sivel]

BPO	45955
Nosy	@corona10, @sivel

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-12-01.19:12:05.368>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'Calling read() on HTTPError may cause KeyError in tempfile'
updated_at = <Date 2022-02-07.03:08:26.172>
user = 'https://github.com/sivel'

bugs.python.org fields:

activity = <Date 2022-02-07.03:08:26.172>
actor = 'corona10'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-12-01.19:12:05.368>
creator = 'sivel'
dependencies = []
files = []
hgrepos = []
issue_num = 45955
keywords = []
message_count = 1.0
messages = ['407482']
nosy_count = 2.0
nosy_names = ['corona10', 'sivel']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45955'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[sivel]

HTTPError may not be fully initialized in some scenarios leading to an inconsistent interface. This is documented in code at:

cpython/Lib/urllib/error.py

Lines 45 to 50 in 55fe1ae

	# The addinfourl classes depend on fp being a valid file
	# object. In some cases, the HTTPError may not have a valid
	# file object. If this happens, the simplest workaround is to
	# not initialize the base classes.
	if fp is not None:
	self.__super_init(fp, hdrs, url, code)

Unfortunately the way this is implemented creates an inconsistent interface, and opaque code, without a number of inline comments explaining the behavior of HTTPError.

Additionally, the way that it currently works, will cause a KeyError to be raised from tempfile, which is rather confusing.

Instead of "partially initializing" the HTTPError object, I'd propose that when fp is None, that we provide it with something like io.BytesIO to fulfill the interface. There may be other recommended solutions, I've not thought through this extensively yet.

I think I just prefer always calling self.__super_init but passing in something like io.BytesIO if fp is None

I'm willing to create the PR once I know which direction seems to make the most sense.

>>> from urllib.error import HTTPError
>>> from urllib.request import HTTPDigestAuthHandler, HTTPPasswordMgrWithDefaultRealm, build_opener
>>> passman = HTTPPasswordMgrWithDefaultRealm()
>>> passman.add_password(None, 'httpbin.org', 'user', 'wrong')
>>> opener = build_opener(HTTPDigestAuthHandler(passman))
>>> try:
...     opener.open('https://httpbin.org/digest-auth/auth/user/passwd')
... except HTTPError as e:
...     e.read()
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 525, in open
    response = meth(req, response)
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 634, in http_response
    response = self.parent.error(
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 557, in error
    result = self._call_chain(*args)
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 496, in _call_chain
    result = func(*args)
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 1238, in http_error_401
    retry = self.http_error_auth_reqed('www-authenticate',
  File ".../3.10.0/lib/python3.10/urllib/request.py", line 1111, in http_error_auth_reqed
    raise HTTPError(req.full_url, 401, "digest auth failed",


During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
  File ".../3.10.0/lib/python3.10/tempfile.py", line 473, in __getattr__
    file = self.__dict__['file']
KeyError: 'file'

[icemac]

Calling getattr(exc_value, "__notes__", ()) on HTTPError causes the same exception.

This happens when pytest >= 7.2 tries to render an exception using the exceptiongroup package.

[corona10]

@icemac #98778 looks similar.
It's still happening with the latest commit version?

[corona10]

Python 3.12.0a5+ (heads/main:8de59c1bb9, Mar  5 2023, 00:53:41) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from urllib.error import HTTPError
>>> from urllib.request import HTTPDigestAuthHandler, HTTPPasswordMgrWithDefaultRealm, build_opener
>>> passman = HTTPPasswordMgrWithDefaultRealm()
>>> passman.add_password(None, 'httpbin.org', 'user', 'wrong')
>>> opener = build_opener(HTTPDigestAuthHandler(passman))
>>> try:
...     opener.open('https://httpbin.org/digest-auth/auth/user/passwd')
... except HTTPError as e:
...     e.read()
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/Users/user/oss/cpython/Lib/urllib/request.py", line 515, in open
    response = self._open(req, data)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/oss/cpython/Lib/urllib/request.py", line 537, in _open
    return self._call_chain(self.handle_open, 'unknown',
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/user/oss/cpython/Lib/urllib/request.py", line 492, in _call_chain
    result = func(*args)
             ^^^^^^^^^^^
  File "/Users/user/oss/cpython/Lib/urllib/request.py", line 1419, in unknown_open
    raise URLError('unknown url type: %s' % type)
urllib.error.URLError: <urlopen error unknown url type: https>
>>>

Looks like be fixed.

[icemac]

@corona10 I forgot to mention that I am using Python 3.9.16. Is there a chance to get the fix backported to that version?

[corona10]

@icemac

@corona10 I forgot to mention that I am using Python 3.9.16. Is there a chance to get the fix backported to that version?

Unfortunately, no

[corona10]

This issue was solved by the following issues.

• urllib.error.HTTPError(..., fp=None) raises a KeyError instead of an AttributeError on attribute access #98778
• HTTPError fp.read returns string instead of bytes  #101936