Skip to content

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 notifications@github.com
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

3 participants