agent: Fix NO_PROXY not respected for HTTPS urls

packages/network/lib/agent.ts

@@ -240,7 +240,7 @@ class HttpsAgent extends https.Agent {
     if (process.env.HTTPS_PROXY) {
       const proxy = getProxyForUrl(options.href)
 
-      if (typeof proxy === "string") {
+      if (proxy) {
         options.proxy = <string>proxy
 
         return this.createUpstreamProxyConnection(<RequestOptionsWithProxy>options, cb)

packages/network/test/unit/agent_spec.ts

@@ -1,6 +1,5 @@
 import Bluebird from 'bluebird'
 import chai from 'chai'
-import { EventEmitter } from 'events'
 import http from 'http'
 import https from 'https'
 import net from 'net'
@@ -26,12 +25,11 @@ const HTTPS_PORT = 31443
 
 describe('lib/agent', function() {
   beforeEach(function() {
-    this.oldEnv = Object.assign({}, process.env)
     process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'
   })
 
   afterEach(function() {
-    process.env = this.oldEnv
+    process.env.NO_PROXY = process.env.HTTP_PROXY = process.env.HTTPS_PROXY = ''
     sinon.restore()
   })
 
@@ -207,8 +205,40 @@ describe('lib/agent', function() {
     })
 
     context('HttpsAgent', function() {
+      beforeEach(function() {
+        this.agent = new CombinedAgent()
+
+        this.request = request.defaults({
+          agent: <any>this.agent,
+          proxy: null
+        })
+      })
+
+      it("#createUpstreamProxyConnection does not go to proxy if domain in NO_PROXY", function () {
+        const spy = sinon.spy(this.agent.httpsAgent, 'createUpstreamProxyConnection')
+
+        process.env.HTTP_PROXY = process.env.HTTPS_PROXY = 'http://0.0.0.0:0'
+        process.env.NO_PROXY = 'mtgox.info,example.com,homestarrunner.com,'
+
+        return this.request({
+          url: 'https://example.com/',
+        })
+        .then(() => {
+          expect(spy).to.not.be.called
+
+          return this.request({
+            url: 'https://example.org/'
+          })
+          .then(() => {
+            throw new Error('should not be able to connect')
+          })
+          .catch({ message: 'Error: A connection to the upstream proxy could not be established: connect ECONNREFUSED 0.0.0.0'}, () => {
+            expect(spy).to.be.calledOnce
+          })
+        })
+      })
+
       it("#createUpstreamProxyConnection calls to super for caching, TLS-ifying", function() {
-        const combinedAgent = new CombinedAgent()
         const spy = sinon.spy(https.Agent.prototype, 'createConnection')
 
         const proxy = new DebuggingProxy()
@@ -219,26 +249,22 @@ describe('lib/agent', function() {
 
         return proxy.start(proxyPort)
         .then(() => {
-          return request({
+          return this.request({
             url: `https://localhost:${HTTPS_PORT}/get`,
-            agent: <any>combinedAgent,
-            proxy: null
           })
         })
         .then(() => {
           const options = spy.getCall(0).args[0]
-          const session = combinedAgent.httpsAgent._sessionCache.map[options._agentKey]
+          const session = this.agent.httpsAgent._sessionCache.map[options._agentKey]
           expect(spy).to.be.calledOnce
-          expect(combinedAgent.httpsAgent._sessionCache.list).to.have.length(1)
+          expect(this.agent.httpsAgent._sessionCache.list).to.have.length(1)
           expect(session).to.not.be.undefined
 
           return proxy.stop()
         })
       })
 
       it("#createUpstreamProxyConnection throws when connection is accepted then closed", function() {
-        const combinedAgent = new CombinedAgent()
-
         const proxy = Bluebird.promisifyAll(
           net.createServer((socket) => {
             socket.end()
@@ -252,10 +278,8 @@ describe('lib/agent', function() {
 
         return proxy.listenAsync(proxyPort)
         .then(() => {
-          return request({
+          return this.request({
             url: `https://localhost:${HTTPS_PORT}/get`,
-            agent: <any>combinedAgent,
-            proxy: null
           })
         })
         .then(() => {
@@ -268,6 +292,41 @@ describe('lib/agent', function() {
         })
       })
     })
+
+    context('HttpAgent', function() {
+      beforeEach(function() {
+        this.agent = new CombinedAgent()
+
+        this.request = request.defaults({
+          agent: <any>this.agent,
+          proxy: null
+        })
+      })
+
+      it("#createSocket does not go to proxy if domain in NO_PROXY", function () {
+        const spy = sinon.spy(this.agent.httpAgent, '_createProxiedSocket')
+
+        process.env.HTTP_PROXY = process.env.HTTPS_PROXY = 'http://0.0.0.0:0'
+        process.env.NO_PROXY = 'mtgox.info,example.com,homestarrunner.com,'
+
+        return this.request({
+          url: 'http://example.com/',
+        })
+        .then(() => {
+          expect(spy).to.not.be.called
+
+          return this.request({
+            url: 'http://example.org/'
+          })
+          .then(() => {
+            throw new Error('should not be able to connect')
+          })
+          .catch({ message: 'Error: connect ECONNREFUSED 0.0.0.0'}, () => {
+            expect(spy).to.be.calledOnce
+          })
+        })
+      })
+    })
   })
 
   context(".buildConnectReqHead", function() {