Merge pull request #4794 from node-red/4792-ui-proxy-fix

UI proxy should setup agents for both http_proxy and https_proxy

packages/node_modules/@node-red/nodes/core/network/21-httprequest.js

@@ -108,12 +108,13 @@ 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)
         }
-        let prox = getProxy(nodeUrl || '')
+        let prox = nodeUrl ? getProxy(nodeUrl) : null
 
         let timingLog = false;
         if (RED.settings.hasOwnProperty("httpRequestTimingLog")) {
@@ -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 {

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');
@@ -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 env var http_proxy', function(done) {
-        /* */
+            const url = getTestURL('/postInspect')
-        it('should use http_proxy', function(done) {
+            const proxyUrl = "http://localhost:" + testProxyPort
-            var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"POST",ret:"obj",url:getTestURL('/postInspect')},
+            
-                {id:"n2", type:"helper"}];
+            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;
+            process.env.http_proxy = proxyUrl
-            helper.load(httpRequestNode, flow, function() {
+            helper.load(testNode, flow, function (msg) {
-                var n1 = helper.getNode("n1");
+                try {
-                var n2 = helper.getNode("n2");
+                    // static URL set in the nodes configuration and the proxy will be setup upon initialisation
-                n2.on("input", function(msg) {
+                    proxySpy.calledOnce.should.be.true()
-                    try {
+                    proxySpy.calledWith(url, { }).should.be.true()
-                        msg.should.have.property('statusCode',200);
+                    proxySpy.returnValues[0].should.be.equal(proxyUrl)
-                        msg.payload.should.have.property('headers');
+                    done()
-                        //msg.payload.headers.should.have.property('x-testproxy-header','foobar');
+                } catch (err) {
-                        done();
+                    done(err);
-                    } catch(err) {
+                }
-                        done(err);
+            });
-                    }
+        });
-                });
+
-                n1.receive({payload:"foo"});
+        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() {