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: https://github.com/iojs/io.js/issues/1044
PR-URL: https://github.com/iojs/io.js/pull/1062
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Brian White <mscdex@mscdex.net>

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 2e64632..f6144ba 100644 (file)
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -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,
@@ -56,6 +57,7 @@ function OutgoingMessage() {
   this.sendDate = false;
   this._removedHeader = {};
 
+  this._contentLength = null;
   this._hasBody = true;
   this._trailer = '';
 
@@ -185,6 +187,7 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
     sentTransferEncodingHeader: false,
     sentDateHeader: false,
     sentExpect: false,
+    sentTrailer: false,
     messageHeader: firstLine
   };
 
@@ -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;
     }
   }
 
@@ -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;
   }
 }
 
@@ -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();
   }
 

diff --git a/test/parallel/test-http-automatic-headers.js b/test/parallel/test-http-automatic-headers.js
index cc71cb7..76e5a61 100644 (file)
--- a/test/parallel/test-http-automatic-headers.js
+++ b/test/parallel/test-http-automatic-headers.js
@@ -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);
@@ -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();
   });

diff --git a/test/parallel/test-http-client-default-headers-exist.js b/test/parallel/test-http-client-default-headers-exist.js
index 85ef2ac..0217538 100644 (file)
--- a/test/parallel/test-http-client-default-headers-exist.js
+++ b/test/parallel/test-http-client-default-headers-exist.js
@@ -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);

diff --git a/test/parallel/test-http-content-length.js b/test/parallel/test-http-content-length.js
new file mode 100644 (file)
index 0000000..f986a2a
--- /dev/null
+++ b/test/parallel/test-http-content-length.js
@@ -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);
+  });
+
+});

diff --git a/test/parallel/test-http-raw-headers.js b/test/parallel/test-http-raw-headers.js
index 76447c6..b4c1401 100644 (file)
--- a/test/parallel/test-http-raw-headers.js
+++ b/test/parallel/test-http-raw-headers.js
@@ -44,6 +44,7 @@ http.createServer(function(req, res) {
   });
 
   req.resume();
+  res.setHeader('Trailer', 'x-foo');
   res.addTrailers([
     ['x-fOo', 'xOxOxOx'],
     ['x-foO', 'OxOxOxO'],
@@ -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',
@@ -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);

diff --git a/test/parallel/test-http-remove-header-stays-removed.js b/test/parallel/test-http-remove-header-stays-removed.js
index a9a5f04..e963989 100644 (file)
--- a/test/parallel/test-http-remove-header-stays-removed.js
+++ b/test/parallel/test-http-remove-header-stays-removed.js
@@ -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');