Skip to content

Commit

Permalink
🐛 fix (early) informational response attempt to read their bodies (#156)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ousret authored Oct 9, 2024
1 parent 3863dfc commit 5ee811c
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 8 deletions.
7 changes: 7 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2.10.902 (2024-10-09)
=====================

- Fixed call to ``stream(..)`` on (early) informational responses. The inner ``fp`` was set to ``None`` and the function
``is_fp_closed`` is not meant to handle this case. Through you should never expect a body in those responses.
- Fixed ``read()``, and ``data`` returns None for (early) informational responses.

2.10.901 (2024-10-08)
=====================

Expand Down
2 changes: 1 addition & 1 deletion src/urllib3/_async/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ async def early_response_handler(
)

early_response = AsyncHTTPResponse(
body=b"",
body=early_low_response,
headers=early_low_response.msg,
status=early_low_response.status,
version=early_low_response.version,
Expand Down
2 changes: 2 additions & 0 deletions src/urllib3/_async/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ async def read( # type: ignore[override]
async def stream( # type: ignore[override]
self, amt: int | None = 2**16, decode_content: bool | None = None
) -> typing.AsyncGenerator[bytes, None]:
if self._fp is None:
return
while not is_fp_closed(self._fp) or len(self._decoded_buffer) > 0:
data = await self.read(amt=amt, decode_content=decode_content)

Expand Down
2 changes: 1 addition & 1 deletion src/urllib3/_version.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# This file is protected via CODEOWNERS
from __future__ import annotations

__version__ = "2.10.901"
__version__ = "2.10.902"
2 changes: 1 addition & 1 deletion src/urllib3/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ def early_response_handler(early_low_response: LowLevelResponse) -> None:
early_resp_options: _ResponseOptions = _promise.get_parameter("response_options") # type: ignore[assignment]

early_response = HTTPResponse(
body=b"",
body=early_low_response,
headers=early_low_response.msg,
status=early_low_response.status,
version=early_low_response.version,
Expand Down
2 changes: 2 additions & 0 deletions src/urllib3/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,8 @@ def stream(
If True, will attempt to decode the body based on the
'content-encoding' header.
"""
if self._fp is None:
return
while not is_fp_closed(self._fp) or len(self._decoded_buffer) > 0:
data = self.read(amt=amt, decode_content=decode_content)

Expand Down
20 changes: 15 additions & 5 deletions test/test_informational.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import pytest

from urllib3 import AsyncPoolManager, HTTPResponse, PoolManager
from urllib3 import AsyncHTTPResponse, AsyncPoolManager, HTTPResponse, PoolManager

# Unfortunately gohttpbin does not support Informational Responses
# as of october 2024. We'll need to create a feature request at
Expand Down Expand Up @@ -33,6 +33,7 @@ def dump_callback(prior_response: HTTPResponse) -> None:
assert early_response is not None
assert early_response.status == 103
assert early_response.headers
assert early_response.data == b""
assert "link" in early_response.headers

assert resp.version == 20 # failsafe/guard in case the remote changes!
Expand Down Expand Up @@ -69,6 +70,13 @@ def dump_callback(prior_response: HTTPResponse) -> None:
assert early_response.headers
assert "link" in early_response.headers

payload: bytes = b""

for chunk in early_response.stream():
payload += chunk

assert payload == b""

assert resp.version == 20 # failsafe/guard in case the remote changes!

assert resp.status == 200
Expand All @@ -79,9 +87,9 @@ def dump_callback(prior_response: HTTPResponse) -> None:
@requires_network()
@pytest.mark.asyncio
async def test_async_no_multiplex_early_response() -> None:
early_response: HTTPResponse | None = None
early_response: AsyncHTTPResponse | None = None

async def dump_callback(prior_response: HTTPResponse) -> None:
async def dump_callback(prior_response: AsyncHTTPResponse) -> None:
nonlocal early_response
early_response = prior_response

Expand All @@ -95,6 +103,7 @@ async def dump_callback(prior_response: HTTPResponse) -> None:
assert early_response is not None
assert early_response.status == 103
assert early_response.headers
assert (await early_response.read()) == b""
assert "link" in early_response.headers

assert resp.version == 20 # failsafe/guard in case the remote changes!
Expand All @@ -107,9 +116,9 @@ async def dump_callback(prior_response: HTTPResponse) -> None:
@requires_network()
@pytest.mark.asyncio
async def test_async_with_multiplex_early_response() -> None:
early_response: HTTPResponse | None = None
early_response: AsyncHTTPResponse | None = None

async def dump_callback(prior_response: HTTPResponse) -> None:
async def dump_callback(prior_response: AsyncHTTPResponse) -> None:
nonlocal early_response
early_response = prior_response

Expand All @@ -130,6 +139,7 @@ async def dump_callback(prior_response: HTTPResponse) -> None:
assert early_response.status == 103
assert early_response.version == 20 # failsafe/guard in case the remote changes!
assert early_response.headers
assert (await early_response.data) == b""
assert "link" in early_response.headers

assert resp.version == 20 # failsafe/guard in case the remote changes!
Expand Down

0 comments on commit 5ee811c

Please sign in to comment.