Skip to content

HTTP/1: request with \n\r\n head terminator is accepted only when it arrives in a single read #4145

Description

@adwait

Version

hyper 1.11.0, httparse 1.10.1

Platform

Linux 6.8.0-1057-aws x86_64 (Ubuntu 22.04), rustc 1.96.0

Summary

is_complete_fast (src/proto/h1/role.rs:100-115) recognizes \r\n\r\n and \n\n as the end of the header block, but not \n\r\n — a header line ended by a bare LF, followed by a CRLF empty line. httparse, which performs the real parse, does accept a bare LF as a line terminator.

Because the fast scan is consulted only after a partial read (role.rs:88-94), the two paths disagree:

  • arrives in one read → scan skipped → httparse frames the request → accepted
  • arrives in two reads → scan runs → reports the head incomplete → hyper waits for bytes that never come → IncompleteMessage

Since the read split is chosen by the kernel, acceptance of the affected input is non-deterministic in production.

Code Sample

Predicate level — copy is_complete_fast verbatim and call it on the witness. It returns false for every prev_len, though the buffer holds a complete head that httparse parses:

let buf = b"GET /a HTTP/1.1\r\nHost: h\n\r\n";   // 27 bytes, head terminator \n\r\n
assert!((1..buf.len()).all(|p| !is_complete_fast(buf, p)));  // passes — never reported complete
Full standalone end-to-end reproduction (no async runtime; cargo run)
[dependencies]
hyper = { version = "1.11", features = ["server", "http1"] }
http-body-util = "0.1"
bytes = "1"
use bytes::Bytes;
use http_body_util::Full;
use hyper::body::Incoming;
use hyper::rt::{Read, ReadBufCursor, Write};
use hyper::service::service_fn;
use hyper::{Request, Response};
use std::cell::RefCell;
use std::collections::VecDeque;
use std::future::Future;
use std::pin::Pin;
use std::rc::Rc;
use std::task::{Context, Poll, Waker};

struct Shared { chunks: VecDeque<Vec<u8>>, closed: bool }
struct ScriptedIo(Rc<RefCell<Shared>>);

impl Read for ScriptedIo {
    fn poll_read(self: Pin<&mut Self>, _cx: &mut Context<'_>, mut buf: ReadBufCursor<'_>)
        -> Poll<std::io::Result<()>>
    {
        let mut s = self.0.borrow_mut();
        match s.chunks.pop_front() {
            Some(c) => { let n = c.len().min(buf.remaining()); buf.put_slice(&c[..n]); Poll::Ready(Ok(())) }
            None if s.closed => Poll::Ready(Ok(())),   // zero-fill == EOF
            None => Poll::Pending,
        }
    }
}

impl Write for ScriptedIo {
    fn poll_write(self: Pin<&mut Self>, _: &mut Context<'_>, b: &[u8]) -> Poll<std::io::Result<usize>> { Poll::Ready(Ok(b.len())) }
    fn poll_flush(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<std::io::Result<()>> { Poll::Ready(Ok(())) }
    fn poll_shutdown(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<std::io::Result<()>> { Poll::Ready(Ok(())) }
}

fn run(chunks: Vec<Vec<u8>>) -> (Vec<String>, Option<String>) {
    let shared = Rc::new(RefCell::new(Shared { chunks: chunks.into_iter().collect(), closed: false }));
    let seen = Rc::new(RefCell::new(Vec::<String>::new()));
    let sink = Rc::clone(&seen);
    let svc = service_fn(move |req: Request<Incoming>| {
        sink.borrow_mut().push(req.uri().to_string());
        async { Ok::<_, std::convert::Infallible>(Response::new(Full::new(Bytes::from_static(b"ok")))) }
    });

    let mut builder = hyper::server::conn::http1::Builder::new();
    builder.auto_date_header(false);
    let mut fut = Box::pin(builder.serve_connection(ScriptedIo(Rc::clone(&shared)), svc));
    let mut cx = Context::from_waker(Waker::noop());
    let mut error = None;

    // Phase 0 delivers the bytes and holds the connection open; phase 1 closes the
    // peer's half, so a request already parsed still gets dispatched.
    'outer: for phase in 0..2 {
        if phase == 1 { shared.borrow_mut().closed = true; }
        for _ in 0..10_000 {
            match fut.as_mut().poll(&mut cx) {
                Poll::Ready(Ok(())) => break 'outer,
                Poll::Ready(Err(e)) => { error = Some(e.to_string()); break 'outer; }
                Poll::Pending => { if shared.borrow().chunks.is_empty() { break; } }
            }
        }
    }
    let framed = seen.borrow().clone();
    (framed, error)
}

fn main() {
    let s: &[u8] = b"GET /a HTTP/1.1\r\nHost: h\n\r\n";
    println!("one read  -> {:?}", run(vec![s.to_vec()]));
    for split in 1..s.len() {
        let (framed, err) = run(vec![s[..split].to_vec(), s[split..].to_vec()]);
        assert!(framed.is_empty(), "split {split} framed {framed:?}");
        if split == 1 { println!("split at 1 -> framed {framed:?}, error {err:?}"); }
    }
    println!("all {} two-way splits framed nothing", s.len() - 1);
}

Output:

one read  -> (["/a"], None)
split at 1 -> framed [], error Some("connection closed before message completed")
all 26 two-way splits framed nothing

Expected Behavior

Framing depends on the bytes, not on how the transport delivered them. hyper accepts bare-LF line endings when the whole request arrives at once, so it should accept them when the request arrives split — or reject them in both cases. Either is defensible; differing by read boundary is not.

Actual Behavior

Delivery Result
one read (all 27 bytes) 1 request framed, no error
any two reads — all 26 split offsets 0 requests, connection closed before message completed
one byte per read 0 requests, same error

Additional Context

Scope. Of the four ways the two line endings can combine, exactly one misbehaves — measured exhaustively over every split point:

last header line empty line is_complete_fast ever true? end to end
CRLF CRLF yes fine under every split
LF LF yes fine under every split
CRLF LF yes fine under every split
LF CRLF never accepted whole, rejected on every split

Independent of the rest of the message: reproduces with no headers, with several headers, and with a Content-Length body.

Suggested fix. The narrow fix is to add the \n\r\n case to the scan.

The broader one is to make the scan conservative: on anything it does not positively recognize, fall through to T::parse rather than returning Ok(None). It exists only to skip a full parse on a slow connection, so a false negative costs one parse attempt while a false positive costs a dropped request.

That may be worth weighing given the history — #3811 was also is_complete_fast misbehaving on a split delivery. Both share a shape: the fast path is consulted only when a message arrives in pieces, so any disagreement it has with the real parser is invisible to whole-delivery testing. A conservative scan cannot disagree.

Happy to open a PR for either, but I'd rather hear which you prefer first.

Related. #3764 introduced is_complete_fast. #3811 was a panic in the same function on a broken-up 1xx response — same trigger class, different defect; the current bounds-safe chunks(3) form is that fix, and it did not add \n\r\n. I searched for is_complete_fast, bare LF header, IncompleteMessage partial read, and line ending LF CRLF headers and found no existing report of this one — apologies if I missed it.

How this was found. By replaying a corpus of HTTP/1 byte streams under every possible split and comparing what got framed — a property test for "framing must not depend on read boundaries", derived from a formal model of HTTP/1 message framing. This came out of verification work at Aretta; the other 62 streams in the corpus were perfectly chunking-invariant, which is a nice result for hyper.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions