fix: no_proxy issue

Author: Avishagp

lib/needle.js

@@ -191,19 +191,34 @@ function host_and_ports_match(url1, url2) {
 }
 
 // returns false if a no_proxy host matches given url
-function should_proxy_to(url) {
-  var no_proxy = get_env_var(['NO_PROXY'], true);
-  if (!no_proxy) return true;
-
-  var host, hosts = no_proxy.split(',');
-  for (var i in hosts) {
-    host = hosts[i];
-    if (host_and_ports_match(host, url)) {
-      return false;
+function should_proxy_to(uri) {
+  const noProxy = get_env_var(["NO_PROXY"], true);
+  if (!noProxy) {
+    return true;
+  }
+
+  let urlMatchedNoProxyPattern = false
+  const requestUrl = new URL(uri);
+  const patternList = noProxy.split(/[\s,]+/);
+
+  // iterate over all NO_PROXY patterns and determine whether the given URL matches any of them
+  for (const pattern of patternList) {
+    if(pattern.trim().length == 0) {
+      continue
+    }
+
+    // replace leading dot by asterisk, escape dots and finally replace asterisk by .*
+    const preparedPattern = pattern.replace(/^\./, "*").replace(/[.]/g, '\\$&').replace(/\*/g, '.*')
+    const regex = new RegExp(preparedPattern)
+    const matches = (uri.match(regex)?.length > 0);
+    if (matches) {
+      // hostname + port of the request URL match a given NO_PROXY pattern
+      urlMatchedNoProxyPattern = true
+      break;
     }
   }
 
-  return true;
+  return !urlMatchedNoProxyPattern;
 }
 
 function pump_streams(streams, cb) {
@@ -914,3 +929,6 @@ module.exports.defaults = function(obj) {
 module.exports.request = function(method, uri, data, opts, callback) {
   return new Needle(method, uri, data, opts, callback).start();
 };
+
+// Exporting for testing
+module.exports.should_proxy_to = should_proxy_to;

test/proxy_spec.js

@@ -137,9 +137,9 @@ describe('proxy option', function() {
         }))
       })
 
-      it('proxies request if matching host in list but different port', function(done) {
+      it('does not proxy request if matching host in list and just has a different port', function(done) {
         process.env.NO_PROXY = 'localhost';
-        send_request({ proxy: nonexisting_host + ':123/done' }, proxied(nonexisting_host, '123', function() {
+        send_request({ proxy: nonexisting_host + ':123/done' }, not_proxied(function() {
           delete process.env.NO_PROXY;
           done();
         }))
@@ -152,6 +152,96 @@ describe('proxy option', function() {
           done();
         }))
       })
+
+      describe('should_proxy_to()', function() {
+
+        const noProxy = ".ic1.mycorp,localhost,127.0.0.1,*.mycorp.org";
+        const noProxyWithPorts = " ,.mycorp.org:1234,.ic1.mycorp,localhost,127.0.0.1";
+        const URI = "http://registry.random.opr.mycorp.org";
+        const URIWithPort = "http://registry.random.opr.mycorp.org:9874";
+        const URIWithPort1234 = "http://registry.random.opr.mycorp.org:1234";
+        const URIlocalhost = "http://localhost";
+        const URIip = "http://127.0.0.1";
+
+        it("shall return true if NO_PROXY is undefined", function(done) {
+          process.env.NO_PROXY = undefined;
+          needle.should_proxy_to(URI).should.true()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return true if NO_PROXY is empty", function(done) {
+          process.env.NO_PROXY = "";
+          needle.should_proxy_to(URI).should.true()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return true if the host matches and the ports don't (URI doesn't have port specified)", function(done) {
+          process.env.NO_PROXY = noProxyWithPorts;
+          needle.should_proxy_to(URI).should.true()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return true if the host matches and the ports don't (both have a port specified but just different values)", function(done) {
+          process.env.NO_PROXY = noProxyWithPorts;
+          needle.should_proxy_to(URIWithPort).should.true()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host matches and the ports don't (no_proxy pattern doesn't have a port)", function(done) {
+          process.env.NO_PROXY = noProxy;
+          needle.should_proxy_to(URIWithPort).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if host matches", function(done) {
+          process.env.NO_PROXY = noProxy;
+          needle.should_proxy_to(URI).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host and port matches", function(done) {
+          process.env.NO_PROXY = noProxyWithPorts;
+          needle.should_proxy_to(URIWithPort1234).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host matches (localhost)", function(done) {
+          process.env.NO_PROXY = noProxy;
+          needle.should_proxy_to(URIlocalhost).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host matches (ip)", function(done) {
+          process.env.NO_PROXY = noProxy;
+          needle.should_proxy_to(URIip).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host matches (ip)", function(done) {
+          process.env.NO_PROXY = noProxy.replace(/,g/, " ");
+          needle.should_proxy_to(URIip).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+        it("shall return false if the host matches (ip)", function(done) {
+          process.env.NO_PROXY = noProxy.replace(/,g/, " ");
+          needle.should_proxy_to(URIip).should.false()
+          delete process.env.NO_PROXY;
+          done();
+        });
+
+      })
+      
     })
 
     describe('and proxy url contains user:pass', function() {