Skip to content

HTTP 1.1 keepalive is only determined by presence of Connection: close. #142

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

Merged
merged 1 commit into from
Oct 2, 2012

Conversation

taer
Copy link
Contributor

@taer taer commented Oct 2, 2012

The RFC for HTTP 1.1 only states that keepalive should be terminated on presence of Connection:close. The default behavior in 1.1 is to keep the connection open, so the presence of a Connection: keep-alive is optional. I believe the keep-alive status was for 1.0 persistent connection negotiation.

This patch doesn't look at the HTTP version. The existing code would have had issues with 1.0 as well, so I'm assuming we can safely ignore that.

The grizly provider seems to have the proper logic in it already.

connection:close header instead of keep-alive presence.
jfarcand added a commit that referenced this pull request Oct 2, 2012
HTTP 1.1 keepalive is only determined by presence of Connection: close.
@jfarcand jfarcand merged commit 4cba52d into AsyncHttpClient:master Oct 2, 2012
@jfarcand
Copy link
Contributor

jfarcand commented Oct 2, 2012

Agree, thanks!

@taer
Copy link
Contributor Author

taer commented Oct 2, 2012

However, this doesn't fix an issue we're having. The target server isn't sending a Connection: header at all. So
future.setKeepAlive(header==null || otherStuff) should evaluate to true based on lack of header. Do you have any other idea as to what would cause async-http-client to close the socket and make a new one in this case with the netty provider?

@jfarcand
Copy link
Contributor

jfarcand commented Oct 2, 2012

Salut, I would needs a unit test for reproducing...I'm not active on AHC those days and I can be dangerous :-) So if you have a unit test I can take a look.

cs-workco pushed a commit to cs-workco/async-http-client that referenced this pull request Apr 13, 2023
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.

2 participants