Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
seanmonstar opened this issue Apr 26, 2024 · 3 comments
Open

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

seanmonstar opened this issue Apr 26, 2024 · 3 comments

Comments

@seanmonstar
Copy link
Member

seanmonstar commented Apr 26, 2024

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 hyperium/hyper#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.

@Darksonn
Copy link
Contributor

Yeah, this seems like a problem.

@paolobarbolini
Copy link
Contributor

Turns out this was already discovered by #474

@Darksonn
Copy link
Contributor

How about removing the guarantee about chunks_vectored being complete, and adding a new method chunks_vectored_is_complete that returns a boolean describing whether this type provides the guarantee that chunks_vectored always provides all slices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants