http: append Cookie header values with semicolon

lib/_http_incoming.js

@@ -194,6 +194,9 @@ function matchKnownFields(field) {
       case 'Set-Cookie':
       case 'set-cookie':
         return '\u0001';
+      case 'Cookie':
+      case 'cookie':
+        return '\u0002cookie';
       // The fields below are not used in _addHeaderLine(), but they are common
       // headers where we can avoid toLowerCase() if the mixed or lower case
       // versions match the first time through.
@@ -215,9 +218,6 @@ function matchKnownFields(field) {
       case 'Content-Encoding':
       case 'content-encoding':
         return '\u0000content-encoding';
-      case 'Cookie':
-      case 'cookie':
-        return '\u0000cookie';
       case 'Origin':
       case 'origin':
         return '\u0000origin';
@@ -263,18 +263,20 @@ function matchKnownFields(field) {
 //
 // Per RFC2616, section 4.2 it is acceptable to join multiple instances of the
 // same header with a ', ' if the header in question supports specification of
-// multiple values this way. If not, we declare the first instance the winner
-// and drop the second. Extended header fields (those beginning with 'x-') are
-// always joined.
+// multiple values this way. The one exception to this is the Cookie header,
+// which has multiple values joined with a '; ' instead. If a header's values
+// cannot be joined in either of these ways, we declare the first instance the
+// winner and drop the second. Extended header fields (those beginning with
+// 'x-') are always joined.
 IncomingMessage.prototype._addHeaderLine = _addHeaderLine;
 function _addHeaderLine(field, value, dest) {
   field = matchKnownFields(field);
   var flag = field.charCodeAt(0);
-  if (flag === 0) {
+  if (flag === 0 || flag === 2) {
     field = field.slice(1);
-    // Make comma-separated list
+    // Make a delimited list
     if (typeof dest[field] === 'string') {
-      dest[field] += ', ' + value;
+      dest[field] += (flag === 0 ? ', ' : '; ') + value;
     } else {
       dest[field] = value;
     }

lib/_http_outgoing.js

@@ -20,6 +20,27 @@ var RE_FIELDS = new RegExp('^(?:Connection|Transfer-Encoding|Content-Length|' +
 var RE_CONN_VALUES = /(?:^|\W)close|upgrade(?:$|\W)/ig;
 var RE_TE_CHUNKED = common.chunkExpression;
 
+// isCookieField performs a case-insensitive comparison of a provided string
+// against the word "cookie." This method (at least as of V8 5.4) is faster than
+// the equivalent case-insensitive regexp, even if isCookieField does not get
+// inlined.
+function isCookieField(s) {
+  if (s.length !== 6) return false;
+  var ch = s.charCodeAt(0);
+  if (ch !== 99 && ch !== 67) return false;
+  ch = s.charCodeAt(1);
+  if (ch !== 111 && ch !== 79) return false;
+  ch = s.charCodeAt(2);
+  if (ch !== 111 && ch !== 79) return false;
+  ch = s.charCodeAt(3);
+  if (ch !== 107 && ch !== 75) return false;
+  ch = s.charCodeAt(4);
+  if (ch !== 105 && ch !== 73) return false;
+  ch = s.charCodeAt(5);
+  if (ch !== 101 && ch !== 69) return false;
+  return true;
+}
+
 var dateCache;
 function utcDate() {
   if (!dateCache) {
@@ -275,12 +296,14 @@ function _storeHeader(firstLine, headers) {
       value = entry[1];
 
       if (value instanceof Array) {
-        for (j = 0; j < value.length; j++) {
-          storeHeader(this, state, field, value[j], false);
+        if (value.length < 2 || !isCookieField(field)) {
+          for (j = 0; j < value.length; j++)
+            storeHeader(this, state, field, value[j], false);
+          continue;
         }
-      } else {
-        storeHeader(this, state, field, value, false);
+        value = value.join('; ');
       }
+      storeHeader(this, state, field, value, false);
     }
   } else if (headers instanceof Array) {
     for (i = 0; i < headers.length; i++) {
@@ -302,12 +325,14 @@ function _storeHeader(firstLine, headers) {
       value = headers[field];
 
       if (value instanceof Array) {
-        for (j = 0; j < value.length; j++) {
-          storeHeader(this, state, field, value[j], true);
+        if (value.length < 2 || !isCookieField(field)) {
+          for (j = 0; j < value.length; j++)
+            storeHeader(this, state, field, value[j], true);
+          continue;
         }
-      } else {
-        storeHeader(this, state, field, value, true);
+        value = value.join('; ');
       }
+      storeHeader(this, state, field, value, true);
     }
   }
 

test/parallel/test-http-incoming-matchKnownFields.js

@@ -6,9 +6,10 @@ const IncomingMessage = require('http').IncomingMessage;
 function checkDest(field, result, value) {
   const dest = {};
 
-  if (value) dest[field] = 'test';
   const incomingMessage = new IncomingMessage(field);
   // dest is changed by IncomingMessage._addHeaderLine
+  if (value)
+    incomingMessage._addHeaderLine(field, 'test', dest);
   incomingMessage._addHeaderLine(field, value, dest);
   assert.deepStrictEqual(dest, result);
 }
@@ -49,7 +50,7 @@ checkDest('age', {age: 'test'}, 'value');
 checkDest('Expires', {expires: undefined});
 checkDest('expires', {expires: 'test'}, 'value');
 checkDest('Set-Cookie', {'set-cookie': [undefined]});
-checkDest('set-cookie', {'set-cookie': [undefined]});
+checkDest('set-cookie', {'set-cookie': ['test', 'value']}, 'value');
 checkDest('Transfer-Encoding', {'transfer-encoding': undefined});
 checkDest('transfer-encoding', {'transfer-encoding': 'test, value'}, 'value');
 checkDest('Date', {date: undefined});
@@ -64,8 +65,8 @@ checkDest('Vary', {vary: undefined});
 checkDest('vary', {vary: 'test, value'}, 'value');
 checkDest('Content-Encoding', {'content-encoding': undefined}, undefined);
 checkDest('content-encoding', {'content-encoding': 'test, value'}, 'value');
-checkDest('Cookies', {cookies: undefined});
-checkDest('cookies', {cookies: 'test, value'}, 'value');
+checkDest('Cookie', {cookie: undefined});
+checkDest('cookie', {cookie: 'test; value'}, 'value');
 checkDest('Origin', {origin: undefined});
 checkDest('origin', {origin: 'test, value'}, 'value');
 checkDest('Upgrade', {upgrade: undefined});
@@ -88,3 +89,5 @@ checkDest('X-Forwarded-Host', {'x-forwarded-host': undefined});
 checkDest('x-forwarded-host', {'x-forwarded-host': 'test, value'}, 'value');
 checkDest('X-Forwarded-Proto', {'x-forwarded-proto': undefined});
 checkDest('x-forwarded-proto', {'x-forwarded-proto': 'test, value'}, 'value');
+checkDest('X-Foo', {'x-foo': undefined});
+checkDest('x-foo', {'x-foo': 'test, value'}, 'value');

test/parallel/test-http-server-multiheaders2.js

@@ -54,8 +54,10 @@ const srv = http.createServer(function(req, res) {
                        'foo', 'header parsed incorrectly: ' + header);
   });
   multipleAllowed.forEach(function(header) {
+    const sep = (header.toLowerCase() === 'cookie' ? '; ' : ', ');
     assert.strictEqual(req.headers[header.toLowerCase()],
-                       'foo, bar', 'header parsed incorrectly: ' + header);
+                       'foo' + sep + 'bar',
+                       'header parsed incorrectly: ' + header);
   });
 
   res.writeHead(200, {'Content-Type': 'text/plain'});

test/parallel/test-http.js

@@ -1,84 +1,113 @@
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const http = require('http');
 const url = require('url');
 
-let responses_sent = 0;
-let responses_recvd = 0;
-let body0 = '';
-let body1 = '';
+const expectedRequests = ['/hello', '/there', '/world'];
 
-const server = http.Server(function(req, res) {
-  if (responses_sent === 0) {
-    assert.strictEqual('GET', req.method);
-    assert.strictEqual('/hello', url.parse(req.url).pathname);
+const server = http.Server(common.mustCall(function(req, res) {
+  assert.strictEqual(expectedRequests.shift(), req.url);
 
-    console.dir(req.headers);
-    assert.strictEqual(true, 'accept' in req.headers);
-    assert.strictEqual('*/*', req.headers['accept']);
-
-    assert.strictEqual(true, 'foo' in req.headers);
-    assert.strictEqual('bar', req.headers['foo']);
+  switch (req.url) {
+    case '/hello':
+      assert.strictEqual(req.method, 'GET');
+      assert.strictEqual(req.headers['accept'], '*/*');
+      assert.strictEqual(req.headers['foo'], 'bar');
+      assert.strictEqual(req.headers.cookie, 'foo=bar; bar=baz; baz=quux');
+      break;
+    case '/there':
+      assert.strictEqual(req.method, 'PUT');
+      assert.strictEqual(req.headers.cookie, 'node=awesome; ta=da');
+      break;
+    case '/world':
+      assert.strictEqual(req.method, 'POST');
+      assert.deepStrictEqual(req.headers.cookie, 'abc=123; def=456; ghi=789');
+      break;
+    default:
+      assert(false, `Unexpected request for ${req.url}`);
   }
 
-  if (responses_sent === 1) {
-    assert.strictEqual('POST', req.method);
-    assert.strictEqual('/world', url.parse(req.url).pathname);
+  if (expectedRequests.length === 0)
     this.close();
-  }
 
   req.on('end', function() {
     res.writeHead(200, {'Content-Type': 'text/plain'});
     res.write('The path was ' + url.parse(req.url).pathname);
     res.end();
-    responses_sent += 1;
   });
   req.resume();
-
-  //assert.strictEqual('127.0.0.1', res.connection.remoteAddress);
-});
+}, 3));
 server.listen(0);
 
 server.on('listening', function() {
   const agent = new http.Agent({ port: this.address().port, maxSockets: 1 });
-  http.get({
+  const req = http.get({
     port: this.address().port,
     path: '/hello',
-    headers: {'Accept': '*/*', 'Foo': 'bar'},
+    headers: {
+      Accept: '*/*',
+      Foo: 'bar',
+      Cookie: [ 'foo=bar', 'bar=baz', 'baz=quux' ]
+    },
     agent: agent
-  }, function(res) {
-    assert.strictEqual(200, res.statusCode);
-    responses_recvd += 1;
+  }, common.mustCall(function(res) {
+    const cookieHeaders = req._header.match(/^Cookie: .+$/img);
+    assert.deepStrictEqual(cookieHeaders,
+                           ['Cookie: foo=bar; bar=baz; baz=quux']);
+    assert.strictEqual(res.statusCode, 200);
+    let body = '';
     res.setEncoding('utf8');
-    res.on('data', function(chunk) { body0 += chunk; });
-    console.error('Got /hello response');
-  });
+    res.on('data', function(chunk) { body += chunk; });
+    res.on('end', common.mustCall(function() {
+      assert.strictEqual(body, 'The path was /hello');
+    }));
+  }));
 
-  setTimeout(function() {
+  setTimeout(common.mustCall(function() {
+    const req = http.request({
+      port: server.address().port,
+      method: 'PUT',
+      path: '/there',
+      agent: agent
+    }, common.mustCall(function(res) {
+      const cookieHeaders = req._header.match(/^Cookie: .+$/img);
+      assert.deepStrictEqual(cookieHeaders, ['Cookie: node=awesome; ta=da']);
+      assert.strictEqual(res.statusCode, 200);
+      let body = '';
+      res.setEncoding('utf8');
+      res.on('data', function(chunk) { body += chunk; });
+      res.on('end', common.mustCall(function() {
+        assert.strictEqual(body, 'The path was /there');
+      }));
+    }));
+    req.setHeader('Cookie', ['node=awesome', 'ta=da']);
+    req.end();
+  }), 1);
+
+  setTimeout(common.mustCall(function() {
     const req = http.request({
       port: server.address().port,
       method: 'POST',
       path: '/world',
+      headers: [ ['Cookie', 'abc=123'],
+                 ['Cookie', 'def=456'],
+                 ['Cookie', 'ghi=789'] ],
       agent: agent
-    }, function(res) {
-      assert.strictEqual(200, res.statusCode);
-      responses_recvd += 1;
+    }, common.mustCall(function(res) {
+      const cookieHeaders = req._header.match(/^Cookie: .+$/img);
+      assert.deepStrictEqual(cookieHeaders,
+                             ['Cookie: abc=123',
+                              'Cookie: def=456',
+                              'Cookie: ghi=789']);
+      assert.strictEqual(res.statusCode, 200);
+      let body = '';
       res.setEncoding('utf8');
-      res.on('data', function(chunk) { body1 += chunk; });
-      console.error('Got /world response');
-    });
+      res.on('data', function(chunk) { body += chunk; });
+      res.on('end', common.mustCall(function() {
+        assert.strictEqual(body, 'The path was /world');
+      }));
+    }));
     req.end();
-  }, 1);
-});
-
-process.on('exit', function() {
-  console.error('responses_recvd: ' + responses_recvd);
-  assert.strictEqual(2, responses_recvd);
-
-  console.error('responses_sent: ' + responses_sent);
-  assert.strictEqual(2, responses_sent);
-
-  assert.strictEqual('The path was /hello', body0);
-  assert.strictEqual('The path was /world', body1);
+  }), 2);
 });