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

http: 205 reset status violation #3096

Closed
jasnell opened this issue Sep 27, 2015 · 11 comments
Closed

http: 205 reset status violation #3096

jasnell opened this issue Sep 27, 2015 · 11 comments
Assignees
Labels
http Issues or PRs related to the http subsystem.

Comments

@jasnell
Copy link
Member

jasnell commented Sep 27, 2015

Per RFC 7231: "Since the 205 status code implies that no additional content will be provided, a server MUST NOT generate a payload in a 205 response. In other words, a server MUST do one of the following for a 205 response: a) indicate a zero-length body for the response by including a Content-Length header field with a value of 0; b) indicate a zero-length payload for the response by including a Transfer-Encoding header field with a value of chunked and a message body consisting of a single chunk of zero-length; or, c) close the connection immediately after sending the blank line terminating the header section."

Node currently ignores this requirement. To test, create a simple server:

http.createServer(function(req,res) {
  res.writeHeader(205);
  res.end('ok');
}).listen(8080);
$ telnet localhost 8080
GET / HTTP/1.1

HTTP/1.1 205 Reset Content
Date: Sun, 27 Sep 2015 22:51:02 GMT
Connection: keep-alive
Transfer-Encoding: chunked

2
ok
0
@jasnell jasnell self-assigned this Sep 27, 2015
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Sep 27, 2015
@bnoordhuis
Copy link
Member

Enforcing such requirements feels like we're moving from mechanism to policy. I think it's better to view require('http') not as a framework but as a set of building blocks for HTTP servers.

If people feel more policy is called for, I would suggest to split the module in two: a no frills low-level part and a high-level, standards-enforcing part.

@jasnell
Copy link
Member Author

jasnell commented Sep 28, 2015

I strongly disagree. Node currently does the right thing on other status codes (response payload on a 204 No Content is ignored, for instance). Implementing the exact same behavior for a 205 is no different and maintains consistency. The fact that it's not doing the right thing for 205 is a clear bug.

Also, pushing a content payload when none is expected is a great way to do bad things. Clients that get a 205 can assume that it is safe to stop reading from the socket as soon as it get's through the headers, even if the server is still sending data. The next time the client goes to read from the socket, likely expecting another legitimate response, it could encounter something quite unexpected.

@Fishrock123
Copy link
Contributor

I would suggest to split the module in two: a no frills low-level part and a high-level, standards-enforcing part.

So just use net to open a TCP socket and then read the data yourself? :P

Ok I'm mostly joking here, we should probably do the right http standards thing for a 205. I don't have particularly strong feelings about it however.

@Trott
Copy link
Member

Trott commented May 3, 2016

@jasnell In your opinion, how should this be enforced? If there is a violation, should an error be thrown/emitted? Should it warn and correct the issue? Something else?

@nodejs/http

@jasnell
Copy link
Member Author

jasnell commented May 3, 2016

If a user does

http.createServer(function(req,res) {
  res.writeHeader(205);
  res.end('ok');
}).listen(8080);

Personally I would treat it as an error because the developer is clearly doing something incorrect. If we wanted to be lenient, then simply sending Content-Length: 0 and ignoring the payload (not sending it) would be fine. If we did that, I'd prefer to emit a process warning.

@Trott
Copy link
Member

Trott commented Jul 7, 2017

@jasnell Does it matter that the prohibition against a payload accompanying a 205 status code is in a Proposed Standard? Should we wait for RFC 7231 to become an Internet Standard before acting on this? Without giving it too much consideration, I'm in favor of being lenient until we don't have the legitimate option to do so.

@jasnell
Copy link
Member Author

jasnell commented Jul 9, 2017

No, the RFC status has no bearing.

@dougwilson
Copy link
Member

The 205 is, though, defined differently that the 204, which is what it's being compared against here. The 204 (and 304) are actually defined in the RFC to terminate at the first empty line after the header fields, while the 205 status code is not defined that way at all. The way the 205 status code is defined (at least in RFC 7231) makes having a payload not a protocol error (but it is a protocol error for 204/304) and instead having a payload is a semantic issue. The existing http server implementation has stayed the course of only preventing protocol issues, not preventing semantic issues.

@apapirovski
Copy link
Member

@jasnell @bnoordhuis @dougwilson any further opinions on this? Would be good to figure out a direction re: 205 status one way or another.

@bnoordhuis
Copy link
Member

I personally don't think this issue needs addressing but if we wanted to, it's a matter of setting res._hasBody = false. That's the same mechanism that makes node drop the body for HEAD responses.

@dougwilson
Copy link
Member

My opinion is still the same as I have above 👍

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

7 participants