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

fix(server): Drain requests on drop. #310

Merged
merged 1 commit into from
Feb 14, 2015
Merged

fix(server): Drain requests on drop. #310

merged 1 commit into from
Feb 14, 2015

Conversation

seanmonstar
Copy link
Member

If a client sent an illegal request (like a GET request with a message
body), or if there was a legal request with a body but the Handler
didn't read all of it, the remaining bytes would be left in the stream.
The next request to come from the same client would error, as the server
would confuse the remaining bytes, and think the request was malformed.

Fixes #197
Fixes #309

If a client sent an illegal request (like a GET request with a message
body), or if there was a legal request with a body but the Handler
didn't read all of it, the remaining bytes would be left in the stream.
The next request to come from the same client would error, as the server
would confuse the remaining bytes, and think the request was malformed.

Fixes #197
Fixes #309
@seanmonstar
Copy link
Member Author

@reem fixed the draining logic. I just use copy to a NullWriter.

Switching to new io will just require changing NullWriter to Sink. The EOF logic is handled inside copy, so nothing to screw up.

@reem
Copy link
Contributor

reem commented Feb 14, 2015

Looks great.

reem added a commit that referenced this pull request Feb 14, 2015
fix(server): Drain requests on drop.
@reem reem merged commit 361e451 into master Feb 14, 2015
@reem reem deleted the server-request-drop branch February 14, 2015 22:36
@reem
Copy link
Contributor

reem commented Feb 14, 2015

Actually, this is a horrible idea security-wise, I think. I may start reading a request body, see it's super long, and quit. I do not expect hyper to silently read the other 40 GB of that request.

@seanmonstar
Copy link
Member Author

Oh dang, good catch. I'll look at nodejs for inspiration.

On Sat, Feb 14, 2015, 3:12 PM Jonathan Reem [email protected]
wrote:

Actually, this is a horrible idea security-wise, I think. I may start
reading a request body, see it's super long, and quit. I do not expect
hyper to silently read the other 40 GB of that request.


Reply to this email directly or view it on GitHub
#310 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read body for all requests with a Content-Length header request error = HttpMethodError logs
2 participants