Skip to content

Commit

Permalink
http: send Content-Length when possible
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Brian White <[email protected]>
  • Loading branch information
tellnes committed Mar 5, 2015
1 parent 1640ded commit 4874182
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 12 deletions.
35 changes: 29 additions & 6 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const closeExpression = /close/i;
const contentLengthExpression = /^Content-Length$/i;
const dateExpression = /^Date$/i;
const expectExpression = /^Expect$/i;
const trailerExpression = /^Trailer$/i;

const automaticHeaders = {
connection: true,
Expand Down Expand Up @@ -56,6 +57,7 @@ function OutgoingMessage() {
this.sendDate = false;
this._removedHeader = {};

this._contentLength = null;
this._hasBody = true;
this._trailer = '';

Expand Down Expand Up @@ -185,6 +187,7 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
sentTransferEncodingHeader: false,
sentDateHeader: false,
sentExpect: false,
sentTrailer: false,
messageHeader: firstLine
};

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

if (state.sentContentLengthHeader === false &&
state.sentTransferEncodingHeader === false) {
if (this._hasBody && !this._removedHeader['transfer-encoding']) {
if (this.useChunkedEncodingByDefault) {
if (!this._hasBody) {
// Make sure we don't end the 0\r\n\r\n at the end of the message.
this.chunkedEncoding = false;
} else if (!this.useChunkedEncodingByDefault) {
this._last = true;
} else {
if (!state.sentTrailer &&
!this._removedHeader['content-length'] &&
typeof this._contentLength === 'number') {
state.messageHeader += 'Content-Length: ' + this._contentLength +
'\r\n';
} else if (!this._removedHeader['transfer-encoding']) {
state.messageHeader += 'Transfer-Encoding: chunked\r\n';
this.chunkedEncoding = true;
} else {
this._last = true;
// We should only be able to get here if both Content-Length and
// Transfer-Encoding are removed by the user.
// See: test/parallel/test-http-remove-header-stays-removed.js
debug('Both Content-Length and Transfer-Encoding are removed');
}
} else {
// Make sure we don't end the 0\r\n\r\n at the end of the message.
this.chunkedEncoding = false;
}
}

Expand Down Expand Up @@ -304,6 +317,8 @@ function storeHeader(self, state, field, value) {
state.sentDateHeader = true;
} else if (expectExpression.test(field)) {
state.sentExpect = true;
} else if (trailerExpression.test(field)) {
state.sentTrailer = true;
}
}

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

if (!this._header) {
if (data) {
if (typeof data === 'string')
this._contentLength = Buffer.byteLength(data, encoding);
else
this._contentLength = data.length;
} else {
this._contentLength = 0;
}
this._implicitHeader();
}

Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-http-automatic-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var http = require('http');
var server = http.createServer(function(req, res) {
res.setHeader('X-Date', 'foo');
res.setHeader('X-Connection', 'bar');
res.setHeader('X-Transfer-Encoding', 'baz');
res.setHeader('X-Content-Length', 'baz');
res.end();
});
server.listen(common.PORT);
Expand All @@ -20,10 +20,10 @@ server.on('listening', function() {
assert.equal(res.statusCode, 200);
assert.equal(res.headers['x-date'], 'foo');
assert.equal(res.headers['x-connection'], 'bar');
assert.equal(res.headers['x-transfer-encoding'], 'baz');
assert.equal(res.headers['x-content-length'], 'baz');
assert(res.headers['date']);
assert.equal(res.headers['connection'], 'keep-alive');
assert.equal(res.headers['transfer-encoding'], 'chunked');
assert.equal(res.headers['content-length'], '0');
server.close();
agent.destroy();
});
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-client-default-headers-exist.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ var expectedHeaders = {
'GET': ['host', 'connection'],
'HEAD': ['host', 'connection'],
'OPTIONS': ['host', 'connection'],
'POST': ['host', 'connection', 'transfer-encoding'],
'PUT': ['host', 'connection', 'transfer-encoding']
'POST': ['host', 'connection', 'content-length'],
'PUT': ['host', 'connection', 'content-length']
};

var expectedMethods = Object.keys(expectedHeaders);
Expand Down
89 changes: 89 additions & 0 deletions test/parallel/test-http-content-length.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
var common = require('../common');
var assert = require('assert');
var http = require('http');

var expectedHeadersMultipleWrites = {
'connection': 'close',
'transfer-encoding': 'chunked',
};

var expectedHeadersEndWithData = {
'connection': 'close',
'content-length': 'hello world'.length,
};

var expectedHeadersEndNoData = {
'connection': 'close',
'content-length': '0',
};

var receivedRequests = 0;
var totalRequests = 2;

var server = http.createServer(function(req, res) {
res.removeHeader('Date');

switch (req.url.substr(1)) {
case 'multiple-writes':
assert.deepEqual(req.headers, expectedHeadersMultipleWrites);
res.write('hello');
res.end('world');
break;
case 'end-with-data':
assert.deepEqual(req.headers, expectedHeadersEndWithData);
res.end('hello world');
break;
case 'empty':
assert.deepEqual(req.headers, expectedHeadersEndNoData);
res.end();
break;
default:
throw new Error('Unreachable');
break;
}

receivedRequests++;
if (totalRequests === receivedRequests) server.close();
});

server.listen(common.PORT, function() {
var req;

req = http.request({
port: common.PORT,
method: 'POST',
path: '/multiple-writes'
});
req.removeHeader('Date');
req.removeHeader('Host');
req.write('hello ');
req.end('world');
req.on('response', function(res) {
assert.deepEqual(res.headers, expectedHeadersMultipleWrites);
});

req = http.request({
port: common.PORT,
method: 'POST',
path: '/end-with-data'
});
req.removeHeader('Date');
req.removeHeader('Host');
req.end('hello world');
req.on('response', function(res) {
assert.deepEqual(res.headers, expectedHeadersEndWithData);
});

req = http.request({
port: common.PORT,
method: 'POST',
path: '/empty'
});
req.removeHeader('Date');
req.removeHeader('Host');
req.end();
req.on('response', function(res) {
assert.deepEqual(res.headers, expectedHeadersEndNoData);
});

});
6 changes: 5 additions & 1 deletion test/parallel/test-http-raw-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ http.createServer(function(req, res) {
});

req.resume();
res.setHeader('Trailer', 'x-foo');
res.addTrailers([
['x-fOo', 'xOxOxOx'],
['x-foO', 'OxOxOxO'],
Expand Down Expand Up @@ -72,6 +73,8 @@ http.createServer(function(req, res) {
req.end('y b a r');
req.on('response', function(res) {
var expectRawHeaders = [
'Trailer',
'x-foo',
'Date',
null,
'Connection',
Expand All @@ -80,11 +83,12 @@ http.createServer(function(req, res) {
'chunked'
];
var expectHeaders = {
trailer: 'x-foo',
date: null,
connection: 'close',
'transfer-encoding': 'chunked'
};
res.rawHeaders[1] = null;
res.rawHeaders[3] = null;
res.headers.date = null;
assert.deepEqual(res.rawHeaders, expectRawHeaders);
assert.deepEqual(res.headers, expectHeaders);
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-http-remove-header-stays-removed.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ var server = http.createServer(function(request, response) {
// to the output:
response.removeHeader('connection');
response.removeHeader('transfer-encoding');
response.removeHeader('content-length');

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

0 comments on commit 4874182

Please sign in to comment.