Skip to content

fix(fullstack): buffer streaming frames across physical reads - #5783

Open
andoowhy wants to merge 1 commit into
DioxusLabs:mainfrom
andoowhy:fix/chunked-byte-stream-partial-frame-corruption
Open

fix(fullstack): buffer streaming frames across physical reads#5783
andoowhy wants to merge 1 commit into
DioxusLabs:mainfrom
andoowhy:fix/chunked-byte-stream-partial-frame-corruption

Conversation

@andoowhy

Copy link
Copy Markdown

Streaming<T, E>'s FromRequest/FromResponse decoding (byte_stream_to_client_stream) only looked for a complete length-prefixed frame within whatever bytes a single physical network read handed back, discarding any leftover partial frame instead of carrying it over to the next read. HTTP/TCP give no guarantee that physical chunk boundaries line up with frame boundaries, so any frame larger than a few KB routinely gets split across multiple reads - and each time that happened, the frame surfaced as StreamingError::Decoding instead of being reassembled. This affects ChunkedByteStream, CborStream<T>, and any other Streaming<T, E> with a non-() encoding, on both the server (FromRequest) and client (FromResponse) sides.

I hit this using ChunkedByteStream to stream file blobs from a client to a server: an 8MB blob reliably failed after the first ~8KB arrived, regardless of how the sender chunked its writes (chunking the application-level frames doesn't help, since the transport is free to coalesce/split them independently of that).

Fix

Reworks byte_stream_to_client_stream to accumulate a buffer across as many physical reads as it takes to see one complete frame, only attempting to decode once the buffer actually contains a whole frame - the same approach tokio_util::codec::Decoder/FramedRead use. No public API changes.

Testing

  • Added payloads::stream::tests::frame_split_across_many_physical_chunks_decodes_correctly, which reproduces the bug in-process (no real network needed) by feeding byte_stream_to_client_stream a stream that hands back a single frame's bytes split into 37-byte pieces. This fails with StreamingError::Decoding on current main and passes with this fix.
  • Added payloads::stream::tests::multiple_frames_in_one_physical_chunk_decode_in_order, confirming multiple frames arriving in a single physical chunk still decode correctly and in order (guards against a regression in the buffering rework).
  • cargo test -p dioxus-fullstack --features server passes.
  • cargo fmt --check and cargo clippy --tests are clean (no new warnings).
  • Also validated against a real end-to-end reproduction outside this repo: an axum server using ChunkedByteStream over real loopback TCP, streaming an 8MB payload from a reqwest client - fails on main, passes with this patch.

`Streaming<T, E>`'s `FromRequest`/`FromResponse` decoding
(`byte_stream_to_client_stream`) only looked for a complete length-prefixed
frame within whatever bytes a single physical network read handed back,
discarding any leftover partial frame instead of carrying it over to the
next read. HTTP/TCP give no guarantee that physical chunk boundaries line
up with frame boundaries, so any frame larger than a few KB routinely gets
split across multiple reads - and each time that happened, the frame
surfaced as `StreamingError::Decoding` instead of being reassembled. This
affects `ChunkedByteStream`, `CborStream<T>`, and any other `Streaming<T, E>`
with a non-`()` encoding, on both the server (`FromRequest`) and client
(`FromResponse`) sides.

This reworks `byte_stream_to_client_stream` to accumulate a buffer across
as many physical reads as it takes to see one complete frame before
attempting to decode it, the same pattern `tokio_util::codec::Decoder`
uses, instead of resetting per physical chunk.

Found via a real reproduction: streaming an 8MB blob through
`ChunkedByteStream` over real loopback TCP reliably failed after the
first one or two physical reads; after this fix it round-trips correctly
regardless of size.

Added two regression tests in `payloads::stream::tests`:
- a frame deliberately split into many small physical chunks now decodes
  correctly (fails on the old code with `StreamingError::Decoding`)
- multiple frames packed into a single physical chunk still decode in
  order (unchanged behavior, guards against a regression in the rework)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant