Skip to content
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

node does not process HTTP informational responses correctly #9282

Closed
daguej opened this issue Oct 25, 2016 · 3 comments
Closed

node does not process HTTP informational responses correctly #9282

daguej opened this issue Oct 25, 2016 · 3 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@daguej
Copy link
Contributor

daguej commented Oct 25, 2016

There are currently two places in the HTTP client that look for a HTTP 1xx response. Both are to implement 100 Continue:

  • L405: Don't free the parser when the 100 message is complete because a final response will come later
  • L449: Emit the continue event and reset the parser

There is also some code to handle upgrades, which covers HTTP 101 Switching Protocols.

This is good, but is not fully in line with the spec:

A client MUST be able to parse one or more 1xx responses received prior to a final response, even if the client does not expect one. A agent MAY ignore unexpected 1xx responses.

node should deal with responses other than Continue and Switching Protocols -- ≥ 102 ≤ 199. The current behavior is for node to emit a response with the 1xx status code and headers, ending the response. This is incorrect: we MUST be able to cope with one or more 1xx informational responses before the final response arrives.

A good example of this is 102 Processing1. A server MAY send this informational response to inform the client that it's still working on the request:

GET /long-running-operation HTTP/1.1
Host: some-server

HTTP/1.1 102 Processing
X-Maybe-Extra-Header: foo

HTTP/1.1 102 Processing

HTTP/1.1 200 OK
...

1 - I know that RFC 4918 removed a definition for HTTP 102 "due to lack of implementation," but it remains an IANA-registered HTTP response code as originally defined in the otherwise obsolete RFC 2518. In other words, HTTP 102 is still a perfectly legal response code that should be supported.

So when a HTTP response is received with a status code ≥ 102 and ≤ 199, node should not emit a request event; instead, it should reset the HTTP parser in preparation for the final response message.

Additionally, I propose adding an info event to the ClientRequest object that provides a IncomingMessage representing the informational response. This will allow an application interested in handling the 1xx update an opportunity to do so.

@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Oct 25, 2016
@Trott
Copy link
Member

Trott commented May 27, 2017

@nodejs/http

@miles-po
Copy link
Contributor

miles-po commented Jan 8, 2018

I opened PR #18033 to address this. Will need help with tests.

@apapirovski
Copy link
Member

Fixed in #18033

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants