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

Proposal: http: automaticly send Content-Length header when possible #1044

Closed
tellnes opened this issue Mar 3, 2015 · 4 comments
Closed
Labels
http Issues or PRs related to the http subsystem.

Comments

@tellnes
Copy link
Contributor

tellnes commented Mar 3, 2015

I propose we make http set the Content-Length header when the user does only call OutgoingMessage.prototype.end and not write or writeHead.

Eg.

require('http').createServer(function (req, res) {
  res.end('Hello World')
}).listen(1337)
HTTP/1.1 200 OK
Date: Tue, 03 Mar 2015 19:42:18 GMT
Connection: keep-alive
Content-Length: 11

Hello World

It is a trivial change in lib/, but breaks a lot of tests.

@tellnes
Copy link
Contributor Author

tellnes commented Mar 3, 2015

It breaks 20 tests, but mostly it looks like trivial stuff like:

AssertionError: { date: 'coffee o clock', 'content-length': '10' } deepEqual { date: 'coffee o clock' }

@Fishrock123
Copy link
Contributor

Sending content-length here seams like it should be expected, given .write()'s automatic headers behaviour.

@bnoordhuis
Copy link
Member

Seems reasonable to me. It would save a few bytes per response as well.

@tellnes
Copy link
Contributor Author

tellnes commented Mar 3, 2015

Ok, I'll see if I can get something ready later tonight or tomorrow.

@brendanashworth brendanashworth added the http Issues or PRs related to the http subsystem. label Mar 4, 2015
tellnes added a commit to tellnes/io.js that referenced this issue Mar 5, 2015
This changes the behavior for http to send send a Content-Length header
instead of using chunked encoding when we know the size of the body when
sending the headers.

Fixes: nodejs#1044
PR-URL: nodejs#1062
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Brian White <mscdex@mscdex.net>
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

4 participants