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

res.sendStatus(205) returns a Reset Content body #4592

Closed
tkesgar opened this issue May 16, 2021 · 7 comments · Fixed by #4596
Closed

res.sendStatus(205) returns a Reset Content body #4592

tkesgar opened this issue May 16, 2021 · 7 comments · Fixed by #4596

Comments

@tkesgar
Copy link
Contributor

tkesgar commented May 16, 2021

It seems that HTTP 204 and 205 responses should not have a response body:

res.sendStatus(204) correctly returns no response body, but res.sendStatus(205) returns a Reset Content string. Is this expected?

How to reproduce

require('express')()
  .get('/204', (req, res) => res.sendStatus(204))
  .get('/205', (req, res) => res.sendStatus(205))
  .listen(8080, () => console.log('Ready'))
$ curl http://localhost:8080/204

$ curl http://localhost:8080/205
Reset Content

$ curl -I -XGET http://192.168.100.232:8080/204
HTTP/1.1 204 No Content
X-Powered-By: Express
ETag: W/"a-bAsFyilMr4Ra1hIU5PyoyFRunpI"
Date: Sun, 16 May 2021 12:26:18 GMT
Connection: keep-alive
Keep-Alive: timeout=5

$ curl -I -XGET http://192.168.100.232:8080/205
HTTP/1.1 205 Reset Content
X-Powered-By: Express
Content-Type: text/plain; charset=utf-8
Content-Length: 13
ETag: W/"d-yL62EER5Ke7tFugRpssh46aJ7As"
Date: Sun, 16 May 2021 12:26:25 GMT
Connection: keep-alive
Keep-Alive: timeout=5

Express version: 4.17.1
Node.js version: 15.14.0

@dougwilson
Copy link
Contributor

dougwilson commented May 16, 2021

Thank you for the report! It looks like Express can only fix part of the issue, unfortunately. We can change it to send an empty body, but cannot send no body at all. This is because Node.js does not procide an API to send no body, instead sending a zero length body at minimum, which violates the spec: https://datatracker.ietf.org/doc/html/rfc7231#section-6.3.6

If having the body is messing up the client, the change in Express is unlikely to be a complete fix and Node.js would also need to add 205 to the list of status codes that must not have a response body.

https://github.com/nodejs/node/blob/82eddca23d35bcf779d9d1905744cc9cfa6ad9a0/lib/_http_server.js#L327

https://github.com/nodejs/node/blob/82eddca23d35bcf779d9d1905744cc9cfa6ad9a0/lib/_http_outgoing.js#L462

@dougwilson
Copy link
Contributor

Apologies, I just reread 205 and apparently it is slightly different from 204 and 304, in that it does have a payload, but just must be zero length. We can fix that here, but still can only guarantee the fix as long as the users use the Express methods like res.send, res.json, etc. Using res.end and res.write would require Node.js to add a rule as those methods are delivered by the Node.js framework.

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.

@tkesgar
Copy link
Contributor Author

tkesgar commented May 16, 2021

Oops, I missed the details for HTTP 205. I thought it should behave exactly like 204 🐧

still can only guarantee the fix as long as the users use the Express methods like res.send, res.json, etc.

I think that is a fair deal, since users wanting to send HTTP 205 would probably use res.sendStatus(205), since res.send(205) will show the deprecation message.

This is a workaround for now I think:

res.status(205).send('')
$ curl -I -XGET http://localhost:8080/205-empty
HTTP/1.1 205 Reset Content
X-Powered-By: Express
Content-Type: text/html; charset=utf-8
Content-Length: 0
ETag: W/"0-2jmj7l5rSw0yVb/vlWAYkK/YBwk"
Date: Sun, 16 May 2021 13:59:03 GMT
Connection: keep-alive
Keep-Alive: timeout=5

@tkesgar
Copy link
Contributor Author

tkesgar commented May 20, 2021

I tried some fix at #4596.

While it matches the spec (send a Content-Length: 0 and empty chunk), I am still unsure if that is the best way to handle this.

@dougwilson dougwilson mentioned this issue Feb 19, 2022
20 tasks
dougwilson pushed a commit to tkesgar/express that referenced this issue Feb 20, 2022
@everhardt
Copy link

This is a breaking change that hurt our production services. I see expressjs is following semver, shouldn't this have increased the major number?

Also, there's currently no way to disable this new functionality

@everhardt
Copy link

Would you consider a PR that adds a parameter strict = true to the signature of .send, so that it becomes .send(body, strict = true)? For strict === false, we would then not do the 204/205-specific cleaning actions.

@dougwilson
Copy link
Contributor

dougwilson commented Jun 23, 2022

Hi @everhardt thank you for the feedback. Yes, Express.js uses semver, but sometimes fixing bugs is a hard choice. Express.js considers violations of the HTTP specs to be bugs and the prior behavior, as pointed out by the OP, violated the HTTP spec so we fixed that bug.

You can skip this functionality by using the underlying .write/.end methods, at least until Node.js also releases a fix to stop sending a body for a 205 response.

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 a pull request may close this issue.

3 participants