search

snoyberg / http-enumerator

HTTP client package with enumerator interface and HTTPS support.
27 stars 9 forks source link

Manager does not handle "Connection: close" header #28

Closed cpettitt closed 13 years ago

cpettitt commented 13 years ago

Per RFC-2616, section 14.10, the "Connection: close" header is used by the server to indicate that a connection will be closed upon completion of the response. It appears that the withManagedConn function doesn't handle this header, but instead returns the connection to the Manager's pool. When this header is sent and I follow up with my next request, I get this error:

ParseError {errorContexts = ["HTTP status line","demandInput"], errorMessage = "not enough bytes"}

I believe this error occurs because the connection is reused, the server is immediately responding with an RST, and the parsing code is treating this as an unexpected end of stream.

It seems like this should be handled down at the connection management layer, which has, rightly, been abstracted away from the user.

snoyberg commented 13 years ago

I've just pushed a commit that seems to fix the problem, together with a test case to reproduce it. Can you test to ensure this resolves your issue before release?

cpettitt commented 13 years ago

Thanks! I've verified that the client is correctly handling the "Connection: close" header now. Once that header is received, the client opens a new socket. However, the change also has a file descriptor leak - though the socket is not reused, it is not closed, which leaves it in a CLOSED_WAIT state until the process terminates. For long running processes, this could lead to file descriptor exhaustion.

snoyberg commented 13 years ago

Good catch, mind checking the newest commit?

cpettitt commented 13 years ago

Verified fixed. Thanks!

snoyberg commented 13 years ago

Thanks, new version released.