Permalink
Browse files

http: append Cookie header values with semicolon

Previously, separate incoming Cookie headers would be concatenated
with a comma, which can cause confusion in userland code when it
comes to parsing the final Cookie header value. This commit
concatenates using a semicolon instead.

Fixes: #11256
PR-URL: #11259
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information...
mscdex committed Feb 9, 2017
1 parent 8243ca0 commit 6b2cef65c99cb40fb9ca0789670b9ea9f5fcc2dd
View
@@ -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;
}
@@ -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');
@@ -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'});

0 comments on commit 6b2cef6

Please sign in to comment.