tokio-util's LengthDelimitedCodec doesn't adjust decoded length before checking overflow

[minghuaw]

Version
tokio-util = { version = "0.7.3", features = ["codec"] }

Platform
Windows

Description

When .length_adjustment() is set to a negative value, a encoded max-sized frame cannot be decoded because overflow is checked before making length adjustment to the decoded length.
[short summary of the bug]

I tried this code:

use bytes::Bytes;
use futures_util::{SinkExt, StreamExt};
use tokio_util::codec::LengthDelimitedCodec;

#[tokio::main]
async fn main() {
    // All the configs
    const MAX_FRAME_LENGTH: usize = 10;
    const LENGTH_FIELD_LENGTH: usize = 4;
    const LENGTH_ADJUSTMENT: isize = -4;

    // test write
    let mut writer = vec![];
    let mut framed = LengthDelimitedCodec::builder()
        .big_endian()
        .length_field_length(LENGTH_FIELD_LENGTH)
        .max_frame_length(MAX_FRAME_LENGTH)
        .length_adjustment(LENGTH_ADJUSTMENT)
        .new_write(&mut writer);

    let payload = Bytes::from(vec![b'a'; MAX_FRAME_LENGTH]);
    framed.send(payload).await.unwrap();

    // test read
    let reader = &writer[..];
    let mut framed = LengthDelimitedCodec::builder()
        .big_endian()
        .length_field_length(LENGTH_FIELD_LENGTH)
        .max_frame_length(MAX_FRAME_LENGTH)
        .length_adjustment(LENGTH_ADJUSTMENT)
        .new_read(reader);
    let outcome = framed.next().await.unwrap().unwrap(); // This panics if `LENGTH_ADJUSTMENT` is negative
    assert_eq!(MAX_FRAME_LENGTH, outcome.len());
}

I expected to see this happen:

I expect that the codec is able to decode the bytes that are encoded by essentially the same codec.

Instead, this happened:

The decoding would fail when it checks for overflow

tokio/tokio-util/src/codec/length_delimited.rs

Line 496 in 4daeea8

if n > self.builder.max_frame_len as u64 {

[minghuaw]

I believe this is caused by the decoder checking overflow

tokio/tokio-util/src/codec/length_delimited.rs

Line 496 in 4daeea8

if n > self.builder.max_frame_len as u64 {

before making length adjustment

tokio/tokio-util/src/codec/length_delimited.rs

Line 508 in 4daeea8

n.checked_sub(-self.builder.length_adjustment as usize)

[minghuaw]

A PR #4816 has been submitted to solve this issue. A new unit test covering this scenario is also added in the PR

[Darksonn]

This is blocked because fixing it is a breaking change. We will release this change next time we make a breaking release of tokio-util. See #4816 for more details.

[rukmal]

+1 on this.