Hyper test would fail if an existing bug in tokio-util's `LengthDelimitedCodec` is fixed
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 1.6k
- Forks
- 382
- Avg merge
- 20h 23m
- Merged PRs (30d)
- 9
Description
Description
The current LengthDelimitedCodec implementation has a bug (tokio-rs/tokio#4815) where length adjustment is not applied before checking overflow. And the PR that tries to fix the bug (tokio-rs/tokio#4816) currently has failed h2_connect_large_body test with hyper because of the way LengthDelimitedCodec is used in h2 right now.
Suspected cause
The cause of the problem is that FramedRead internally uses a LengthDelimitedCodec while FramedWrite uses a custom encoder. Though the LengthDelimitedCodec in FramedRead has a length_adjustment of 9, the encoder doesn't seem to make adjustment to the encoded length. For a length adjustment of 9 and a real max_frame_length of N, the LengthDelimitedCodec would expect an encoded length of N-9 (ie. if the encodec length is N-9, the codec would read N-9+9=N bytes). Thus if the encoded length is N, the codec would try to read N+9 which exceeds the max_frame_length and would cause an error.
One of the newly added unit test for the above mentioned PR is attached below as an example
#[test]
fn read_single_frame_positive_length_adjusted_and_max_sized() {
let mut d: Vec<u8> = vec![];
d.extend_from_slice(b"\x00\x00\x00\x07Hello world");
let io = length_delimited::Builder::new()
.length_field_offset(0)
.length_field_length(4)
.max_frame_length(11)
.length_adjustment(4)
.new_read(mock! {
data(&d),
});
pin_mut!(io);
assert_next_eq!(io, b"Hello world");
assert_done!(io);
}
Workaround (a sketchy one)
This can be circumvented if an additional 9 is added when setting the decoders max_frame_length ie.
/// Updates the max frame size setting.
///
/// Must be within 16,384 and 16,777,215.
#[inline]
pub fn set_max_frame_size(&mut self, val: usize) {
assert!(DEFAULT_MAX_FRAME_SIZE as usize <= val && val <= MAX_MAX_FRAME_SIZE as usize);
self.inner.decoder_mut().set_max_frame_length(val + 9)
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the h2_connect_large_body test and the decoder set_max_frame_size method shown in the issue. Trace how FramedRead uses LengthDelimitedCodec alongside the custom FramedWrite encoder, then verify behavior against the linked tokio-util overflow fix. Done means the large-body test passes without relying on the described workaround.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100