fix: infinite proxy loop (#4676)

I think the problem is that when a proxy is not in use proxy-agent
returns the global agent...which is itself since we set it globally,
causing the loop.

VS Code already covers proxies meaning we only need to do it in our own
requests so to fix this pass in the agent in the version fetch request
instead of overidding globally.

Also avoid proxy-from-env and pass in the proxy URI instead as both
http_proxy and https_proxy can be used for either http or https requests
but it does not allow that.

package.json

@@ -92,7 +92,6 @@
     "limiter": "^1.1.5",
     "pem": "^1.14.2",
     "proxy-agent": "^5.0.0",
-    "proxy-from-env": "^1.1.0",
     "qs": "6.10.2",
     "rotating-file-stream": "^3.0.0",
     "safe-buffer": "^5.1.1",

src/node/constants.ts

@@ -25,3 +25,5 @@ export const rootPath = path.resolve(__dirname, "../..")
 export const vsRootPath = path.join(rootPath, "vendor/modules/code-oss-dev")
 export const tmpdir = path.join(os.tmpdir(), "code-server")
 export const isDevMode = commit === "development"
+export const httpProxyUri =
+  process.env.HTTPS_PROXY || process.env.https_proxy || process.env.HTTP_PROXY || process.env.http_proxy

src/node/entry.ts

@@ -2,12 +2,9 @@ import { logger } from "@coder/logger"
 import { optionDescriptions, parse, readConfigFile, setDefaults, shouldOpenInExistingInstance } from "./cli"
 import { commit, version } from "./constants"
 import { openInExistingInstance, runCodeServer, runVsCodeCli, shouldSpawnCliProcess } from "./main"
-import { monkeyPatchProxyProtocols } from "./proxy_agent"
 import { isChild, wrapper } from "./wrapper"
 
 async function entry(): Promise<void> {
-  monkeyPatchProxyProtocols()
-
   // There's no need to check flags like --help or to spawn in an existing
   // instance for the child process because these would have already happened in
   // the parent and the child wouldn't have been spawned. We also get the

src/node/proxy_agent.ts

@@ -1,71 +0,0 @@
-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Coder Technologies. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-import ProxyAgent from "proxy-agent"
-import { getProxyForUrl } from "proxy-from-env"
-
-/**
- * This file has nothing to do with the code-server proxy.
- * It is to support $HTTP_PROXY, $HTTPS_PROXY and $NO_PROXY.
- *
- * - https://github.com/cdr/code-server/issues/124
- * - https://www.npmjs.com/package/proxy-agent
- * - https://www.npmjs.com/package/proxy-from-env
- *
- */
-
-/**
- * monkeyPatch patches the node http,https modules to route all requests through the
- * agent we get from the proxy-agent package.
- *
- * This approach only works if there is no code specifying an explicit agent when making
- * a request.
- *
- * None of our code ever passes in a explicit agent to the http,https modules.
- * VS Code's does sometimes but only when a user sets the http.proxy configuration.
- * See https://code.visualstudio.com/docs/setup/network#_legacy-proxy-server-support
- *
- * Even if they do, it's probably the same proxy so we should be fine! And those knobs
- * are deprecated anyway.
- */
-export function monkeyPatchProxyProtocols(): void {
-  if (!shouldEnableProxy()) {
-    return
-  }
-
-  const http = require("http")
-  const https = require("https")
-
-  // If we do not pass in a proxy URL, proxy-agent will get the URL from the environment.
-  // See https://www.npmjs.com/package/proxy-from-env.
-  // Also see shouldEnableProxy.
-  const pa = new ProxyAgent()
-  http.globalAgent = pa
-  https.globalAgent = pa
-}
-
-const sampleUrls = [new URL("http://example.com"), new URL("https://example.com")]
-
-// If they have $NO_PROXY set to example.com then this check won't work!
-// But that's drastically unlikely.
-export function shouldEnableProxy(): boolean {
-  const testedProxyEndpoints = sampleUrls.map((url) => {
-    return {
-      url,
-      proxyUrl: getProxyForUrl(url.toString()),
-    }
-  })
-
-  let shouldEnable = false
-
-  for (const { url, proxyUrl } of testedProxyEndpoints) {
-    if (proxyUrl) {
-      console.debug(`${url.protocol} -- Using "${proxyUrl}"`)
-      shouldEnable = true
-    }
-  }
-
-  return shouldEnable
-}

src/node/update.ts

@@ -1,9 +1,10 @@
 import { field, logger } from "@coder/logger"
 import * as http from "http"
 import * as https from "https"
+import ProxyAgent from "proxy-agent"
 import * as semver from "semver"
 import * as url from "url"
-import { version } from "./constants"
+import { httpProxyUri, version } from "./constants"
 import { SettingsProvider, UpdateSettings } from "./settings"
 
 export interface Update {
@@ -102,8 +103,10 @@ export class UpdateProvider {
     return new Promise((resolve, reject) => {
       const request = (uri: string): void => {
         logger.debug("Making request", field("uri", uri))
-        const httpx = uri.startsWith("https") ? https : http
+        const isHttps = uri.startsWith("https")
-        const client = httpx.get(uri, { headers: { "User-Agent": "code-server" } }, (response) => {
+        const agent = httpProxyUri ? new ProxyAgent(httpProxyUri) : undefined
+        const httpx = isHttps ? https : http
+        const client = httpx.get(uri, { headers: { "User-Agent": "code-server" }, agent }, (response) => {
           if (!response.statusCode || response.statusCode < 200 || response.statusCode >= 400) {
             response.destroy()
             return reject(new Error(`${uri}: ${response.statusCode || "500"}`))

test/unit/node/proxy_agent.test.ts

@@ -1,43 +0,0 @@
-import { shouldEnableProxy } from "../../../src/node/proxy_agent"
-import { mockLogger, useEnv } from "../../utils/helpers"
-
-describe("shouldEnableProxy", () => {
-  const [setHTTPProxy, resetHTTPProxy] = useEnv("HTTP_PROXY")
-  const [setHTTPSProxy, resetHTTPSProxy] = useEnv("HTTPS_PROXY")
-  const [setNoProxy, resetNoProxy] = useEnv("NO_PROXY")
-
-  beforeAll(() => {
-    mockLogger()
-  })
-
-  beforeEach(() => {
-    jest.resetModules() // Most important - it clears the cache
-    resetHTTPProxy()
-    resetNoProxy()
-    resetHTTPSProxy()
-  })
-
-  it("returns true when HTTP_PROXY is set", () => {
-    setHTTPProxy("http://proxy.example.com")
-    expect(shouldEnableProxy()).toBe(true)
-  })
-  it("returns true when HTTPS_PROXY is set", () => {
-    setHTTPSProxy("https://proxy.example.com")
-    expect(shouldEnableProxy()).toBe(true)
-  })
-  it("returns false when NO_PROXY is set", () => {
-    setNoProxy("*")
-    expect(shouldEnableProxy()).toBe(false)
-  })
-  it("should return false when neither HTTP_PROXY nor HTTPS_PROXY is set", () => {
-    expect(shouldEnableProxy()).toBe(false)
-  })
-  it("should return false when NO_PROXY is set to https://example.com", () => {
-    setNoProxy("https://example.com")
-    expect(shouldEnableProxy()).toBe(false)
-  })
-  it("should return false when NO_PROXY is set to http://example.com", () => {
-    setNoProxy("http://example.com")
-    expect(shouldEnableProxy()).toBe(false)
-  })
-})

yarn.lock

@@ -890,6 +890,11 @@ bytes@3.1.0:
   resolved "https://registry.yarnpkg.com/bytes/-/bytes-3.1.0.tgz#f6cf7933a360e0588fa9fde85651cdc7f805d1f6"
   integrity sha512-zauLjrfCG+xvoyaqLoV8bLVXXNGC4JqlxFCutSDWA6fJrTo2ZuvLYTqZ7aHBLZSMOopbzwv8f+wZcVzfVTI2Dg==
 
+bytes@3.1.1:
+  version "3.1.1"
+  resolved "https://registry.yarnpkg.com/bytes/-/bytes-3.1.1.tgz#3f018291cb4cbad9accb6e6970bca9c8889e879a"
+  integrity sha512-dWe4nWO/ruEOY7HkUJ5gFt1DCFV9zPRoJr8pV0/ASQermOZjtq8jMjOprC0Kd10GLN+l7xaUPvxzJFWtxGu8Fg==
+
 call-bind@^1.0.0, call-bind@^1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/call-bind/-/call-bind-1.0.2.tgz#b1d4e89e688119c3c9a903ad30abb2f6a919be3c"
@@ -2190,7 +2195,18 @@ http-errors@1.7.2:
     statuses ">= 1.5.0 < 2"
     toidentifier "1.0.0"
 
-http-errors@1.7.3, http-errors@~1.7.2:
+http-errors@1.8.1:
+  version "1.8.1"
+  resolved "https://registry.yarnpkg.com/http-errors/-/http-errors-1.8.1.tgz#7c3f28577cbc8a207388455dbd62295ed07bd68c"
+  integrity sha512-Kpk9Sm7NmI+RHhnj6OIWDI1d6fIoFAtFt9RLaTMRlg/8w49juAStsrBgp0Dp4OdxdVbRIeKhtCUvoi/RuAhO4g==
+  dependencies:
+    depd "~1.1.2"
+    inherits "2.0.4"
+    setprototypeof "1.2.0"
+    statuses ">= 1.5.0 < 2"
+    toidentifier "1.0.1"
+
+http-errors@~1.7.2:
   version "1.7.3"
   resolved "https://registry.yarnpkg.com/http-errors/-/http-errors-1.7.3.tgz#6c619e4f9c60308c38519498c14fbb10aacebb06"
   integrity sha512-ZTTX0MWrsQ2ZAhA1cejAwDLycFsd7I7nVtnkT3Ol0aqodaKW+0CTZDQ1uBv5whptCnc8e8HeRRJxRs0kmm/Qfw==
@@ -3459,7 +3475,7 @@ proxy-agent@^5.0.0:
     proxy-from-env "^1.0.0"
     socks-proxy-agent "^5.0.0"
 
-proxy-from-env@^1.0.0, proxy-from-env@^1.1.0:
+proxy-from-env@^1.0.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2"
   integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==
@@ -3512,12 +3528,12 @@ raw-body@2.4.0:
     unpipe "1.0.0"
 
 raw-body@^2.2.0:
-  version "2.4.1"
+  version "2.4.2"
-  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.4.1.tgz#30ac82f98bb5ae8c152e67149dac8d55153b168c"
+  resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.4.2.tgz#baf3e9c21eebced59dd6533ac872b71f7b61cb32"
-  integrity sha512-9WmIKF6mkvA0SLmA2Knm9+qj89e+j1zqgyn8aXGd7+nAduPoqgI9lO57SAZNn/Byzo5P7JhXTyg9PzaJbH73bA==
+  integrity sha512-RPMAFUJP19WIet/99ngh6Iv8fzAbqum4Li7AD6DtGaW2RpMB/11xDoalPiJMTbu6I3hkbMVkATvZrqb9EEqeeQ==
   dependencies:
-    bytes "3.1.0"
+    bytes "3.1.1"
-    http-errors "1.7.3"
+    http-errors "1.8.1"
     iconv-lite "0.4.24"
     unpipe "1.0.0"
 
@@ -3817,6 +3833,11 @@ setprototypeof@1.1.1:
   resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.1.1.tgz#7e95acb24aa92f5885e0abef5ba131330d4ae683"
   integrity sha512-JvdAWfbXeIGaZ9cILp38HntZSFSo3mWg6xGcJJsd+d4aRMOqauag1C63dJfDw7OaMYwEbHMOxEZ1lqVRYP2OAw==
 
+setprototypeof@1.2.0:
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/setprototypeof/-/setprototypeof-1.2.0.tgz#66c9a24a73f9fc28cbe66b09fed3d33dcaf1b424"
+  integrity sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==
+
 shebang-command@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-2.0.0.tgz#ccd0af4f8835fbdc265b82461aaf0c36663f34ea"
@@ -4238,6 +4259,11 @@ toidentifier@1.0.0:
   resolved "https://registry.yarnpkg.com/toidentifier/-/toidentifier-1.0.0.tgz#7e1be3470f1e77948bc43d94a3c8f4d7752ba553"
   integrity sha512-yaOH/Pk/VEhBWWTlhI+qXxDFXlejDGcQipMlyxda9nthulaxLZUNcUqFxokp0vcYnvteJln5FNQDRrxj3YcbVw==
 
+toidentifier@1.0.1:
+  version "1.0.1"
+  resolved "https://registry.yarnpkg.com/toidentifier/-/toidentifier-1.0.1.tgz#3be34321a88a820ed1bd80dfaa33e479fbb8dd35"
+  integrity sha512-o5sSPKEkg/DIQNmH43V0/uerLrpzVedkUh8tGNvaeXpfpuwjKenlSox/2O/BTlZUtEe+JG7s5YhEz608PlAHRA==
+
 traverse@^0.6.6:
   version "0.6.6"
   resolved "https://registry.yarnpkg.com/traverse/-/traverse-0.6.6.tgz#cbdf560fd7b9af632502fed40f918c157ea97137"