bytes
bytes copied to clipboard
Docs: `Buf::chunk` should have more restrictions.
This issue could also be a sub-issue of #178.
According to the docs of Buf::chunk
:
https://github.com/tokio-rs/bytes/blob/8cc940779fd6a489a2d7ca53fbbc44f84210083e/src/buf/buf_impl.rs#L142-L150
It only requires the implementer to return an empty slice when Buf::remaining
returns 0, but a non-continuous implementation of Buf::chunk
can return an empty slice even if Buf::remaining
> 0.
For example:
pub struct BufVecDeque<B> {
inner: VecDeque<B>
}
impl<B> Buf for BufVecDeque<B> where B: Buf {
fn remaining(&self) -> usize {
self.inner.iter().map(|b| b.remaining()).sum()
}
fn chunk(&self) -> &[u8] {
self.inner.front().map_or(b"", |b| b.chunk())
}
fn advance(&mut self, mut cnt: usize) {
while cnt > 0 {
let Some(front) = self.inner.front_mut() else {
return;
};
let min = std::cmp::min(front.remaining(), cnt);
front.advance(min);
cnt -= min;
if !front.has_remaining() {
self.inner.pop_front();
}
}
}
}
impl<B> BufVecDeque<B> {
pub fn new() -> Self {
Self {
inner: VecDeque::new(),
}
}
pub fn push_back(&mut self, buf: B) {
self.inner.push_back(buf)
}
}
The problem is when you call some methods like Buf::copy_to_slice
or Buf::copy_to_bytes
, it will cause an infinite loop.
Let's see the default implementations of Buf::copy_to_slice
:
https://github.com/tokio-rs/bytes/blob/8cc940779fd6a489a2d7ca53fbbc44f84210083e/src/buf/buf_impl.rs#L268-L282
The let src = self.chunk();
causes the problem, and its docs does not remind the implementer to override its behavior:
https://github.com/tokio-rs/bytes/blob/8cc940779fd6a489a2d7ca53fbbc44f84210083e/src/buf/buf_impl.rs#L247-L268
The Buf::copy_to_bytes
has the same problem.
As for some methods like Buf::get_u8
, they panic:
https://github.com/tokio-rs/bytes/blob/8cc940779fd6a489a2d7ca53fbbc44f84210083e/src/buf/buf_impl.rs#L300-L307
let ret = self.chunk()[0];
cause panic, and its docs only says: "This function panics if there is no more remaining data in self
.":
https://github.com/tokio-rs/bytes/blob/8cc940779fd6a489a2d7ca53fbbc44f84210083e/src/buf/buf_impl.rs#L284-L300
But it also panics when Buf::remaining
> 0.
In fact, the problem is already mentioned in https://github.com/tokio-rs/bytes/issues/178#issuecomment-458213222.
These are all because the Buf::chunk
can return an empty slice even if Buf::remaining
> 0.
Solutions
A quick fix is restricting the Buf::chunk
to return an empty slice if and only if Buf::remaining
returns 0, but it also changes the semantics of Buf::chunk
, should it be considered a breaking change? I can open a PR if the change is acceptable.
Any more ideas?