LengthDelimitedCodec example not correct in Document

[ygf11]

tokio-util version
0.6.8.

Platform
Darwin MacBook-Pro.local 19.6.0 Darwin Kernel Version 19.6.0: Thu Oct 29 22:56:45 PDT 2020; root:xnu-6153.141.2.2~1/RELEASE_X86_64 x86_64

Description

I find some examples in LengthDelimitedCodec seems not correct.
First:

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

Lines 76 to 101 in 44e9013

	//! ## Example 1
	//!
	//! 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_util::codec::LengthDelimitedCodec;
	//! # fn bind_read<T: AsyncRead>(io: T) {
	//! LengthDelimitedCodec::builder()
	//! .length_field_offset(0) // default value
	//! .length_field_length(2)
	//! .length_adjustment(0) // default value
	//! .num_skip(0) // Do not strip frame header
	//! .new_read(io);
	//! # }
	//! # 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 |
	//! +----------+---------------+ +----------+---------------+

I run this example, the decoded result is \x00 \x0B Hello wor, which in the example should \x00\x0B Hello world ， the issue is that the length does not contain the length of the header, or we can just set length_adjustment.

Second:

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

Lines 141 to 168 in 44e9013

	//! ## Example 3
	//!
	//! The following will parse a `u16` length field at offset 0, including the
	//! frame head in the yielded `BytesMut`. In this case, the length field
	//! **includes** the frame head length.
	//!
	//! ```
	//! # use tokio::io::AsyncRead;
	//! # use tokio_util::codec::LengthDelimitedCodec;
	//! # fn bind_read<T: AsyncRead>(io: T) {
	//! LengthDelimitedCodec::builder()
	//! .length_field_offset(0) // default value
	//! .length_field_length(2)
	//! .length_adjustment(-2) // size of head
	//! .num_skip(0)
	//! .new_read(io);
	//! # }
	//! # 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 |
	//! +----------+---------------+ +----------+---------------+
	//! ```

My decoded result is \x00\x0D Hello wor, which in the example is \x00\x0D Hello world.

Third:

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

Lines 176 to 206 in 44e9013

	//! ## Example 4
	//!
	//! The following will parse a 3 byte length field at offset 0 in a 5 byte
	//! frame head, including the frame head in the yielded `BytesMut`.
	//!
	//! ```
	//! # use tokio::io::AsyncRead;
	//! # use tokio_util::codec::LengthDelimitedCodec;
	//! # fn bind_read<T: AsyncRead>(io: T) {
	//! LengthDelimitedCodec::builder()
	//! .length_field_offset(0) // default value
	//! .length_field_length(3)
	//! .length_adjustment(2) // remaining head
	//! .num_skip(0)
	//! .new_read(io);
	//! # }
	//! # pub fn main() {}
	//! ```
	//!
	//! The following frame will be decoded as such:
	//!
	//! ```text
	//! INPUT
	//! +---- len -----+- head -+--- Payload ---+
	//! | \x00\x00\x0B | \xCAFE | Hello world |
	//! +--------------+--------+---------------+
	//!
	//! DECODED
	//! +---- len -----+- head -+--- Payload ---+
	//! | \x00\x00\x0B | \xCAFE | Hello world |
	//! +--------------+--------+---------------+

My decoded result is \x00\x00\x0B\xCA\xFE Hello wo, which in document is \x00\x00\x0B \xCA\xFE Hello world.

---

[bIgBV]

@Darksonn I can take a look at this one this week.

[Rustin170506]

The results of my tests look as if they are correct.

My code:

use bytes::Bytes;
use futures::SinkExt;

use tokio::fs::File;
use tokio::io::{AsyncRead, AsyncWrite};
use tokio_util::codec::{Framed, LengthDelimitedCodec};

async fn write_frame<T>(io: T) -> Result<(), Box<dyn std::error::Error>>
where
    T: AsyncRead + AsyncWrite + Unpin,
{
    let codec = LengthDelimitedCodec::builder()
        .length_field_offset(0) // default value
        .length_field_length(2)
        .length_adjustment(-2) // size of head
        .num_skip(0) // Do not strip frame header
        .new_codec();
    let mut transport = Framed::new(io, codec);
    let frame = Bytes::from("hello world");

    print!("{:?}", frame);
    transport.send(frame).await?;
    Ok(())
}

#[tokio::main]
async fn main() {
    let file = File::create("foo.txt").await.unwrap();
    write_frame(file).await.unwrap();
}

[matthiasdebernardini]

Is this issue still open? I can't figure out what the problem is with the original issue. The docs appears to be correct, curious what the issue was.

[Darksonn]

@matthiasdebernardini Not sure. Has the file changed since this issue?

[matthiasdebernardini]

yes it has, length_field_length(2) was changed to .length_field_type::<u16>() in examples 1, 3 and 4. Looks like the builder API has changed since then?

The example output from @hi-rustin outputs b"hello world"11% regardless if I use length_field_length(2) or .length_field_type::<u16>()

I'm just going through these "good first issues" so I can learn the project better.

[Darksonn]

If this issue has been fixed, then we should close it.

[ygf11]

Sorry for response later, I close it now.

[vnermolaev]

The results of my tests look as if they are correct.

My code:

use bytes::Bytes;
use futures::SinkExt;

use tokio::fs::File;
use tokio::io::{AsyncRead, AsyncWrite};
use tokio_util::codec::{Framed, LengthDelimitedCodec};

async fn write_frame<T>(io: T) -> Result<(), Box<dyn std::error::Error>>
where
    T: AsyncRead + AsyncWrite + Unpin,
{
    let codec = LengthDelimitedCodec::builder()
        .length_field_offset(0) // default value
        .length_field_length(2)
        .length_adjustment(-2) // size of head
        .num_skip(0) // Do not strip frame header
        .new_codec();
    let mut transport = Framed::new(io, codec);
    let frame = Bytes::from("hello world");

    print!("{:?}", frame);
    transport.send(frame).await?;
    Ok(())
}

#[tokio::main]
async fn main() {
    let file = File::create("foo.txt").await.unwrap();
    write_frame(file).await.unwrap();
}

It seems that there is no method send in Framed, however, there is start_send.
I find the docs for start_send a bit cryptic: before each call of start_send one needs to call poll_ready until it returns a certain result. How do I need to call it? in an infinite loop? should I arrange my own delay? Why is this behaviour not hidden behind, say, send?

Update: it is imperative to have import use futures::SinkExt;

[Darksonn]

I find the docs for start_send a bit cryptic

The start_send method should only be used from poll methods. You should use the SinkExt methods instead if you're writing async code.