Surprising behavior of Take with misbehaved inner reader

[HeroicKatora]

Nothing here is unsound, just surprising. I tried this code:

use std::io::Read;

struct LieLieLie(bool);

impl Read for LieLieLie {
    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
        if core::mem::take(&mut self.0) {
            // Take my hand and let's end it all
            Ok(buf.len() + 1)
        } else {
            Ok(buf.len())
        }
    }
}

fn main() {
    let mut buffer = vec![0; 4];
    let mut reader = LieLieLie(true).take(4);
    // Primed the `Limit` by lying about the read size.
    let _ = reader.read(&mut buffer[..]);
    // Oops, limit is now u64::MAX.
    reader.read_to_end(&mut buffer);
}

I expected to see this happen: The wrapping into Take ensures that no more than the specified number are appended to the underlying vector.

Instead, this happened: read_to_end enters a very long loop, eventually fails to allocate and crashes.

Meta

rustc --version --verbose:

1.61.0-nightly (2022-03-14 285fa7ecd05dcbfdaf2f)

The standard library includes this code:

rust/library/std/src/io/mod.rs

Lines 2561 to 2562 in 83460d5

	let n = self.inner.read(&mut buf[..max])?;
	self.limit -= n as u64;

impl<T: Read> Read for Take<T> {
    fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
// …
        let n = self.inner.read(&mut buf[..max])?;
        self.limit -= n as u64;

When the inner: T is misbehaved then n may end up larger than max, causing a wrapping subtraction. A remedy may be changing this to a saturating subtraction.

[HeroicKatora]

This is especially surprising because BufRead::consumes protects against this kind of misuse by the caller:

    fn consume(&mut self, amt: usize) {
        // Don't let callers reset the limit by passing an overlarge value
        let amt = cmp::min(amt as u64, self.limit) as usize;
        self.limit -= amt as u64;
        self.inner.consume(amt);
    }

[frank-king]

@rustbot claim