Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Responses will never complete if nothing is written to the response body #173

Closed
halter73 opened this issue Aug 25, 2015 · 8 comments
Closed
Assignees
Milestone

Comments

@halter73
Copy link
Member

When an AppFunc returns a completed task without writing to the response stream (and certain headers have not been set that would disable the new automatic chunking feature), the chunk meant to end the response will never be sent.

This means that the client sees their request hang indefinitely, and this causes the connection the response was sent over to become unusable for future requests despite being a “keep-alive” connection.

Not writing to the response stream is fairly common, particularly for non-200 responses (redirects, 404s, etc…).

@halter73 halter73 added this to the 1.0.0-beta7 milestone Aug 25, 2015
@halter73 halter73 self-assigned this Aug 25, 2015
@Tratcher
Copy link
Member

You should set content-length: 0 in this scenario rather than chunking.

@halter73
Copy link
Member Author

What do you suggest doing if the content-length was set to a non-zero value already, but nothing was written? Keep the incorrect content-length?

@Tratcher
Copy link
Member

500 internal server error?

What do you do if the app doesn't write the same amount of data as the specified content length?

@halter73
Copy link
Member Author

We don't verify that apps correctly write the amount of data they specify in content-length header. Even if we tried, the best we could do is kill the connection after recognizing the error. Since we don't buffer the response, we couldn't change the status code to a 500.

In the case of the apps that set a non-zero content-length header but don't write anything, we could change the status code to a 500. I don't think it's worth it to this though, since we can't change the status code to a 500 for bad content-lengths in the general case.

halter73 added a commit that referenced this issue Aug 26, 2015
- Previously an incomplete chunked response would be written instead.
- Add test to verify Content-Length: 0 is set automatically.
- Add test to verify HTTP/1.0 keep-alive isn't used if no Content-Length
  is set for the response.
- Add tests to verify errors are handled properly after chunked writes.

#173
@Tratcher
Copy link
Member

That's going to cause connection leaks, protocol violations, etc.. For comparison what do you do if there's an unhandled exception before a response body has started? How about after the response has started?

@halter73
Copy link
Member Author

If there is an unhandled exception we respond with a 500 or close the connection completely if that's not possible as of beta7. You already filed an issue for that and I fixed it: #43

We don't protect against protocol violations. Do you know if other servers verify that the content-length header is set correctly?

@Tratcher
Copy link
Member

WebListener does. I don't know about other servers.

@muratg
Copy link
Contributor

muratg commented Aug 27, 2015

This one is done, yes? Closing. I believe @Tratcher is filing issues for other cases he investigated.

@muratg muratg closed this as completed Aug 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants