Skip to content
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

Distinguish between truncated and excess content in response #3273

Merged
merged 8 commits into from
Feb 12, 2024
1 change: 1 addition & 0 deletions changelog/3261.misc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Made raised ``urllib3.exceptions.ProtocolError`` more verbose when a response contains content unexpectedly.
7 changes: 5 additions & 2 deletions src/urllib3/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,16 @@ class IncompleteRead(HTTPError, httplib_IncompleteRead):
for ``partial`` to avoid creating large objects on streamed reads.
"""

partial: int # type: ignore[assignment]
expected: int

def __init__(self, partial: int, expected: int) -> None:
self.partial = partial # type: ignore[assignment]
self.partial = partial
self.expected = expected

def __repr__(self) -> str:
return "IncompleteRead(%i bytes read, %i more expected)" % (
self.partial, # type: ignore[str-format]
self.partial,
self.expected,
)

Expand Down
12 changes: 11 additions & 1 deletion src/urllib3/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,18 @@ def _error_catcher(self) -> typing.Generator[None, None, None]:

raise ReadTimeoutError(self._pool, None, "Read timed out.") from e # type: ignore[arg-type]

except IncompleteRead as e:
if (
e.expected is not None
and e.partial is not None
and e.expected == -e.partial
):
arg = "Response may not contain content."
else:
arg = f"Connection broken: {e!r}"
raise ProtocolError(arg, e) from e

except (HTTPException, OSError) as e:
# This includes IncompleteRead.
raise ProtocolError(f"Connection broken: {e!r}", e) from e

# If no exception is thrown, we should avoid cleaning up
Expand Down
23 changes: 22 additions & 1 deletion test/test_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,7 @@ def test_buggy_incomplete_read(self) -> None:

orig_ex = ctx.value.args[1]
assert isinstance(orig_ex, IncompleteRead)
assert orig_ex.partial == 0 # type: ignore[comparison-overlap]
assert orig_ex.partial == 0
assert orig_ex.expected == content_length

def test_incomplete_chunk(self) -> None:
Expand Down Expand Up @@ -1504,6 +1504,27 @@ def make_bad_mac_fp() -> typing.Generator[BytesIO, None, None]:
resp.read()
assert e.value.args[0] == mac_error

def test_unexpected_body(self) -> None:
with pytest.raises(ProtocolError) as excinfo:
fp = BytesIO(b"12345")
headers = {"content-length": "5"}
resp = HTTPResponse(fp, status=204, headers=headers)
resp.read(16)
assert "Response may not contain content" in str(excinfo.value)

with pytest.raises(ProtocolError):
fp = BytesIO(b"12345")
headers = {"content-length": "0"}
resp = HTTPResponse(fp, status=204, headers=headers)
resp.read(16)
assert "Response may not contain content" in str(excinfo.value)

with pytest.raises(ProtocolError):
fp = BytesIO(b"12345")
resp = HTTPResponse(fp, status=204)
resp.read(16)
assert "Response may not contain content" in str(excinfo.value)


class MockChunkedEncodingResponse:
def __init__(self, content: list[bytes]) -> None:
Expand Down