Skip to content

http: append Cookie header values with semicolon #11259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mscdex merged 2 commits into from
Mar 9, 2017
Merged

http: append Cookie header values with semicolon #11259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b2cef6
http: append Cookie header values with semicolon
mscdex Feb 9, 2017
d348077
http: concatenate outgoing Cookie headers
mscdex Feb 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
http: concatenate outgoing Cookie headers 
This commit enables automatic concatenation of multiple Cookie header
values with a semicolon, except when 2D header arrays are used.

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
mscdex committed Mar 9, 2017
commit d3480776c70bb1622afee5a2a5f39abf9e42a507
41 changes: 33 additions & 8 deletions lib/_http_outgoing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor

@thefourtheye thefourtheye Feb 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about lowercasing the string and comparing?

Copy link
Copy Markdown
Contributor Author

@mscdex mscdex Feb 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toLowerCase() + compare is very slow (slower than a case-insensitive regexp IIRC).

// 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) {
Expand Down Expand Up @@ -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++) {
Expand All @@ -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);
}
}

Expand Down
129 changes: 79 additions & 50 deletions test/parallel/test-http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});