Fix bug in proxy where HTTPS_PROXY became 'undefined' (#3913)

* server: fix bug in proxy where HTTPS_PROXY became 'undefined'

* server: loose undefined check

* allow empty string, 0, false to disable proxy

* fix failing tests

packages/server/lib/util/proxy.ts

@@ -12,7 +12,11 @@ const copyLowercaseEnvToUppercase = (name: string) => {
 }
 
 const normalizeEnvironmentProxy = () => {
-  if (!process.env.HTTPS_PROXY) {
+  if (process.env.HTTP_PROXY === 'false' || process.env.HTTP_PROXY === '0' || !process.env.HTTP_PROXY) {
+    delete process.env.HTTP_PROXY
+  }
+
+  if (!process.env.HTTPS_PROXY && process.env.HTTP_PROXY) {
     // request library will use HTTP_PROXY as a fallback for HTTPS urls, but
     // proxy-from-env will not, so let's just force it to fall back like this
     process.env.HTTPS_PROXY = process.env.HTTP_PROXY
@@ -27,7 +31,7 @@ const normalizeEnvironmentProxy = () => {
 export const loadSystemProxySettings = () => {
   ['NO_PROXY', 'HTTP_PROXY', 'HTTPS_PROXY'].forEach(copyLowercaseEnvToUppercase)
 
-  if (process.env.HTTP_PROXY) {
+  if (typeof process.env.HTTP_PROXY !== 'undefined') {
     normalizeEnvironmentProxy()
 
     return

packages/server/test/spec_helper.coffee

@@ -38,6 +38,7 @@ hasOnly = false
 
     backup.apply(@, arguments)
 
+originalEnv = process.env
 env = _.clone(process.env)
 
 sinon.usingPromise(Promise)
@@ -78,6 +79,8 @@ before ->
   appData.ensure()
 
 beforeEach ->
+  @originalEnv = originalEnv
+
   nock.disableNetConnect()
   nock.enableNetConnect(/localhost/)
 

packages/server/test/unit/args_spec.coffee

@@ -304,15 +304,14 @@ describe "lib/util/args", ->
 
   context "with proxy", ->
     beforeEach ->
-      @beforeEnv = Object.assign({}, process.env)
+      process.env = @originalEnv
       delete process.env.HTTP_PROXY
       delete process.env.HTTPS_PROXY
       delete process.env.NO_PROXY
       delete process.env.http_proxy
       delete process.env.https_proxy
       delete process.env.no_proxy
 
-
     it "sets options from environment", ->
       process.env.HTTP_PROXY = "http://foo-bar.baz:123"
       process.env.NO_PROXY = "a,b,c"
@@ -337,6 +336,31 @@ describe "lib/util/args", ->
       expect(options.proxyBypassList).to.eq "d,e,f"
       expect(options.proxyBypassList).to.eq process.env.NO_PROXY
 
+    ['', 'false', '0'].forEach (override) ->
+      it "doesn't load from Windows registry if HTTP_PROXY overridden with string '#{override}'", ->
+        sinon.stub(getWindowsProxyUtil, "getWindowsProxy").returns()
+        sinon.stub(os, "platform").returns("win32")
+        process.env.HTTP_PROXY = override
+        options = @setup()
+        expect(getWindowsProxyUtil.getWindowsProxy).to.not.beCalled
+        expect(options.proxySource).to.be.undefined
+        expect(options.proxyServer).to.be.undefined
+        expect(options.proxyBypassList).to.be.undefined
+        expect(process.env.HTTP_PROXY).to.be.undefined
+        expect(process.env.HTTPS_PROXY).to.be.undefined
+        expect(process.env.NO_PROXY).to.eq "localhost"
+
+    it "doesn't mess with env vars if Windows registry doesn't have proxy", ->
+      sinon.stub(getWindowsProxyUtil, "getWindowsProxy").returns()
+      sinon.stub(os, "platform").returns("win32")
+      options = @setup()
+      expect(options.proxySource).to.be.undefined
+      expect(options.proxyServer).to.be.undefined
+      expect(options.proxyBypassList).to.be.undefined
+      expect(process.env.HTTP_PROXY).to.be.undefined
+      expect(process.env.HTTPS_PROXY).to.be.undefined
+      expect(process.env.NO_PROXY).to.eq "localhost"
+
     it "sets a default NO_PROXY", ->
       process.env.HTTP_PROXY = "http://foo-bar.baz:123"
       options = @setup()
@@ -361,6 +385,3 @@ describe "lib/util/args", ->
       expect(options.proxySource).to.be.undefined
       expect(options.proxyServer).to.eq process.env.HTTP_PROXY
       expect(options.proxyBypassList).to.eq process.env.NO_PROXY
-
-    afterEach ->
-      Object.assign(process.env, @beforeEnv)