Skip to content

Commit

Permalink
Distinguish between truncated and excess content in response
Browse files Browse the repository at this point in the history
`HTTPResponse._raw_read` raises `IncompleteRead` if the content length does
not match the expected content length. For malformed responses (e.g. 204
response with content) the re-raised `ProtocolError` was a bit too unclear
about the unexpected excess content being the reason for the exception.

With this change, the exception points out, that the client is not dealing
with a connection error but a protocol violation.

Closes urllib3#3261
  • Loading branch information
crazyscientist committed Feb 5, 2024
1 parent 23f2287 commit da7e153
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/3261.misc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Made re-raised ProtocolError more verbose regarding corner cases.
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 int(e.expected) == -int(e.partial)
):
arg = f"Protocol violation: Response may not contain content: {e!r}"
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
27 changes: 27 additions & 0 deletions test/test_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,33 @@ 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 "Protocol violation: 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 "Protocol violation: 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 "Protocol violation: Response may not contain content" in str(
excinfo.value
)


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

0 comments on commit da7e153

Please sign in to comment.