serve_connection_with_upgrades does not enforce its timeout from initial connection, but initial data.

[randomairborne]

Sysinfo: Hyper 1.4.1 on Darwin 23.6.0 root:xnu-10063.141.2~1/RELEASE_ARM64_T6020 arm64

While l was looking into tokio-rs/axum#2741

I tried this code:

use hyper::{body::Incoming, Request, Response};
use hyper_util::rt::{TokioExecutor, TokioIo, TokioTimer};
use std::convert::Infallible;
use std::time::Duration;
use tokio::net::TcpListener;

#[tokio::main]
async fn main() {
    let listener = TcpListener::bind("0.0.0.0:3000").await.unwrap();
    loop {
        let (socket, _remote_addr) = listener.accept().await.unwrap();
        tokio::spawn(async move {
            let socket = TokioIo::new(socket);
            let hyper_service =
                hyper::service::service_fn(move |_request: Request<Incoming>| async {
                    let t: Result<_, Infallible> = Ok(Response::new("test".to_string()));
                    t
                });

            let mut server = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new());

            server
                .http1()
                .timer(TokioTimer::new())
                .header_read_timeout(Duration::from_secs(1));

            if let Err(err) = server.serve_connection_with_upgrades(socket, hyper_service).await {
                eprintln!("failed to serve connection: {err:#}");
            }
        });
    }
}

I expected a TCP connection opened to the server terminated after not sending data within one second, however, the connection was persisted indefinitely, and only terminated after some amount of data was sent. However, if instead of serve_connection_with_upgrades, serve_connection is used, and http1_only() is also set, Hyper exhibits the correct behavior

[seanmonstar]

I suspect it's not the upgrades part, but rather the auto part. Since it's doing an initial read to detect the HTTP version before passing on to the http1 state machine which knows about the timeout.

[randomairborne]

after some further testing, that makes sense. Is this something that hyper-util concerns itself with? Is it not possible to do SlowLoris with HTTP/2.0 in some other way?

[MerlijnW70]

This still reproduces on hyper-util 0.1.20. The root cause is in the auto server builder's version sniffing:

• ReadVersion::poll (src/server/conn/auto/mod.rs) reads the H2 preface in a bare poll_read loop with no timeout, and it's driven before any handoff at let (version, io) = ready!(read_version.poll(cx))?;.
• header_read_timeout is configured on the inner http1 dispatcher, which only becomes active after read_version resolves. So a connection that sends no bytes parks in the sniff read indefinitely. With serve_connection + http1_only() there's no sniff, which is why the timeout works there.

I'd like to fix this and see two reasonable approaches:

A. Contain it in hyper-util. Store a Timer plus a read-version timeout on the auto::Builder and wrap the read_version future in a Sleep, failing the connection if the preface isn't received in time. Single-crate change, but it means either reusing the value set via http1().header_read_timeout() (not currently readable back from the inner builder) or introducing a dedicated knob.

B. Expose it from hyper. Add getters on http1::Builder for the timer and header_read_timeout so hyper-util can apply the user's existing header_read_timeout to the sniff read directly. Cleaner semantics, but spans both crates and adds public API.

Two questions before I open a PR:

1. Should the sniff-phase timeout reuse header_read_timeout, or be its own setting (e.g. read_version_timeout)?
2. Is a hyper-util-only change (A) preferred, or are you open to the small getter additions in hyper (B)?

Happy to implement whichever direction you prefer.