Repository: tokio-rs/tokio (language: rust)

Issue:
LengthDelimitedCodec misses that last N bytes of the frame with num_skip(0)
**Version**

tokio master branch (14c17fc09656a30230177b600bacceb9db33e942, tokio 1.38.0), also tokio-util 0.7.11.

**Platform**

Linux

**Description**

The [documentation](https://docs.rs/tokio-util/latest/tokio_util/codec/length_delimited/index.html#example-1) gives this example:

```rust
LengthDelimitedCodec::builder()
    .length_field_offset(0) // default value
    .length_field_type::<u16>()
    .length_adjustment(0)   // default value
    .num_skip(0) // Do not strip frame header
    .new_read(io);
```

The following frame will be decoded as such:

```rust
         INPUT                           DECODED
+-- len ---+--- Payload ---+     +-- len ---+--- Payload ---+
| \x00\x0B |  Hello world  | --> | \x00\x0B |  Hello world  |
+----------+---------------+     +----------+---------------+
```

However, the following (new) test fails in tokio-util/tests/length_delimited.rs:

```rust
#[test]
fn read_single_frame_skip_none() {
    let mut d: Vec<u8> = vec![];
    d.extend_from_slice(b"\x00\x0BHello world");

    let io = length_delimited::Builder::new()
        .length_field_offset(0)
        .length_field_type::<u16>()
        .length_adjustment(0)
        .num_skip(0)
        .new_read(mock! {
            data(&d),
        });
    pin_mut!(io);

    assert_next_eq!(io, b"\x00\x0BHello world");
    assert_done!(io);
}
```

Error message:

```
thread 'read_single_frame_skip_none' panicked at tokio-util/tests/length_delimited.rs:389:5:
assertion `left == right` failed
  left: b"\0\x0bHello wor"
 right: [0, 11, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100]
```

The last two bytes of "world" is missing. With a longer size prefix (u32), the number of missing byte increases (4). It appears to me that the logic that reads the size advances the pointer, but this is incorrectly adjusted later, when num_skip(0) is taken into account.

A possible workaround:

```
        .length_adjustment(2) # 2 must match the size prefix size.
```


Discussion / hints:
According to this test, it seems that use `length_adjustment` to adjust result length is an expected behavior when `.num_skip(0)` is set.

https://github.com/tokio-rs/tokio/blob/16fccafb416eb7dd0e324114886cf94021e208ff/tokio-util/tests/length_delimited.rs#L351-L372

We may need to update the example in documentation.

Fix patch:
diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs
index c37f5863bd4..c1f24167a06 100644
--- a/tokio-util/src/codec/length_delimited.rs
+++ b/tokio-util/src/codec/length_delimited.rs
@@ -75,30 +75,32 @@
 //!
 //! ## Example 1
 //!
-//! The following will parse a `u16` length field at offset 0, including the
+//! The following will parse a `u16` length field at offset 0, omitting the
 //! frame head in the yielded `BytesMut`.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\x00\x0BHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(0) // default value
 //!     .length_field_type::<u16>()
 //!     .length_adjustment(0)   // default value
-//!     .num_skip(0) // Do not strip frame header
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"Hello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:
 //!
 //! ```text
-//!          INPUT                           DECODED
-//! +-- len ---+--- Payload ---+     +-- len ---+--- Payload ---+
-//! | \x00\x0B |  Hello world  | --> | \x00\x0B |  Hello world  |
-//! +----------+---------------+     +----------+---------------+
+//!          INPUT                        DECODED
+//! +-- len ---+--- Payload ---+     +--- Payload ---+
+//! | \x00\x0B |  Hello world  | --> |  Hello world  |
+//! +----------+---------------+     +---------------+
 //! ```
 //!
 //! The value of the length field is 11 (`\x0B`) which represents the length
@@ -109,62 +111,69 @@
 //!
 //! ## Example 2
 //!
-//! The following will parse a `u16` length field at offset 0, omitting the
+//! The following will parse a `u16` length field at offset 0, including the
 //! frame head in the yielded `BytesMut`.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\x00\x0BHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(0) // default value
 //!     .length_field_type::<u16>()
-//!     .length_adjustment(0)   // default value
-//!     // `num_skip` is not needed, the default is to skip
+//!     .length_adjustment(2)   // Add head size to length
+//!     .num_skip(0)            // Do NOT skip the head
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"\x00\x0BHello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:
 //!
 //! ```text
-//!          INPUT                        DECODED
-//! +-- len ---+--- Payload ---+     +--- Payload ---+
-//! | \x00\x0B |  Hello world  | --> |  Hello world  |
-//! +----------+---------------+     +---------------+
+//!          INPUT                           DECODED
+//! +-- len ---+--- Payload ---+     +-- len ---+--- Payload ---+
+//! | \x00\x0B |  Hello world  | --> | \x00\x0B |  Hello world  |
+//! +----------+---------------+     +----------+---------------+
 //! ```
 //!
 //! This is similar to the first example, the only difference is that the
-//! frame head is **not** included in the yielded `BytesMut` value.
+//! frame head is **included** in the yielded `BytesMut` value. To achieve
+//! this, we need to add the header size to the length with `length_adjustment`,
+//! and set `num_skip` to `0` to prevent skipping the head.
 //!
 //! ## Example 3
 //!
-//! The following will parse a `u16` length field at offset 0, including the
+//! The following will parse a `u16` length field at offset 0, omitting the
 //! frame head in the yielded `BytesMut`. In this case, the length field
 //! **includes** the frame head length.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\x00\x0DHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(0) // default value
 //!     .length_field_type::<u16>()
 //!     .length_adjustment(-2)  // size of head
-//!     .num_skip(0)
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"Hello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:
 //!
 //! ```text
 //!          INPUT                           DECODED
-//! +-- len ---+--- Payload ---+     +-- len ---+--- Payload ---+
-//! | \x00\x0D |  Hello world  | --> | \x00\x0D |  Hello world  |
-//! +----------+---------------+     +----------+---------------+
+//! +-- len ---+--- Payload ---+     +--- Payload ---+
+//! | \x00\x0D |  Hello world  | --> |  Hello world  |
+//! +----------+---------------+     +---------------+
 //! ```
 //!
 //! In most cases, the length field represents the length of the payload
@@ -179,17 +188,20 @@
 //! frame head, including the frame head in the yielded `BytesMut`.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\x00\x00\x0B\xCA\xFEHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(0) // default value
 //!     .length_field_length(3)
-//!     .length_adjustment(2)  // remaining head
+//!     .length_adjustment(3 + 2)  // len field and remaining head
 //!     .num_skip(0)
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"\x00\x00\x0B\xCA\xFEHello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:
@@ -223,17 +235,20 @@
 //! included.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\xCA\x00\x0B\xFEHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(1) // length of hdr1
 //!     .length_field_type::<u16>()
 //!     .length_adjustment(1)  // length of hdr2
 //!     .num_skip(3) // length of hdr1 + LEN
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"\xFEHello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:
@@ -269,15 +284,19 @@
 //! length.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\xCA\x00\x0F\xFEHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(1) // length of hdr1
 //!     .length_field_type::<u16>()
 //!     .length_adjustment(-3)  // length of hdr1 + LEN, negative
 //!     .num_skip(3)
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"\xFEHello world");
 //! # }
 //! ```
 //!
@@ -308,17 +327,20 @@
 //! frame head, excluding the 4th byte from the yielded `BytesMut`.
 //!
 //! ```
-//! # use tokio::io::AsyncRead;
+//! # use tokio_stream::StreamExt;
 //! # use tokio_util::codec::LengthDelimitedCodec;
-//! # fn bind_read<T: AsyncRead>(io: T) {
-//! LengthDelimitedCodec::builder()
+//! # #[tokio::main]
+//! # async fn main() {
+//! # let io: &[u8] = b"\x00\x00\x0B\xFFHello world";
+//! let mut reader = LengthDelimitedCodec::builder()
 //!     .length_field_offset(0) // default value
 //!     .length_field_length(3)
 //!     .length_adjustment(0)  // default value
 //!     .num_skip(4) // skip the first 4 bytes
 //!     .new_read(io);
+//! # let res = reader.next().await.unwrap().unwrap().to_vec();
+//! # assert_eq!(res, b"Hello world");
 //! # }
-//! # pub fn main() {}
 //! ```
 //!
 //! The following frame will be decoded as such:


Test patch: