Skip to content

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

Description

@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

Activity

  1. added
    C-bugCategory: bug. Something is wrong. This is bad!
    on Sep 14, 2024
  2. seanmonstar commented on Sep 14, 2024

    @seanmonstar
    Member

    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.

  3. randomairborne commented on Sep 14, 2024

    @randomairborne
    Author

    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?

  4. MerlijnW70 commented on Jun 26, 2026

    @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.

  5. Catwoman08 commented on Oct 9, 2026

    @Catwoman08
    Contributor

    A different PR is welcome. Strong preference for human written communication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-serverArea: server.C-bugCategory: bug. Something is wrong. This is bad!E-pr-welcomeEffort: a pull request is welcome.K-hyper-utilCrate: hyper-util

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions