From 80c9ef0b6be57d42632526818a3b0c3f20f225a1 Mon Sep 17 00:00:00 2001
From: Artur G Vieira <vieira.artur.g@gmail.com>
Date: Sat, 27 May 2017 03:00:09 +0000
Subject: [PATCH] http: edit _storeHeader to check for Trailer header

Test non-chunked message does not have trailer header set,
message will be terminated by the first empty line after the
header fields, regardless of the header fields present in the
message, and thus cannot contain a message body or 'trailers'.

PR-URL: https://github.com/nodejs/node/pull/12990
Ref: https://github.com/nodejs/node/issues/2842
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
---
 lib/_http_outgoing.js                         |  9 +++++
 lib/internal/errors.js                        |  2 ++
 .../test-http-server-de-chunked-trailer.js    | 33 +++++++++++++++++++
 3 files changed, 44 insertions(+)
 create mode 100644 test/parallel/test-http-server-de-chunked-trailer.js

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index b3706d5a9e..636afc4e46 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -33,6 +33,7 @@ const checkInvalidHeaderChar = common._checkInvalidHeaderChar;
 const outHeadersKey = require('internal/http').outHeadersKey;
 const async_id_symbol = process.binding('async_wrap').async_id_symbol;
 const nextTick = require('internal/process/next_tick').nextTick;
+const errors = require('internal/errors');
 
 const CRLF = common.CRLF;
 const debug = common.debug;
@@ -427,6 +428,14 @@ function _storeHeader(firstLine, headers) {
     }
   }
 
+  // Test non-chunked message does not have trailer header set,
+  // message will be terminated by the first empty line after the
+  // header fields, regardless of the header fields present in the
+  // message, and thus cannot contain a message body or 'trailers'.
+  if (this.chunkedEncoding !== true && state.trailer) {
+    throw new errors.Error('ERR_HTTP_TRAILER_INVALID');
+  }
+
   this._header = state.header + CRLF;
   this._headerSent = false;
 
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index c700d3d7d5..a7ec2d7509 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -155,6 +155,8 @@ E('ERR_PARSE_HISTORY_DATA',
   (oldHistoryPath) => `Could not parse history data in ${oldHistoryPath}`);
 E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed');
 E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed');
+E('ERR_HTTP_TRAILER_INVALID',
+  'Trailers are invalid with this transfer encoding');
 E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`);
 E('ERR_UNKNOWN_SIGNAL', (signal) => `Unknown signal: ${signal}`);
 E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type');
diff --git a/test/parallel/test-http-server-de-chunked-trailer.js b/test/parallel/test-http-server-de-chunked-trailer.js
new file mode 100644
index 0000000000..d483297c5a
--- /dev/null
+++ b/test/parallel/test-http-server-de-chunked-trailer.js
@@ -0,0 +1,33 @@
+'use strict';
+const common = require('../common');
+
+// This test ensures that a Trailer header is set only when a chunked transfer
+// encoding is used.
+
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  res.setHeader('Trailer', 'baz');
+  const trailerInvalidErr = {
+    code: 'ERR_HTTP_TRAILER_INVALID',
+    message: 'Trailers are invalid with this transfer encoding',
+    type: Error
+  };
+  assert.throws(() => res.writeHead(200, {'Content-Length': '2'}),
+                common.expectsError(trailerInvalidErr));
+  res.removeHeader('Trailer');
+  res.end('ok');
+}));
+server.listen(0, common.mustCall(() => {
+  http.get({ port: server.address().port }, common.mustCall((res) => {
+    assert.strictEqual(res.statusCode, 200);
+    let buf = '';
+    res.on('data', (chunk) => {
+      buf += chunk;
+    }).on('end', common.mustCall(() => {
+      assert.strictEqual(buf, 'ok');
+    }));
+    server.close();
+  }));
+}));