Skip to content

Commit 13af40d

Browse files
committed
fix(http2): preserve body order in vectored writes
Keep SendBuf on Buf's single-chunk default instead of forwarding arbitrary user chunks_vectored output. This keeps the payload contiguous when h2 combines it with frame bytes.
1 parent 4ef4d88 commit 13af40d

1 file changed

Lines changed: 69 additions & 6 deletions

File tree

src/proto/h2/mod.rs

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::error::Error as StdError;
22
use std::future::Future;
3-
use std::io::{Cursor, IoSlice};
3+
use std::io::Cursor;
44
use std::pin::Pin;
55
use std::task::{Context, Poll};
66

@@ -282,6 +282,9 @@ enum SendBuf<B> {
282282
None,
283283
}
284284

285+
// Intentionally use `Buf`'s single-chunk `chunks_vectored` default. Forwarding
286+
// to `B::chunks_vectored` could expose non-contiguous user data to h2's framing
287+
// buffer when `B::chunk()` is only a partial prefix.
285288
impl<B: Buf> Buf for SendBuf<B> {
286289
#[inline]
287290
fn remaining(&self) -> usize {
@@ -309,12 +312,72 @@ impl<B: Buf> Buf for SendBuf<B> {
309312
Self::None => {}
310313
}
311314
}
315+
}
312316

313-
fn chunks_vectored<'a>(&'a self, dst: &mut [IoSlice<'a>]) -> usize {
314-
match *self {
315-
Self::Buf(ref b) => b.chunks_vectored(dst),
316-
Self::Cursor(ref c) => c.chunks_vectored(dst),
317-
Self::None => 0,
317+
#[cfg(test)]
318+
mod tests {
319+
use std::io::IoSlice;
320+
321+
use bytes::{Buf, Bytes};
322+
323+
use super::SendBuf;
324+
325+
struct OneByteAtATime {
326+
data: &'static [u8],
327+
pos: usize,
328+
}
329+
330+
impl Buf for OneByteAtATime {
331+
fn remaining(&self) -> usize {
332+
self.data.len() - self.pos
333+
}
334+
335+
fn chunk(&self) -> &[u8] {
336+
let end = std::cmp::min(self.pos + 1, self.data.len());
337+
&self.data[self.pos..end]
338+
}
339+
340+
fn advance(&mut self, cnt: usize) {
341+
self.pos += cnt;
318342
}
319343
}
344+
345+
fn drain_vectored(mut buf: impl Buf) -> Vec<u8> {
346+
let mut out = Vec::new();
347+
while buf.has_remaining() {
348+
let mut iovs = [IoSlice::new(&[]); 8];
349+
let n = buf.chunks_vectored(&mut iovs);
350+
assert!(n > 0, "chunks_vectored returned 0 while bytes remain");
351+
let mut wrote = 0;
352+
for iov in &iovs[..n] {
353+
out.extend_from_slice(iov);
354+
wrote += iov.len();
355+
}
356+
buf.advance(wrote);
357+
}
358+
out
359+
}
360+
361+
#[test]
362+
fn send_buf_vectored_is_contiguous_prefix() {
363+
let body = OneByteAtATime {
364+
data: b"abc",
365+
pos: 0,
366+
}
367+
.chain(Bytes::from_static(b"XYZ"));
368+
assert_eq!(drain_vectored(SendBuf::Buf(body)), b"abcXYZ");
369+
}
370+
371+
#[test]
372+
fn send_buf_bytes_stays_vectored_with_framing() {
373+
let buf =
374+
Bytes::from_static(b"frame header").chain(SendBuf::Buf(Bytes::from_static(b"hello")));
375+
let mut iovs = [IoSlice::new(&[]); 2];
376+
377+
let n = buf.chunks_vectored(&mut iovs);
378+
379+
assert_eq!(n, 2);
380+
assert_eq!(&*iovs[0], b"frame header");
381+
assert_eq!(&*iovs[1], b"hello");
382+
}
320383
}

0 commit comments

Comments
 (0)