From 66f4a682046e72d34c0011751e9a266936163a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pollak?= Date: Sat, 2 Oct 2021 01:12:18 -0300 Subject: [PATCH 1/4] Allow setting a default agent for all requests via defaults() --- lib/needle.js | 19 ++++-- test/headers_spec.js | 135 +++++++++++++++++++++++++++++++++++---- test/output_spec.js | 7 ++ test/redirect_spec.js | 2 +- test/tls_options_spec.js | 57 +++++++++++++++++ 5 files changed, 199 insertions(+), 21 deletions(-) create mode 100644 test/tls_options_spec.js diff --git a/lib/needle.js b/lib/needle.js index b56c8f66b..f11739811 100644 --- a/lib/needle.js +++ b/lib/needle.js @@ -26,7 +26,7 @@ var version = require('../package.json').version; var user_agent = 'Needle/' + version; user_agent += ' (Node.js ' + process.version + '; ' + process.platform + ' ' + process.arch + ')'; -var tls_options = 'agent pfx key passphrase cert ca ciphers rejectUnauthorized secureProtocol checkServerIdentity family'; +var tls_options = 'pfx key passphrase cert ca ciphers rejectUnauthorized secureProtocol checkServerIdentity family'; // older versions of node (< 0.11.4) prevent the runtime from exiting // because of connections in keep-alive state. so if this is the case @@ -83,7 +83,8 @@ var defaults = { parse_response : 'all', // same as true. valid options: 'json', 'xml' or false/null proxy : null, - // headers + // agent & headers + agent : null, headers : {}, accept : '*/*', user_agent : user_agent, @@ -233,6 +234,7 @@ Needle.prototype.setup = function(uri, options) { var config = { http_opts : { + agent: get_option('agent', defaults.agent), localAddress: get_option('localAddress', undefined), lookup: get_option('lookup', undefined) }, // passed later to http.request() directly @@ -254,9 +256,14 @@ Needle.prototype.setup = function(uri, options) { // populate http_opts with given TLS options tls_options.split(' ').forEach(function(key) { if (typeof options[key] != 'undefined') { - config.http_opts[key] = options[key]; - if (typeof options.agent == 'undefined') - config.http_opts.agent = false; // otherwise tls options are skipped + if (config.http_opts.agent) { // pass option to existing agent + config.http_opts.agent.options[key] = options[key]; + } else { + config.http_opts[key] = options[key]; + // by passing { agent: false } to http.request(), a one-time use Agent is initialized + // and used, meaning we opt out of connection pooling, but given tls options actually work + config.http_opts.agent = false; + } } }); @@ -846,7 +853,7 @@ module.exports.defaults = function(obj) { var target_key = aliased.options[key] || key; if (defaults.hasOwnProperty(target_key) && typeof obj[key] != 'undefined') { - if (target_key != 'parse_response' && target_key != 'proxy') { + if (target_key != 'parse_response' && target_key != 'proxy' && target_key != 'agent') { // ensure type matches the original, except for proxy/parse_response that can be null/bool or string var valid_type = defaults[target_key].constructor.name; diff --git a/test/headers_spec.js b/test/headers_spec.js index a5c17e62d..2a1e38982 100644 --- a/test/headers_spec.js +++ b/test/headers_spec.js @@ -4,6 +4,7 @@ var http = require('http'), var port = 54321; + describe('request headers', function() { var needle, @@ -38,6 +39,7 @@ describe('request headers', function() { describe('old node versions (<0.11.4) with persistent keep-alive connections', function() { + // emulate old node behaviour before(function() { delete require.cache[require.resolve('..')] // in case it was already loaded original_defaultMaxSockets = http.Agent.defaultMaxSockets; @@ -133,23 +135,47 @@ describe('request headers', function() { describe('default options', function() { - // TODO: - // this is weird. by default, new node versions set a 'close' header - // while older versions set a keep-alive header + var node_major_ver = process.version.split('.')[0].replace('v', ''); - it.skip('sets a Connection header', function(done) { - send_request({}, function(err, resp) { - // should.not.exist(resp.body.headers['connection']); - // done(); + if (parseInt(node_major_ver) >= 4) { + + it('sets Connection header to close (> v4)', function(done) { + send_request({}, function(err, resp) { + resp.body.headers['connection'].should.eql('close'); + done() + }) }) - }) - it.skip('one open sockets remain after request', function(done) { - send_request({}, function(err, resp) { - // get_active_sockets().length.should.eql(1); - // done(); - }); - }) + } else { + + it('sets Connection header to keep-alive (< v4)', function(done) { + send_request({}, function(err, resp) { + resp.body.headers['connection'].should.eql('keep-alive'); + done(); + }) + }) + + } + + if (parseInt(node_major_ver) >= 8 || parseInt(node_major_ver) == 0) { + + it('one open socket remains after request (> v8 && v0.10)', function(done) { + send_request({}, function(err, resp) { + get_active_sockets().length.should.eql(1); + done(); + }); + }) + + } else { + + it('no open sockets remain after request (> v0.10 && < v8)', function(done) { + send_request({}, function(err, resp) { + get_active_sockets().length.should.eql(0); + done(); + }); + }) + + } }) @@ -200,4 +226,85 @@ describe('request headers', function() { }) + describe('using shared keep-alive agent', function() { + + before(function() { + needle.defaults({ agent: http.Agent({ keepAlive: true }) }) + }) + + after(function() { + needle.defaults().agent.destroy(); // close existing connections + needle.defaults({ agent: null }); // and reset default value + }) + + describe('default options', function() { + + it('sends a Connection: keep-alive header', function(done) { + send_request({}, function(err, resp) { + resp.body.headers['connection'].should.eql('keep-alive'); + done(); + }) + }) + + it('one open socket remain after request', function(done) { + send_request({ connection: 'keep-alive' }, function(err, resp) { + setTimeout(function() { + get_active_sockets().length.should.eql(existing_sockets + 1); + done(); + }, 10); + }); + }) + + }) + + describe('passing connection: close', function() { + + it('sends a Connection: close header', function(done) { + send_request({ connection: 'close' }, function(err, resp) { + resp.body.headers['connection'].should.eql('close'); + done(); + }) + }) + + it('no open sockets remain after request', function(done) { + send_request({ connection: 'close' }, function(err, resp) { + setTimeout(function() { + get_active_sockets().length.should.eql(existing_sockets); + done(); + }, 10) + }); + }) + + }) + + describe('passing connection: keep-alive', function() { + + it('sends a Connection: keep-alive header (using options.headers.connection)', function(done) { + send_request({ headers: { connection: 'keep-alive' }}, function(err, resp) { + resp.body.headers['connection'].should.eql('keep-alive'); + done(); + }) + }) + + it('sends a Connection: keep-alive header (using options.connection)', function(done) { + send_request({ connection: 'keep-alive' }, function(err, resp) { + resp.body.headers['connection'].should.eql('keep-alive'); + done(); + }) + }) + + it('one open socket remain after request', function(done) { + send_request({ connection: 'keep-alive' }, function(err, resp) { + setTimeout(function() { + get_active_sockets().length.should.eql(existing_sockets + 1); + done(); + }, 10); + }) + }) + + }) + + + }) + }) diff --git a/test/output_spec.js b/test/output_spec.js index 14fc5bb03..64b7be849 100644 --- a/test/output_spec.js +++ b/test/output_spec.js @@ -1,3 +1,10 @@ +// this lets us run tests in ancient node versions (v0.10.x) +if (process.version.split('.')[0] == 'v0' && !Buffer.from) { + Buffer.from = function(args) { + return new Buffer(args); + } +} + var should = require('should'), needle = require('./../'), http = require('http'), diff --git a/test/redirect_spec.js b/test/redirect_spec.js index c3942e279..7d61747f2 100644 --- a/test/redirect_spec.js +++ b/test/redirect_spec.js @@ -65,7 +65,7 @@ describe('redirects', function() { url = protocol + '://' + host + ':' + ports[protocol] + '/hello'; function send_request(opts, cb) { - opts.rejectUnauthorized = false; + if (protocol == 'https') opts.rejectUnauthorized = false; // console.log(' -- sending request ' + url + ' -- redirect to ' + location); needle.post(url, { foo: 'bar' }, opts, cb); } diff --git a/test/tls_options_spec.js b/test/tls_options_spec.js new file mode 100644 index 000000000..208f82995 --- /dev/null +++ b/test/tls_options_spec.js @@ -0,0 +1,57 @@ +var needle = require('..'), + https = require('https'), + helpers = require('./helpers'), + should = require('should'); + +describe('tls options', function() { + + describe('rejectUnauthorized: false', function() { + + var url = 'https://expired-rsa-dv.ssl.com/'; + + it('is an expired cert', function(done) { + needle.get(url, function(err, resp) { + err.code.should.eql('CERT_HAS_EXPIRED') + should.not.exist(resp) + done() + }) + }) + + it('allows fetching pages under expired certificates', function(done) { + needle.get(url, { rejectUnauthorized: false }, function(err, resp) { + should.not.exist(err); + resp.statusCode.should.eql(200); + done() + }) + }) + + it('also works when using custom agent', function(done) { + var agent = new https.Agent({ rejectUnauthorized: true }) + + // should overwrite value from custom agent + needle.get(url, { rejectUnauthorized: false }, function(err, resp) { + should.not.exist(err); + resp.statusCode.should.eql(200); + done() + }) + + }) + + it('also works with shared/default agent', function(done) { + var agent = new https.Agent({ rejectUnauthorized: true }) + needle.defaults({ agent: agent }) + + // should overwrite value from custom agent + needle.get(url, { rejectUnauthorized: false }, function(err, resp) { + should.not.exist(err); + resp.statusCode.should.eql(200); + + needle.defaults({ agent: null }) + done() + }) + + }) + + }) + +}) \ No newline at end of file From 6029d47ab3c4110d555fa36c90fa08ac24e475f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pollak?= Date: Sat, 2 Oct 2021 01:22:57 -0300 Subject: [PATCH 2/4] Apparently setting agent: false when TLS options are given is not needed anymore --- lib/needle.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/needle.js b/lib/needle.js index f11739811..a5899dcc6 100644 --- a/lib/needle.js +++ b/lib/needle.js @@ -260,9 +260,6 @@ Needle.prototype.setup = function(uri, options) { config.http_opts.agent.options[key] = options[key]; } else { config.http_opts[key] = options[key]; - // by passing { agent: false } to http.request(), a one-time use Agent is initialized - // and used, meaning we opt out of connection pooling, but given tls options actually work - config.http_opts.agent = false; } } }); From fb84652c4232f87e99335f27637541543d3c4561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pollak?= Date: Sat, 2 Oct 2021 01:35:33 -0300 Subject: [PATCH 3/4] Update test/headers_spec.js --- test/headers_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/headers_spec.js b/test/headers_spec.js index 2a1e38982..d604d3915 100644 --- a/test/headers_spec.js +++ b/test/headers_spec.js @@ -161,7 +161,7 @@ describe('request headers', function() { it('one open socket remains after request (> v8 && v0.10)', function(done) { send_request({}, function(err, resp) { - get_active_sockets().length.should.eql(1); + get_active_sockets().length.should.eql(existing_sockets + 1); done(); }); }) @@ -170,7 +170,7 @@ describe('request headers', function() { it('no open sockets remain after request (> v0.10 && < v8)', function(done) { send_request({}, function(err, resp) { - get_active_sockets().length.should.eql(0); + get_active_sockets().length.should.eql(existing_sockets); done(); }); }) From 4fff2227c4f5db12a3a8dd2f135d4a22fc69224d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Pollak?= Date: Tue, 2 Nov 2021 08:49:20 -0300 Subject: [PATCH 4/4] Update package-lock.json --- package-lock.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index bff431c90..48e6a1103 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,9 +15,9 @@ } }, "balanced-match": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz", - "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", + "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "dev": true }, "brace-expansion": { @@ -38,7 +38,7 @@ }, "commander": { "version": "2.15.1", - "resolved": "http://registry.npmjs.org/commander/-/commander-2.15.1.tgz", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.15.1.tgz", "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag==", "dev": true }, @@ -140,9 +140,9 @@ } }, "inherits": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.3.tgz", - "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", + "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", "dev": true }, "isarray": { @@ -180,13 +180,13 @@ }, "minimist": { "version": "0.0.8", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", "dev": true }, "mkdirp": { "version": "0.5.1", - "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "dev": true, "requires": {