Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
randomairborne opened this issue Sep 14, 2024 · 2 comments
Labels
C-bug Category: bug. Something is wrong. This is bad!

Comments

@randomairborne
Copy link

randomairborne commented Sep 14, 2024

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

@randomairborne randomairborne added the C-bug Category: bug. Something is wrong. This is bad! label Sep 14, 2024
@seanmonstar
Copy link
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.

@randomairborne
Copy link
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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug. Something is wrong. This is bad!
Projects
None yet
Development

No branches or pull requests

2 participants