futures-rs icon indicating copy to clipboard operation
futures-rs copied to clipboard

Proposal: add `AsyncReadExt::by_ref`

Open jorgecarleitao opened this issue 3 years ago • 4 comments

Hi,

When reading a known number of bytes from futures::io::ReadAsync, it is useful to use the idiom

let mut a = vec![];
a.try_reserve(length0)?;
reader
    .by_ref()
    .take(length0 as u64)
    .read_to_end(a)
    .await?;

a.clear();
a.try_reserve(length1)?;
reader
    .by_ref()
    .take(length1 as u64)
    .read_to_end(a)
    .await?;

however, this is currently not possible because by_ref does not exist like Read::by_ref. This makes it impossible to implement the idiom above.

The proposal is to add such method. Alternatively, any way of doing the above? ^^

jorgecarleitao avatar Jun 29 '22 13:06 jorgecarleitao

Since AsyncRead is implemented on &mut AsyncRead, I think you can use the less idiomatic version:

(&mut reader)
    .take(n)
    .read_to_end(&mut buf)
    .await?

That being said, if the goal is to read n bytes, why don't you just use read_all?

let mut buf = vec![0u8; length0];
reader.read_all(&mut buf).await?

notgull avatar Jun 29 '22 13:06 notgull

awesome, thanks!

That being said, if the goal is to read n bytes, why don't you just use read_all?

I can't find read_all it in the docs. Regardless, what we are trying to avoid is to initialize the vec with zeros and instead read to uninitialized. More on this here (for std::io::Read): https://users.rust-lang.org/t/how-to-efficiently-re-use-a-vec-u8-for-multiple-read-read/77628/6

jorgecarleitao avatar Jun 29 '22 13:06 jorgecarleitao

The reason why we do not have this and it is difficult to add is because it may conflict with StreamExt::by_ref.

It is possible to add this in 0.4 as a breaking change, but I sometimes see code that implements both Stream and AsyncRead, so I feel the cost of the breakage from adding this would not exceed the benefit unless something like a priority for trait resolution is implemented.

taiki-e avatar Jul 02 '22 08:07 taiki-e

reading a known number of bytes from futures::io::ReadAsync

It would be great if this was somehow better supported. In protocols where the remote will close the stream after it is done writing, using read_to_end would be very convenient. However, careless use of it can create a problem if the remote is untrusted: They can just send us data until we run out of memory.

It is possible to write your own loop using take(n).read_to_end and checking on each iteration whether the length of the vector has not yet exceeded some application-defined limit.

Thoughts?

thomaseizinger avatar May 23 '23 08:05 thomaseizinger