requests: Add chunked response body support.#1135
Conversation
Decode Transfer-Encoding: chunked response bodies incrementally with a ChunkedStream wrapper, keeping .raw streaming (read/readinto) and .content lazy without buffering everything in request(). Chunk extensions are skipped and trailers are discarded. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Replace the test that expected a ValueError for chunked responses with coverage of decoding via .content and incremental .raw.readinto(). Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Note chunked response decoding in the feature list and drop the outdated limitation and follow-up entry. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
| self._sock.close() | ||
|
|
||
|
|
||
| class ChunkedStream: |
There was a problem hiding this comment.
This is a lot of extra code. Can you find a way to reduce the code size, maybe combine this class with BodyStream?
To test code size, run mpy-cross __init__.py and look at the size of the resulting __init__.mpy file. Making that file smaller is the goal.
There was a problem hiding this comment.
Thanks for the review. I merged chunked decoding into BodyStream with a chunked flag and inlined the chunk header parsing in readinto(), removing the separate ChunkedStream class (commit 4822856).
__init__.mpy sizes measured with mpy-cross against upstream/master (no chunked support):
| Version | __init__.mpy |
vs master |
|---|---|---|
| master (no chunked) | 2929 bytes | - |
this PR, separate ChunkedStream |
3413 bytes | +484 (+16.5%) |
this PR, merged into BodyStream |
3239 bytes | +310 (+10.6%) |
Merging the two classes saves 174 bytes (-5.1%) vs the separate-class version.
Unix tests pass. ESP32 (ESP32_GENERIC over WiFi) re-tested against httpbin chunked endpoints (.content on /stream/2, readinto on /stream-bytes/64).
Trailers are still discarded and chunk extensions are still ignored. The Content-Length code path is unchanged from master.
There was a problem hiding this comment.
Follow-up in commit 4dafe6c: further reduced bytecode by using a -1 sentinel for chunked mode, a single read() loop, and -1 between chunks after CRLF.
__init__.mpy sizes measured with mpy-cross against upstream/master:
| Version | __init__.mpy |
vs master |
|---|---|---|
| master (no chunked) | 2922 bytes | - |
merged BodyStream (4822856) |
3210 bytes | +288 (+9.9%) |
| current (4dafe6c) | 3178 bytes | +256 (+8.8%) |
Unix tests and ESP32 chunked tests against httpbin still pass.
There was a problem hiding this comment.
One more small size tweak in 09f756e: unified the Content-Length and chunked EOF error string in BodyStream.read() / readinto().
__init__.mpy (mpy-cross vs upstream/master):
Unix and ESP32 chunked tests still pass. Chunk extensions still supported.
There was a problem hiding this comment.
Thanks for optimizing! See if you can tweak it further. Saving even 1 or 2 bytes is a win.
There was a problem hiding this comment.
Thanks again for the review. I use MicroPython in production, so I am well acquainted with the tragedy of every spare byte - I still count them anyway, mostly for sport.
Follow-up commits since your size feedback:
| Commit | __init__.mpy |
vs master | vs previous |
|---|---|---|---|
| master (no chunked) | 2917 | - | - |
| 09f756e (unified EOF string) | 3122 | +205 | - |
b98e90c (drop redundant bytes(), inline chunk size parse) |
3109 | +192 | -13 |
| latest (ruff format fix) | 3109 | +192 | 0 |
The memoryview slice spacing is ruff-required; mpy-cross measures the same 3109 bytes with or without that space, so the -13 byte win is from the other two tweaks only.
Unix tests pass. Chunk extensions still supported.
ESP32 (ESP32_GENERIC over WiFi) re-tested against httpbin:
.contenton/stream/2(372 bytes, 2 lines)readintoon/stream-bytes/64(64 bytes total)
All passed.
Combine ChunkedStream with BodyStream to reduce __init__.mpy size while keeping incremental read/readinto behaviour for chunked responses. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Use a -1 sentinel for chunked mode, a single read loop, and -1 between chunks after CRLF to shrink __init__.mpy while keeping behaviour. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Use one error string for Content-Length and chunked read paths to reduce __init__.mpy size without changing behaviour. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Drop redundant bytes() wrappers, inline chunk size readline parse, and tighten memoryview slice spacing to reduce __init__.mpy by 13 bytes. Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Restore spaced memoryview slice for ruff format; __init__.mpy stays at 3109 bytes (+192 vs master). Signed-off-by: Pablo Ventura <pablogventura@gmail.com>
Summary
Follow-up to #1124, which added HTTP/1.1 with Content-Length streaming and
raised a ValueError on
Transfer-Encoding: chunkedresponses. This PR addsincremental decoding of chunked response bodies so
requestscan consume thecommon case of servers that stream without a Content-Length.
A small
ChunkedStreamwrapper decodes the chunk framing on the fly. It keepsthe existing behaviour intact:
.raw.read(n)and.raw.readinto(buf)stream the decoded body withoutbuffering everything in
request()(works formip)..contentstays lazy and materialises the full body on demand.BodyStream(Content-Length) is untouched; chunked responses selectChunkedStreaminstead.Chunk extensions after the size are skipped and trailers are discarded.
Testing
micropython python-ecosys/requests/test_requests.pypasses,including a replaced test that decodes a chunked response via
.contentanda new test that consumes it via
.raw.readinto().http://httpbin.org/stream/2(decoded via.content) andhttp://httpbin.org/stream-bytes/64(consumed via.raw.readinto()with a16-byte buffer). Both returned the expected bytes.
Trade-offs and Alternatives
.contentstill materialises the whole body, consistent with the existingcontract;
readinto()is the incremental path.Generative AI
I used generative AI tools when creating this PR, but a human has checked the
code and is responsible for the code and the description above.