From aa2a585e009ba0c594402adcb3682eb442c09204 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Mon, 24 Jun 2024 17:34:30 +0100 Subject: [PATCH 1/4] should set https and http proxy agents for UI closes #4792 --- .../@node-red/nodes/core/network/21-httprequest.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js index 6ea4767cf..6561bff22 100644 --- a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js +++ b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js @@ -108,7 +108,8 @@ in your Node-RED user directory (${RED.settings.userDir}). if (n.proxy && proxyConfig) { proxyOptions.env = { no_proxy: (proxyConfig.noproxy || []).join(','), - http_proxy: (proxyConfig.url) + http_proxy: (proxyConfig.url), + https_proxy: (proxyConfig.url) } } return getProxyForUrl(url, proxyOptions) @@ -564,7 +565,7 @@ in your Node-RED user directory (${RED.settings.userDir}). //need both incase of http -> https redirect opts.agent = { http: new HttpProxyAgent(proxyOptions), - https: new HttpProxyAgent(proxyOptions) + https: new HttpsProxyAgent(proxyOptions) }; } else { From d820686e5ac8c740d898e0e577adbbcbc922c603 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Mon, 24 Jun 2024 20:04:26 +0100 Subject: [PATCH 2/4] only initialise proxy if flow url is static --- .../node_modules/@node-red/nodes/core/network/21-httprequest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js index 6561bff22..90c4134a4 100644 --- a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js +++ b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js @@ -114,7 +114,7 @@ in your Node-RED user directory (${RED.settings.userDir}). } return getProxyForUrl(url, proxyOptions) } - let prox = getProxy(nodeUrl || '') + let prox = nodeUrl ? getProxy(nodeUrl) : null let timingLog = false; if (RED.settings.hasOwnProperty("httpRequestTimingLog")) { From ee269caa4a1bfeaa39ca408acc0612369e89a9bb Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Mon, 24 Jun 2024 20:05:37 +0100 Subject: [PATCH 3/4] update and add tests to check correct proxy was returned for flow --- .../nodes/core/network/21-httprequest_spec.js | 227 ++++++++++++++++-- 1 file changed, 206 insertions(+), 21 deletions(-) diff --git a/test/nodes/core/network/21-httprequest_spec.js b/test/nodes/core/network/21-httprequest_spec.js index 7d36903a5..728cda458 100644 --- a/test/nodes/core/network/21-httprequest_spec.js +++ b/test/nodes/core/network/21-httprequest_spec.js @@ -17,6 +17,8 @@ var http = require("http"); var https = require("https"); var should = require("should"); +var sinon = require("sinon"); +var httpProxyHelper = require("nr-test-utils").require("@node-red/nodes/core/network/lib/proxyHelper.js"); var express = require("express"); var bodyParser = require('body-parser'); var stoppable = require('stoppable'); @@ -35,7 +37,7 @@ var crypto = require("crypto"); const { version } = require("os"); const net = require('net') -describe('HTTP Request Node', function() { +describe.only('HTTP Request Node', function() { var testApp; var testServer; var testPort = 10234; @@ -493,6 +495,7 @@ describe('HTTP Request Node', function() { }); afterEach(function() { + sinon.restore(); process.env.http_proxy = preEnvHttpProxyLowerCase; process.env.HTTP_PROXY = preEnvHttpProxyUpperCase; // On Windows, if environment variable of NO_PROXY that includes lower cases @@ -1799,27 +1802,80 @@ describe('HTTP Request Node', function() { }) }); - //Removing HTTP Proxy testcases as GOT + Proxy_Agent doesn't work with mock'd proxy - /* */ - it('should use http_proxy', function(done) { - var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"POST",ret:"obj",url:getTestURL('/postInspect')}, - {id:"n2", type:"helper"}]; + it('should use env var http_proxy', function(done) { + const url = getTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: url }, + { id: "n2", type: "helper" }, + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; deleteProxySetting(); - process.env.http_proxy = "http://localhost:" + testProxyPort; - helper.load(httpRequestNode, flow, function() { - var n1 = helper.getNode("n1"); - var n2 = helper.getNode("n2"); - n2.on("input", function(msg) { - try { - msg.should.have.property('statusCode',200); - msg.payload.should.have.property('headers'); - //msg.payload.headers.should.have.property('x-testproxy-header','foobar'); - done(); - } catch(err) { - done(err); - } - }); - n1.receive({payload:"foo"}); + process.env.http_proxy = proxyUrl + helper.load(testNode, flow, function (msg) { + try { + // static URL set in the nodes configuration and the proxy will be setup upon initialisation + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { }).should.be.true() + proxySpy.returnValues[0].should.be.equal(proxyUrl) + done() + } catch (err) { + done(err); + } + }); + }); + + it('should use env var https_proxy', function(done) { + const url = getSslTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: url }, + { id: "n2", type: "helper" }, + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + process.env.https_proxy = proxyUrl + helper.load(testNode, flow, function (msg) { + try { + // static URL set in the nodes configuration and the proxy will be setup upon initialisation + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { }).should.be.true() + proxySpy.returnValues[0].should.be.equal(proxyUrl) + done() + } catch (err) { + done(err); + } + }); + }); + + it('should not use env var http*_proxy when no_proxy is set', function(done) { + const url = getSslTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: url }, + { id: "n2", type: "helper" }, + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + process.env.http_proxy = proxyUrl + process.env.https_proxy = proxyUrl + process.env.no_proxy = "localhost" + helper.load(testNode, flow, function (msg) { + try { + // static URL set in the nodes configuration and the proxy will be setup upon initialisation + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { }).should.be.true() + proxySpy.returnValues[0].should.be.equal('') + done() + } catch (err) { + done(err); + } }); }); @@ -1997,6 +2053,135 @@ describe('HTTP Request Node', function() { }); }); + it('should use UI proxy for statically configured URL', function (done) { + const url = getTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: url, proxy: "n3" }, + { id: "n2", type: "helper" }, + { id: "n3", type: "http proxy", url: proxyUrl, noproxy: ["foo"] } + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + + // static URL set in the nodes configuration will cause the proxy setup to be called + // no no need to send a message to the node + helper.load(testNode, flow, function () { + try { + // ensure getProxyForUrl was called and returned the correct proxy URL + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { env: { no_proxy: "foo", http_proxy: proxyUrl, https_proxy: proxyUrl } }).should.be.true() + proxySpy.returnValues[0].should.be.equal(proxyUrl) + done(); + } catch (err) { + done(err); + } + }); + }); + it('should use UI proxy for HTTP URL passed in via msg', function (done) { + const url = getTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: "", proxy: "n3" }, + { id: "n2", type: "helper" }, + { id: "n3", type: "http proxy", url: proxyUrl, noproxy: ["foo,bar"] } + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + helper.load(testNode, flow, function () { + const n1 = helper.getNode("n1"); + const n2 = helper.getNode("n2"); + try { + proxySpy.calledOnce.should.be.false() // proxy setup should not be called when there is no URL to check needs proxying + } catch (err) { + done(err); + return + } + n2.on("input", function (msg) { + try { + // ensure getProxyForUrl was called and returned the correct proxy URL + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { env: { no_proxy: "foo,bar", http_proxy: proxyUrl, https_proxy: proxyUrl } }).should.be.true() + proxySpy.returnValues[0].should.be.equal(proxyUrl) + done(); + } catch (err) { + done(err); + } + }); + n1.receive({ url: url }); + }); + }); + it('should use UI proxy for HTTPS URL passed in via msg', function (done) { + const url = getSslTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: "", proxy: "n3" }, + { id: "n2", type: "helper" }, + { id: "n3", type: "http proxy", url: proxyUrl, noproxy: ["foo,bar,baz"] } + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + helper.load(testNode, flow, function () { + const n1 = helper.getNode("n1"); + const n2 = helper.getNode("n2"); + try { + proxySpy.calledOnce.should.be.false() // proxy setup should not be called when there is no URL to check needs proxying + } catch (err) { + done(err); + return + } + n2.on("input", function (msg) { + try { + // ensure getProxyForUrl was called and returned the correct proxy URL + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { env: { no_proxy: "foo,bar,baz", http_proxy: proxyUrl, https_proxy: proxyUrl } }).should.be.true() + proxySpy.returnValues[0].should.be.equal(proxyUrl) + done(); + } catch (err) { + done(err); + } + }); + n1.receive({ url: url }); + }); + }); + it('should not use UI proxy if noproxy excludes it', function (done) { + const url = getSslTestURL('/postInspect') + const proxyUrl = "http://localhost:" + testProxyPort + const flow = [ + { id: "n1", type: "http request", wires: [["n2"]], method: "POST", ret: "obj", url: "", proxy: "n3" }, + { id: "n2", type: "helper" }, + { id: "n3", type: "http proxy", url: proxyUrl, noproxy: ["foo,localhost,baz"] } + ]; + const proxySpy = sinon.spy(httpProxyHelper, 'getProxyForUrl') + const testNode = [httpRequestNode, httpProxyNode]; + deleteProxySetting(); + helper.load(testNode, flow, function () { + const n1 = helper.getNode("n1"); + const n2 = helper.getNode("n2"); + try { + proxySpy.calledOnce.should.be.false() // proxy setup should not be called when there is no URL to check needs proxying + } catch (err) { + done(err); + return + } + n2.on("input", function (msg) { + try { + // ensure getProxyForUrl was called and returned no proxy + proxySpy.calledOnce.should.be.true() + proxySpy.calledWith(url, { env: { no_proxy: "foo,localhost,baz", http_proxy: proxyUrl, https_proxy: proxyUrl } }).should.be.true() + proxySpy.returnValues[0].should.be.equal('') + done(); + } catch (err) { + done(err); + } + }); + n1.receive({ url: url }); + }); + }); + }); describe('authentication', function() { From 40a2d90e080923220a08c24ce7c30fdc577c9b55 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Mon, 24 Jun 2024 20:06:22 +0100 Subject: [PATCH 4/4] remove .only --- test/nodes/core/network/21-httprequest_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/nodes/core/network/21-httprequest_spec.js b/test/nodes/core/network/21-httprequest_spec.js index 728cda458..0df73b970 100644 --- a/test/nodes/core/network/21-httprequest_spec.js +++ b/test/nodes/core/network/21-httprequest_spec.js @@ -37,7 +37,7 @@ var crypto = require("crypto"); const { version } = require("os"); const net = require('net') -describe.only('HTTP Request Node', function() { +describe('HTTP Request Node', function() { var testApp; var testServer; var testPort = 10234;