bytes icon indicating copy to clipboard operation
bytes copied to clipboard

Buf::chunks_vectored() is wrong if chunk() isn't the whole buf

Open seanmonstar opened this issue 3 months ago • 3 comments

The design of Buf::chunks_vectored() does not account for an implementation that returns a partial slice in chunk() and doesn't override chunks_vectored() to be the full thing. It's assumed that if you have a composite buf, such as Chain, that you can just fill the IoSlices with both.

First reported in https://github.com/hyperium/hyper/issues/3650

Example

use bytes::Buf;
use std::io::IoSlice;

struct Trickle<'a>(&'a [u8]);

impl<'a> Buf for Trickle<'a> {
    fn remaining(&self) -> usize {
        self.0.len()
    }
    
    fn advance(&mut self, n: usize) {
        self.0 = &self.0[n..];
    }
    
    fn chunk(&self) -> &[u8] {
        &self.0[0..1]
    }
}

let ab = Trickle(b"hello").chain(&b"world"[..]);    
    
let mut io = [IoSlice::new(&[]); 16];
let n = ab.chunks_vectored(&mut io);
    
println!("{:?}", &io[..n]);
// [[104], [119, 111, 114, 108, 100]]
// eg [[h], [world]]

Interestingly, the documentation for chunks_vectored even declares a guarantee that likely cannot be upheld:

If chunk_vectored does not fill every entry in dst, then dst is guaranteed to contain all remaining slices in self.

Solutions

With the way the function signature is designed, I don't see a cheap way to check after a call if it really represents all of the buf.

An expensive solution would be to provide a helper function that compares remaining() with what was placed in the &mut [IoSlice], and only if true, pass the rest of the slices to the next buffer in the list.

Another option I can think of is to design a whole new function, chunks_vectored_done_right(), with arguments/return types that prevent making that mistake, deprecating the existing method, and probably changing the default to use the expensive solution anyways.

seanmonstar avatar Apr 26 '24 20:04 seanmonstar