http: guard against response splitting in trailers

lib/_http_outgoing.js

@@ -295,11 +295,7 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
 };
 
 function storeHeader(self, state, field, value) {
-  // Protect against response splitting. The if statement is there to
-  // minimize the performance impact in the common case.
-  if (/[\r\n]/.test(value))
-    value = value.replace(/[\r\n]+[ \t]*/g, '');
-
+  value = escapeHeaderValue(value);
   state.messageHeader += field + ': ' + value + CRLF;
 
   if (connectionExpression.test(field)) {
@@ -481,6 +477,13 @@ function connectionCorkNT(conn) {
 }
 
 
+function escapeHeaderValue(value) {
+  // Protect against response splitting. The regex test is there to
+  // minimize the performance impact in the common case.
+  return /[\r\n]/.test(value) ? value.replace(/[\r\n]+[ \t]*/g, '') : value;
+}
+
+
 OutgoingMessage.prototype.addTrailers = function(headers) {
   this._trailer = '';
   var keys = Object.keys(headers);
@@ -496,7 +499,7 @@ OutgoingMessage.prototype.addTrailers = function(headers) {
       value = headers[key];
     }
 
-    this._trailer += field + ': ' + value + CRLF;
+    this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF;
   }
 };
 

test/parallel/test-http-header-response-splitting.js

@@ -3,44 +3,85 @@ var common = require('../common'),
     assert = require('assert'),
     http = require('http');
 
-var testIndex = 0,
-    responses = 0;
+var testIndex = 0;
+const testCount = 2 * 4 * 6;
+const responseBody = 'Hi mars!';
 
 var server = http.createServer(function(req, res) {
-  switch (testIndex++) {
+  function reply(header) {
+    switch (testIndex % 4) {
     case 0:
-      res.writeHead(200, { test: 'foo \r\ninvalid: bar' });
+      res.writeHead(200, { a: header, b: header });
       break;
     case 1:
-      res.writeHead(200, { test: 'foo \ninvalid: bar' });
+      res.setHeader('a', header);
+      res.setHeader('b', header);
+      res.writeHead(200);
       break;
     case 2:
-      res.writeHead(200, { test: 'foo \rinvalid: bar' });
+      res.setHeader('a', header);
+      res.writeHead(200, { b: header });
       break;
     case 3:
-      res.writeHead(200, { test: 'foo \n\n\ninvalid: bar' });
+      res.setHeader('a', [header]);
+      res.writeHead(200, { b: header });
+      break;
+    default:
+      assert.fail('unreachable');
+    }
+    res.write(responseBody);
+    if (testIndex % 8 < 4) {
+      res.addTrailers({ ta: header, tb: header });
+    } else {
+      res.addTrailers([['ta', header], ['tb', header]]);
+    }
+    res.end();
+  }
+  switch ((testIndex / 8) | 0) {
+    case 0:
+      reply('foo \r\ninvalid: bar');
+      break;
+    case 1:
+      reply('foo \ninvalid: bar');
+      break;
+    case 2:
+      reply('foo \rinvalid: bar');
+      break;
+    case 3:
+      reply('foo \n\n\ninvalid: bar');
       break;
     case 4:
-      res.writeHead(200, { test: 'foo \r\n \r\n \r\ninvalid: bar' });
-      server.close();
+      reply('foo \r\n \r\n \r\ninvalid: bar');
+      break;
+    case 5:
+      reply('foo \r \n \r \n \r \ninvalid: bar');
       break;
     default:
       assert(false);
   }
-  res.end('Hi mars!');
+  if (++testIndex === testCount) {
+    server.close();
+  }
 });
 
-server.listen(common.PORT, function() {
-  for (var i = 0; i < 5; i++) {
-    var req = http.get({ port: common.PORT, path: '/' }, function(res) {
-      assert.strictEqual(res.headers.test, 'foo invalid: bar');
+server.listen(common.PORT, common.mustCall(function() {
+  for (var i = 0; i < testCount; i++) {
+    http.get({ port: common.PORT, path: '/' }, common.mustCall(function(res) {
+      assert.strictEqual(res.headers.a, 'foo invalid: bar');
+      assert.strictEqual(res.headers.b, 'foo invalid: bar');
+      assert.strictEqual(res.headers.foo, undefined);
       assert.strictEqual(res.headers.invalid, undefined);
-      responses++;
+      var data = '';
+      res.setEncoding('utf8');
+      res.on('data', function(s) { data += s; });
+      res.on('end', common.mustCall(function() {
+        assert.equal(data, responseBody);
+        assert.strictEqual(res.trailers.ta, 'foo invalid: bar');
+        assert.strictEqual(res.trailers.tb, 'foo invalid: bar');
+        assert.strictEqual(res.trailers.foo, undefined);
+        assert.strictEqual(res.trailers.invalid, undefined);
+      }));
       res.resume();
-    });
+    }));
   }
-});
-
-process.on('exit', function() {
-  assert.strictEqual(responses, 5);
-});
+}));