From 3827ad583fbed35a3c96d063b11b3ed1935f3a31 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 27 Sep 2015 11:54:57 -0700 Subject: [PATCH 1/5] http: add callback is function check We were checking that the callback existed, but not checking that it was a function --- lib/_http_outgoing.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index dde832ee072427..3479524cf2d314 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -89,7 +89,7 @@ exports.OutgoingMessage = OutgoingMessage; OutgoingMessage.prototype.setTimeout = function(msecs, callback) { - if (callback) + if (typeof callback === 'function') this.on('timeout', callback); if (!this.socket) { From 4eed2423447ff1da61f2ebae70a166934433a918 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 27 Sep 2015 11:55:47 -0700 Subject: [PATCH 2/5] http: do not allow multiple instances of certain response headers Response headers such as ETag and Last-Modified do not permit multiple instances, and therefore the comma-separated syntax is not allowed. --- lib/_http_incoming.js | 6 ++++ .../test-http-response-multiheaders.js | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 test/parallel/test-http-response-multiheaders.js diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index e16f198dba71bd..5377c84d5d3c7e 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -152,6 +152,12 @@ IncomingMessage.prototype._addHeaderLine = function(field, value, dest) { case 'from': case 'location': case 'max-forwards': + case 'retry-after': + case 'etag': + case 'last-modified': + case 'server': + case 'age': + case 'expires': // drop duplicates if (dest[field] === undefined) dest[field] = value; diff --git a/test/parallel/test-http-response-multiheaders.js b/test/parallel/test-http-response-multiheaders.js new file mode 100644 index 00000000000000..eb3d1a8fd0cabb --- /dev/null +++ b/test/parallel/test-http-response-multiheaders.js @@ -0,0 +1,33 @@ +'use strict'; + +const http = require('http'); +const common = require('../common'); +const assert = require('assert'); + +// Test that certain response header fields do not repeat +const norepeat = [ + 'retry-after', + 'etag', + 'last-modified', + 'server', + 'age', + 'expires' +]; + +const server = http.createServer(function(req, res) { + for (let name of norepeat) { + res.setHeader(name, ['A', 'B']); + } + res.setHeader('X-A', ['A', 'B']); + res.end('ok'); +}); + +server.listen(common.PORT, function() { + http.get({port:common.PORT}, function(res) { + for (let name of norepeat) { + assert.equal(res.headers[name], 'A'); + } + assert.equal(res.headers['x-a'], 'A, B'); + server.close(); + }); +}); From 5333b54c5345e3c024317fbd14400b75b451daeb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 27 Sep 2015 12:51:12 -0700 Subject: [PATCH 3/5] test: cleanup nits in test-http-response-multiheaders.js Address the feedback from @thefourtheye --- test/parallel/test-http-response-multiheaders.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-http-response-multiheaders.js b/test/parallel/test-http-response-multiheaders.js index eb3d1a8fd0cabb..5dd6bb07a8ddb1 100644 --- a/test/parallel/test-http-response-multiheaders.js +++ b/test/parallel/test-http-response-multiheaders.js @@ -1,7 +1,7 @@ 'use strict'; -const http = require('http'); const common = require('../common'); +const http = require('http'); const assert = require('assert'); // Test that certain response header fields do not repeat @@ -22,12 +22,12 @@ const server = http.createServer(function(req, res) { res.end('ok'); }); -server.listen(common.PORT, function() { - http.get({port:common.PORT}, function(res) { +server.listen(common.PORT, common.mustCall(function() { + http.get({port:common.PORT}, common.mustCall(function(res) { + server.close(); for (let name of norepeat) { assert.equal(res.headers[name], 'A'); } assert.equal(res.headers['x-a'], 'A, B'); - server.close(); - }); -}); + })); +})); From 65047e0c62cec99dbd3d6eb8f6b7d7c2887f7691 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 6 Oct 2015 08:44:56 -0700 Subject: [PATCH 4/5] http: if callback is truthy but not a function, throw in `setTimeout`, if callback is truthy but not a function, throw a TypeError per @bnoordhuis' suggestion --- lib/_http_outgoing.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 3479524cf2d314..283a5ab824b19f 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -89,8 +89,12 @@ exports.OutgoingMessage = OutgoingMessage; OutgoingMessage.prototype.setTimeout = function(msecs, callback) { - if (typeof callback === 'function') + + if (callback) { + if (typeof callback !== 'function') + throw new TypeError('callback must be a function'); this.on('timeout', callback); + } if (!this.socket) { this.once('socket', function(socket) { From 4652af7bb545b4842254348030c60f0d2b0548bc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 6 Oct 2015 08:46:14 -0700 Subject: [PATCH 5/5] test: expand test-http-response-multiheaders test Include writeHead in the test per @bnoordhuis' suggestion --- .../test-http-response-multiheaders.js | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-http-response-multiheaders.js b/test/parallel/test-http-response-multiheaders.js index 5dd6bb07a8ddb1..da6cd73a38ffaa 100644 --- a/test/parallel/test-http-response-multiheaders.js +++ b/test/parallel/test-http-response-multiheaders.js @@ -15,19 +15,40 @@ const norepeat = [ ]; const server = http.createServer(function(req, res) { - for (let name of norepeat) { - res.setHeader(name, ['A', 'B']); + var num = req.headers['x-num']; + if (num == 1) { + for (let name of norepeat) { + res.setHeader(name, ['A', 'B']); + } + res.setHeader('X-A', ['A', 'B']); + } else if (num == 2) { + let headers = {}; + for (let name of norepeat) { + headers[name] = ['A', 'B']; + } + headers['X-A'] = ['A', 'B']; + res.writeHead(200, headers); } - res.setHeader('X-A', ['A', 'B']); res.end('ok'); }); server.listen(common.PORT, common.mustCall(function() { - http.get({port:common.PORT}, common.mustCall(function(res) { - server.close(); - for (let name of norepeat) { - assert.equal(res.headers[name], 'A'); - } - assert.equal(res.headers['x-a'], 'A, B'); - })); + for (let n = 1; n <= 2 ; n++) { + // this runs twice, the first time, the server will use + // setHeader, the second time it uses writeHead. The + // result on the client side should be the same in + // either case -- only the first instance of the header + // value should be reported for the header fields listed + // in the norepeat array. + http.get( + {port:common.PORT, headers:{'x-num': n}}, + common.mustCall(function(res) { + if (n == 2) server.close(); + for (let name of norepeat) { + assert.equal(res.headers[name], 'A'); + } + assert.equal(res.headers['x-a'], 'A, B'); + }) + ); + } }));