hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.63k stars 1.6k forks source link

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

Open randomairborne opened 2 months ago

randomairborne commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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?