Skip to content
This repository has been archived by the owner on Nov 6, 2022. It is now read-only.

parser: returning 2 from on_headers_complete #299

Closed
wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Apr 16, 2016

Returning 2 from on_headers_complete will tell parser that it
should not expect neither a body nor any futher responses on
this connection. This is useful for handling responses to a
CONNECT request which may not contain Upgrade or
Connection: upgrade headers.

See: nodejs/node#6198

cc @bnoordhuis @jasnell @mscdex

See also: nodejs/node#6198 (comment)

Returning `2` from on_headers_complete will tell parser that it
should not expect neither a body nor any futher responses on
this connection. This is useful for handling responses to a
CONNECT request which may not contain `Upgrade` or
`Connection: upgrade` headers.

See: nodejs/node#6198
@mscdex
Copy link
Contributor

mscdex commented Apr 16, 2016

LGTM if tests pass. Do we have the ability to do this in CI yet?

@indutny
Copy link
Member Author

indutny commented Apr 16, 2016

@mscdex thank you, this is semver minor, right?

@mscdex
Copy link
Contributor

mscdex commented Apr 16, 2016

Assuming the original intention was for CONNECT to be handled in this way, then I would say yes.

@indutny
Copy link
Member Author

indutny commented Apr 16, 2016

Yeah, also existing code will work just as it was before.

@indutny
Copy link
Member Author

indutny commented Apr 16, 2016

A question for @nodejs/lts team, do you think this could be backported to v4.x.x? It seems that responses to CONNECT requests are currently a bit broken in LTS

@indutny
Copy link
Member Author

indutny commented Apr 16, 2016

The problem is that this is a semver-minor change in http-parser.

@indutny
Copy link
Member Author

indutny commented Apr 19, 2016

Alright, enough time to collect feedback. Landing.

indutny added a commit that referenced this pull request Apr 19, 2016
Returning `2` from on_headers_complete will tell parser that it
should not expect neither a body nor any futher responses on
this connection. This is useful for handling responses to a
CONNECT request which may not contain `Upgrade` or
`Connection: upgrade` headers.

See: nodejs/node#6198
PR-URL: #299
Reviewed-By: Brian White <[email protected]>
@indutny
Copy link
Member Author

indutny commented Apr 19, 2016

Landed in 04d28a7, thank you everyone!

@indutny indutny closed this Apr 19, 2016
@indutny indutny deleted the fix/gh-node-6198 branch April 19, 2016 15:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants