-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(h1): add server header read timeout #2675
feat(h1): add server header read timeout #2675
Conversation
This is to start timing after parsing the request header of the first block. Clear the timing after parsing all the request headers? |
It is recommended that test cases cover these scenarios:
|
Yup
Done in the last commit |
Resolved the merge conflicts after the last PR was merged. Ping @seanmonstar. We're waiting for a code review 😃 |
@paolobarbolini From the header_read_timeout_no_write test case, it seems that once the connection is established, the timeout mechanism is started. Is that correct? I think this implementation is lacking, which means that the client's connection pool cannot establish connections in advance, and the client's keepAlive configuration is broken and does not keep alive as expected. |
Right, I didn't consider connection pools starting connections in advance. Fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the lag, thanks for writing this! It's very thorough, and I especially appreciate that there's tests so we don't accidentally break it in the future ❤️
I left a few comments inline.
Co-authored-by: Sean McArthur <[email protected]>
Co-authored-by: Sean McArthur <[email protected]>
Great work here! This PR leaves me wondering though, won't hyper also need this for h2 and h3, or are there other mechanisms in place there that prevent slow loris attacks? |
Valiant effort, but still there is no way to set a timeout for clients that connect only to send no data (by experimentation I discovered that the timeout does not fire if I just telnet into the TCP port and hang on there -- it only fires if I send the actual headers too late). Which means it's trivial to gum up a hyper server just by sending a lot of connections and nothing else. |
Implements a timeout for reading the request header. The timer starts either when the connection is established (first request) or when the first byte is read from the header (subsequent requests reusing the same connection). The entire header must be read before the specified timeout is elapsed.
This is the first step towards protecting from Slowloris types of attacks
Supersedes #2661 by fixing #2661 (comment)
Partially fixes #2457
Closes #2661