Skip to content

Commit 4874182

Browse files
tellnestellnes
committed
http: send Content-Length when possible
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>
1 parent 1640ded commit 4874182

6 files changed

Lines changed: 129 additions & 12 deletions

‎lib/_http_outgoing.js‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const closeExpression = /close/i;
1616
const contentLengthExpression = /^Content-Length$/i;
1717
const dateExpression = /^Date$/i;
1818
const expectExpression = /^Expect$/i;
19+
const trailerExpression = /^Trailer$/i;
1920

2021
const automaticHeaders = {
2122
connection: true,
@@ -56,6 +57,7 @@ function OutgoingMessage() {
5657
this.sendDate = false;
5758
this._removedHeader = {};
5859

60+
this._contentLength = null;
5961
this._hasBody = true;
6062
this._trailer = '';
6163

@@ -185,6 +187,7 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
185187
sentTransferEncodingHeader: false,
186188
sentDateHeader: false,
187189
sentExpect: false,
190+
sentTrailer: false,
188191
messageHeader: firstLine
189192
};
190193

@@ -257,16 +260,26 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
257260

258261
if (state.sentContentLengthHeader === false &&
259262
state.sentTransferEncodingHeader === false) {
260-
if (this._hasBody && !this._removedHeader['transfer-encoding']) {
261-
if (this.useChunkedEncodingByDefault) {
263+
if (!this._hasBody) {
264+
// Make sure we don't end the 0\r\n\r\n at the end of the message.
265+
this.chunkedEncoding = false;
266+
} else if (!this.useChunkedEncodingByDefault) {
267+
this._last = true;
268+
} else {
269+
if (!state.sentTrailer &&
270+
!this._removedHeader['content-length'] &&
271+
typeof this._contentLength === 'number') {
272+
state.messageHeader += 'Content-Length: ' + this._contentLength +
273+
'\r\n';
274+
} else if (!this._removedHeader['transfer-encoding']) {
262275
state.messageHeader += 'Transfer-Encoding: chunked\r\n';
263276
this.chunkedEncoding = true;
264277
} else {
265-
this._last = true;
278+
// We should only be able to get here if both Content-Length and
279+
// Transfer-Encoding are removed by the user.
280+
// See: test/parallel/test-http-remove-header-stays-removed.js
281+
debug('Both Content-Length and Transfer-Encoding are removed');
266282
}
267-
} else {
268-
// Make sure we don't end the 0\r\n\r\n at the end of the message.
269-
this.chunkedEncoding = false;
270283
}
271284
}
272285

@@ -304,6 +317,8 @@ function storeHeader(self, state, field, value) {
304317
state.sentDateHeader = true;
305318
} else if (expectExpression.test(field)) {
306319
state.sentExpect = true;
320+
} else if (trailerExpression.test(field)) {
321+
state.sentTrailer = true;
307322
}
308323
}
309324

@@ -509,6 +524,14 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) {
509524
this.once('finish', callback);
510525

511526
if (!this._header) {
527+
if (data) {
528+
if (typeof data === 'string')
529+
this._contentLength = Buffer.byteLength(data, encoding);
530+
else
531+
this._contentLength = data.length;
532+
} else {
533+
this._contentLength = 0;
534+
}
512535
this._implicitHeader();
513536
}
514537

‎test/parallel/test-http-automatic-headers.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ var http = require('http');
55
var server = http.createServer(function(req, res) {
66
res.setHeader('X-Date', 'foo');
77
res.setHeader('X-Connection', 'bar');
8-
res.setHeader('X-Transfer-Encoding', 'baz');
8+
res.setHeader('X-Content-Length', 'baz');
99
res.end();
1010
});
1111
server.listen(common.PORT);
@@ -20,10 +20,10 @@ server.on('listening', function() {
2020
assert.equal(res.statusCode, 200);
2121
assert.equal(res.headers['x-date'], 'foo');
2222
assert.equal(res.headers['x-connection'], 'bar');
23-
assert.equal(res.headers['x-transfer-encoding'], 'baz');
23+
assert.equal(res.headers['x-content-length'], 'baz');
2424
assert(res.headers['date']);
2525
assert.equal(res.headers['connection'], 'keep-alive');
26-
assert.equal(res.headers['transfer-encoding'], 'chunked');
26+
assert.equal(res.headers['content-length'], '0');
2727
server.close();
2828
agent.destroy();
2929
});

‎test/parallel/test-http-client-default-headers-exist.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ var expectedHeaders = {
77
'GET': ['host', 'connection'],
88
'HEAD': ['host', 'connection'],
99
'OPTIONS': ['host', 'connection'],
10-
'POST': ['host', 'connection', 'transfer-encoding'],
11-
'PUT': ['host', 'connection', 'transfer-encoding']
10+
'POST': ['host', 'connection', 'content-length'],
11+
'PUT': ['host', 'connection', 'content-length']
1212
};
1313

1414
var expectedMethods = Object.keys(expectedHeaders);

‎test/parallel/test-http-content-length.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
var common = require('../common');
2+
var assert = require('assert');
3+
var http = require('http');
4+
5+
var expectedHeadersMultipleWrites = {
6+
'connection': 'close',
7+
'transfer-encoding': 'chunked',
8+
};
9+
10+
var expectedHeadersEndWithData = {
11+
'connection': 'close',
12+
'content-length': 'hello world'.length,
13+
};
14+
15+
var expectedHeadersEndNoData = {
16+
'connection': 'close',
17+
'content-length': '0',
18+
};
19+
20+
var receivedRequests = 0;
21+
var totalRequests = 2;
22+
23+
var server = http.createServer(function(req, res) {
24+
res.removeHeader('Date');
25+
26+
switch (req.url.substr(1)) {
27+
case 'multiple-writes':
28+
assert.deepEqual(req.headers, expectedHeadersMultipleWrites);
29+
res.write('hello');
30+
res.end('world');
31+
break;
32+
case 'end-with-data':
33+
assert.deepEqual(req.headers, expectedHeadersEndWithData);
34+
res.end('hello world');
35+
break;
36+
case 'empty':
37+
assert.deepEqual(req.headers, expectedHeadersEndNoData);
38+
res.end();
39+
break;
40+
default:
41+
throw new Error('Unreachable');
42+
break;
43+
}
44+
45+
receivedRequests++;
46+
if (totalRequests === receivedRequests) server.close();
47+
});
48+
49+
server.listen(common.PORT, function() {
50+
var req;
51+
52+
req = http.request({
53+
port: common.PORT,
54+
method: 'POST',
55+
path: '/multiple-writes'
56+
});
57+
req.removeHeader('Date');
58+
req.removeHeader('Host');
59+
req.write('hello ');
60+
req.end('world');
61+
req.on('response', function(res) {
62+
assert.deepEqual(res.headers, expectedHeadersMultipleWrites);
63+
});
64+
65+
req = http.request({
66+
port: common.PORT,
67+
method: 'POST',
68+
path: '/end-with-data'
69+
});
70+
req.removeHeader('Date');
71+
req.removeHeader('Host');
72+
req.end('hello world');
73+
req.on('response', function(res) {
74+
assert.deepEqual(res.headers, expectedHeadersEndWithData);
75+
});
76+
77+
req = http.request({
78+
port: common.PORT,
79+
method: 'POST',
80+
path: '/empty'
81+
});
82+
req.removeHeader('Date');
83+
req.removeHeader('Host');
84+
req.end();
85+
req.on('response', function(res) {
86+
assert.deepEqual(res.headers, expectedHeadersEndNoData);
87+
});
88+
89+
});

‎test/parallel/test-http-raw-headers.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ http.createServer(function(req, res) {
4444
});
4545

4646
req.resume();
47+
res.setHeader('Trailer', 'x-foo');
4748
res.addTrailers([
4849
['x-fOo', 'xOxOxOx'],
4950
['x-foO', 'OxOxOxO'],
@@ -72,6 +73,8 @@ http.createServer(function(req, res) {
7273
req.end('y b a r');
7374
req.on('response', function(res) {
7475
var expectRawHeaders = [
76+
'Trailer',
77+
'x-foo',
7578
'Date',
7679
null,
7780
'Connection',
@@ -80,11 +83,12 @@ http.createServer(function(req, res) {
8083
'chunked'
8184
];
8285
var expectHeaders = {
86+
trailer: 'x-foo',
8387
date: null,
8488
connection: 'close',
8589
'transfer-encoding': 'chunked'
8690
};
87-
res.rawHeaders[1] = null;
91+
res.rawHeaders[3] = null;
8892
res.headers.date = null;
8993
assert.deepEqual(res.rawHeaders, expectRawHeaders);
9094
assert.deepEqual(res.headers, expectHeaders);

‎test/parallel/test-http-remove-header-stays-removed.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var server = http.createServer(function(request, response) {
88
// to the output:
99
response.removeHeader('connection');
1010
response.removeHeader('transfer-encoding');
11+
response.removeHeader('content-length');
1112

1213
// make sure that removing and then setting still works:
1314
response.removeHeader('date');

0 commit comments

Comments
 (0)