From cd7023c3fbd26b8c467b8863e7808d6d09c8493b Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Sat, 18 Apr 2015 13:22:34 -0700 Subject: [PATCH 1/4] Revert "http: don't bother making a copy of the options" This reverts commit 06cfff935012ed2826cac56284cea982630cbc27. Reverted because it introduced a regression where (because options were modified in the later functionality) options.host and options.port would be overridden with values provided in other, supported ways. --- lib/_http_client.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/_http_client.js b/lib/_http_client.js index daa37ef064e3fc..200a08e5d5bf85 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -21,6 +21,8 @@ function ClientRequest(options, cb) { if (typeof options === 'string') { options = url.parse(options); + } else { + options = util._extend({}, options); } var agent = options.agent; From 8bccb491a52d1ccd6f48b64dc9e7fdc6db105fda Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Sat, 18 Apr 2015 13:26:15 -0700 Subject: [PATCH 2/4] test: add test for 06cfff9 regression This commit adds a test to ensure all options are NOT modified after passing them to http.request. Specifically options.host and options.port are the most prominent that would previously error, but add the other options that have default values. options.host and options.port were overridden for the one-argument net.createConnection(options) call. --- .../test-http-dont-override-options.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 test/parallel/test-http-dont-override-options.js diff --git a/test/parallel/test-http-dont-override-options.js b/test/parallel/test-http-dont-override-options.js new file mode 100644 index 00000000000000..e03ed94b5ac6ad --- /dev/null +++ b/test/parallel/test-http-dont-override-options.js @@ -0,0 +1,45 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +var requests = 0; + +http.createServer(function(req, res) { + res.writeHead(200); + res.end('ok'); + + requests++; +}).listen(common.PORT).unref(); + +var agent = new http.Agent(); +agent.defaultPort = common.PORT; + +// options marked as explicitly undefined for readability +// in this test, they should STAY undefined as options should not +// be mutable / modified +var options = { + host: undefined, + hostname: 'localhost', + port: undefined, + defaultPort: undefined, + path: undefined, + method: undefined, + agent: agent +}; + +http.request(options, function(res) { + res.resume(); +}).end(); + +process.on('exit', function() { + assert.equal(requests, 1); + + assert.strictEqual(options.host, undefined); + assert.strictEqual(options.hostname, 'localhost'); + assert.strictEqual(options.port, undefined); + assert.strictEqual(options.defaultPort, undefined); + assert.strictEqual(options.path, undefined); + assert.strictEqual(options.method, undefined); +}); From ff88a4ff45b78ee5d74d783332f736acdee86427 Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Sat, 18 Apr 2015 13:30:17 -0700 Subject: [PATCH 3/4] !fixup fix test's localhost for FreeBSD runs --- test/parallel/test-http-dont-override-options.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-dont-override-options.js b/test/parallel/test-http-dont-override-options.js index e03ed94b5ac6ad..66d82caeac930d 100644 --- a/test/parallel/test-http-dont-override-options.js +++ b/test/parallel/test-http-dont-override-options.js @@ -21,7 +21,7 @@ agent.defaultPort = common.PORT; // be mutable / modified var options = { host: undefined, - hostname: 'localhost', + hostname: common.localhostIPv4, port: undefined, defaultPort: undefined, path: undefined, @@ -37,7 +37,7 @@ process.on('exit', function() { assert.equal(requests, 1); assert.strictEqual(options.host, undefined); - assert.strictEqual(options.hostname, 'localhost'); + assert.strictEqual(options.hostname, common.localhostIPv4); assert.strictEqual(options.port, undefined); assert.strictEqual(options.defaultPort, undefined); assert.strictEqual(options.path, undefined); From 6e5cc0a322908930c77ce5d6b596a365973dfd97 Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Sat, 18 Apr 2015 13:34:29 -0700 Subject: [PATCH 4/4] !fixup more verbose test name --- ...ride-options.js => test-http-request-dont-override-options.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/parallel/{test-http-dont-override-options.js => test-http-request-dont-override-options.js} (100%) diff --git a/test/parallel/test-http-dont-override-options.js b/test/parallel/test-http-request-dont-override-options.js similarity index 100% rename from test/parallel/test-http-dont-override-options.js rename to test/parallel/test-http-request-dont-override-options.js