Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES/12404.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed ``BodyPartReader.read()`` and ``BodyPartReader.read(decode=True)`` returning
``bytearray`` instead of ``bytes``, violating the documented API contract.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use RST roles to link the types.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e650dc4 — both types are now linked with :class:\bytearray`and:class:`bytes``.

-- by :user:`CrepuscularIRIS`.
4 changes: 2 additions & 2 deletions aiohttp/multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ async def read(self, *, decode: bool = False) -> bytes:
decoded_data.extend(d)
if len(decoded_data) > self._client_max_size:
raise self._max_size_error_cls(self._client_max_size)
return decoded_data
return data
return bytes(decoded_data)
return bytes(data)

async def read_chunk(self, size: int = chunk_size) -> bytes:
"""Reads body part content chunk of the specified size.
Expand Down
23 changes: 23 additions & 0 deletions tests/test_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,29 @@ async def test_read(self) -> None:
assert b"Hello, world!" == result
assert obj.at_eof()

async def test_read_returns_bytes_not_bytearray(self) -> None:
# Regression test for https://github.com/aio-libs/aiohttp/issues/12404
# read() must return bytes (not bytearray) to honour the documented API contract.
with Stream(b"Hello, world!\r\n--:") as stream:
d = CIMultiDictProxy[str](CIMultiDict())
obj = aiohttp.BodyPartReader(BOUNDARY, d, stream)
result = await obj.read()
assert isinstance(
result, bytes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have the assertion in the CM.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e650dc4 — the with pytest.raises(...) block no longer contains an assert.

), f"Expected bytes, got {type(result).__name__}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have a custom assertion text, the test runner already reveals the local vars better.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e650dc4 — the custom match string is removed; pytest's default output is enough.


async def test_read_decode_returns_bytes_not_bytearray(self) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use parametrization instead of duplicating the same check with only small difference but exactly the same semantics.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e650dc4 — the two copies are now a single @pytest.mark.parametrize("kwargs", [{}, {"decode": True}]) test.

# Regression test for https://github.com/aio-libs/aiohttp/issues/12404
# read(decode=True) must return bytes (not bytearray) even when no
# Content-Transfer-Encoding / Content-Encoding header is present.
with Stream(b"Hello, world!\r\n--:") as stream:
d = CIMultiDictProxy[str](CIMultiDict())
obj = aiohttp.BodyPartReader(BOUNDARY, d, stream)
result = await obj.read(decode=True)
assert isinstance(
result, bytes
), f"Expected bytes, got {type(result).__name__}"

async def test_read_chunk_at_eof(self) -> None:
with Stream(b"--:") as stream:
d = CIMultiDictProxy[str](CIMultiDict())
Expand Down
Loading