From 2d6e13bf07c661440826e1642a18175a91674c01 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 9 Apr 2019 16:08:39 -0400 Subject: [PATCH] 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 | 8 ++++-- packages/server/test/spec_helper.coffee | 3 +++ packages/server/test/unit/args_spec.coffee | 31 ++++++++++++++++++---- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/server/lib/util/proxy.ts b/packages/server/lib/util/proxy.ts index afa6f0d88d87..e37258a497f0 100644 --- a/packages/server/lib/util/proxy.ts +++ b/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 diff --git a/packages/server/test/spec_helper.coffee b/packages/server/test/spec_helper.coffee index 17f7a65f9a35..0cf110a29676 100644 --- a/packages/server/test/spec_helper.coffee +++ b/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/) diff --git a/packages/server/test/unit/args_spec.coffee b/packages/server/test/unit/args_spec.coffee index 22da3a8a7dfc..fc42d449afe1 100644 --- a/packages/server/test/unit/args_spec.coffee +++ b/packages/server/test/unit/args_spec.coffee @@ -304,7 +304,7 @@ 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 @@ -312,7 +312,6 @@ describe "lib/util/args", -> 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)