Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
:class:`bytearray` instead of :class:`bytes`, violating the documented API contract.
-- 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
2 changes: 1 addition & 1 deletion docs/multipart_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Multipart reference
from ``Content-Encoding`` header. If it
missed data remains untouched

:rtype: bytearray
:rtype: 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.

The PR says it is fixing the return type to match the documentation, but it's clearly changing the documentation?

I don't see any reason we should convert the type currently. We just seem to be getting a ton of automated PRs "fixing" an issue that is already disputed.

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.

The docs do say bytearray — the PR description was wrong about that. Given #12404 is disputed here (and switching to bytes doesn't actually help with JSON serialization, which was the original motivation), there's no reason to keep this open. Closing it — sorry for the noise.


.. method:: read_chunk(size=chunk_size)
:async:
Expand Down
11 changes: 11 additions & 0 deletions tests/test_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ async def test_read(self) -> None:
assert b"Hello, world!" == result
assert obj.at_eof()

@pytest.mark.parametrize("kwargs", [{}, {"decode": True}])
async def test_read_returns_bytes_not_bytearray(
self, kwargs: dict[str, bool]
) -> None:
# Regression test for https://github.com/aio-libs/aiohttp/issues/12404
with Stream(b"Hello, world!\r\n--:") as stream:
d = CIMultiDictProxy[str](CIMultiDict())
obj = aiohttp.BodyPartReader(BOUNDARY, d, stream)
result = await obj.read(**kwargs)
assert isinstance(result, bytes)

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