hyper
hyper copied to clipboard
feat(server): add h1 `idle_timeout`
Motivation
Older browsers sometimes open 6 or more H1 connections and keep them alive and idle after resources are loaded, costing server resources and distracting from DoS attackers. In my specific case, DoS mitigation required limiting TCP connections per IP address, but limits had to be high to account for multiple legitimate H1 browsers on the same IP address.
Related: #1628 Related: #2355
Changes
- [x] Added
idle_timeoutsetting, optimized to avoid creating an additional timer per request - [ ] ~~Automatically send
keep-alive: timeout=#header~~ (deferred due to: https://github.com/hyperium/http/issues/142)
Testing
- [x] Fixed multiple pre-existing and broken, but relevant, tests
- [x] Added a test with idle timeout
- [x] Added a test without idle timeout (improves coverage)
- [x] Tested in my app
# Cargo.toml
[patch.crates-io]
hyper = { git = "https://github.com/finnbear/hyper", branch = "idle_timeout" }
hyper-util = { git = "https://github.com/finnbear/hyper-util", branch = "idle_timeout" }
Future work
- [x] Support in
hyper-util(https://github.com/hyperium/hyper-util/pull/146) - [ ] Consider other timeouts (reading body, writing response)
@sfackler / @seanmonstar is there a possibility on getting this PR reviewed? I am also interested in getting H1 idle timeouts available :)
I am also interested in getting H1 idle timeouts available :)
If you want, you can use the TOML snippet in my PR description to avoid needing to wait for review :)
~~Today I investigated further and reached the conclusion that this PR is unnecessary for my use-case because, contrary to my prior belief, header_read_timeout is applied to all requests, not just the first one. I'm not sure why I didn't realize this before.~~
In other words, unmodified hyper works like this:
- accept connection
- start header read timeout
- read header
- stop header read timeout
- read remainder of request and write response
- ~~skip to step 2~~ see next comment
~~This PR would only be relevant for users who want to set different values for header read timeout and idle timeout. Any such user is welcome to:~~
- ~~explain their use-case.~~
- ~~resurrect this PR, possibly with modifications.~~
- ~~if idle_timeout < header_read_timeout, make a custom stream type and use it with
header_read_timeout.~~ - ~~if idle_timeout > header_read_timeout, make a similar custom stream type, disable
header_read_timeout, and reproduce the behavior ofheader_read_timeoutby communicating with a middleware.~~
~~Other timeouts, like reading the body and writing the response, could also be added outside of hyper, by wrapping the request or response body with a timeout detector, and replacing the response with an error response with a Connection: close header.~~
~~Considering the above, I will close this PR. I intend to open a new PR that only includes my fixes to the existing test cases.~~
It seems I confused myself by forgetting the details of my changes. Unmodified hyper does not "skip to step 2." That requires a simple modification, which was part of this PR:
See #3828, which supersedes this PR