-
Notifications
You must be signed in to change notification settings - Fork 322
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
Raise error on unexpected EOF. #163
Conversation
Fixes infinite read() loop in the case where the server closes the connection without sending a valid HTTP response.
Hi, thanks for the great library! This is a patch for a snag I ran into today, the commit message has the details. There is a spec, but you can reproduce the problem manually like so:
Feedback appreciated! |
I'm a bit uncertain about the semantic implications of this... I'd be curious what @ixti thinks. Looks like the tests pass aside from some Rubbocrap. |
It's actually a kind of small regression. Return of Now I see that this is the cause of most cases described in #152 @tarcieri i'm more curious when you switched from "I'm totally fine if it enforces consistent style" to "rubbocrap" ;)) |
@tarcieri wrote:
@ixti wrote:
If you look at the RuboCop failures, they’re actually related to quality metrics, not style.
If we want to relax our standards for number of lines per method and cyclomatic complexity in order to merge this patch, we can do that, but ideally, we would could create a patch that doesn’t increasing the code complexity above our current threshold. |
@sferik sorry, I thought that I have different style preferences compared to yours (the main difference is single quotes versus double quotes), but other than that I would like to keep current rubocop config as strict as possible. @mickm I don't want to merge this patch as is - there are couple of issues one of them I explained above. The other is specs. They actually cover only one particular case: when connection was unpredictably closed before headers were received. So I'm gonna extend specs and add |
That all makes sense. I'm happy to take a shot at any of those changes, although it sounds like you're already on it :) Let me know if I can help out in any way and thanks for the feedback! |
Fixes infinite read() loop in the case where the server closes the
connection without sending a valid HTTP response.