Skip to content

proxy-agent: Use proxy-from-env and add docs #2487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions doc/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- [Isn't an install script piped into sh insecure?](#isnt-an-install-script-piped-into-sh-insecure)
- [How do I make my keyboard shortcuts work?](#how-do-i-make-my-keyboard-shortcuts-work)
- [Differences compared to Theia?](#differences-compared-to-theia)
- [`$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`](#http_proxy-https_proxy-no_proxy)
- [Enterprise](#enterprise)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
Expand Down Expand Up @@ -338,6 +339,30 @@ You can't just use your VS Code config in Theia like you can with code-server.
To summarize, code-server is a patched fork of VS Code to run in the browser whereas
Theia takes some parts of VS Code but is an entirely different editor.

## `$HTTP_PROXY`, `$HTTPS_PROXY`, `$NO_PROXY`

code-server supports the standard environment variables to allow directing
server side requests through a proxy.

```sh
export HTTP_PROXY=https://134.8.5.4
export HTTPS_PROXY=https://134.8.5.4
# Now all of code-server's server side requests will go through
# https://134.8.5.4 first.
code-server
```

- See [proxy-from-env](https://www.npmjs.com/package/proxy-from-env#environment-variables)
for a detailed reference on the various environment variables and their syntax.
- code-server only uses the `http` and `https` protocols.
- See [proxy-agent](https://www.npmjs.com/package/proxy-agent) for the various supported
proxy protocols.

**note**: Only server side requests will be proxied! This includes fetching extensions,
requests made from extensions etc. To proxy requests from your browser you need to
configure your browser separately. Browser requests would cover exploring the extension
marketplace.

## Enterprise

Visit [our enterprise page](https://coder.com) for more information about our
Expand Down
2 changes: 2 additions & 0 deletions lib/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"native-watchdog": "1.3.0",
"node-pty": "0.10.0-beta17",
"proxy-agent": "^4.0.0",
"proxy-from-env": "^1.1.0",
"rimraf": "^2.2.8",
"spdlog": "^0.11.1",
"sudo-prompt": "9.1.1",
Expand Down Expand Up @@ -95,6 +96,7 @@
"@types/minimist": "^1.2.0",
"@types/mocha": "2.2.39",
"@types/node": "^12.11.7",
"@types/proxy-from-env": "^1.0.1",
"@types/sinon": "^1.16.36",
"@types/trusted-types": "^1.0.6",
"@types/vscode-windows-registry": "^1.0.0",
Expand Down
7 changes: 7 additions & 0 deletions lib/vscode/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,13 @@
resolved "https://registry.yarnpkg.com/@types/node/-/node-13.13.28.tgz#b6d0628b0371d6c629d729c98322de314b640219"
integrity sha512-EM/qFeRH8ZCD+TlsaIPULyyFm9vOhFIvgskY2JmHbEsWsOPgN+rtjSXrcHGgJpob4Nu17VfO95FKewr0XY7iOQ==

"@types/proxy-from-env@^1.0.1":
version "1.0.1"
resolved "https://registry.yarnpkg.com/@types/proxy-from-env/-/proxy-from-env-1.0.1.tgz#b5f3e99230ca4518af196c18267055fc51f892b7"
integrity sha512-luG++TFHyS61eKcfkR1CVV6a1GMNXDjtqEQIIfaSHax75xp0HU3SlezjOi1yqubJwrG8e9DeW59n6wTblIDwFg==
dependencies:
"@types/node" "*"

"@types/semver@^5.4.0", "@types/semver@^5.5.0":
version "5.5.0"
resolved "https://registry.yarnpkg.com/@types/semver/-/semver-5.5.0.tgz#146c2a29ee7d3bae4bf2fcb274636e264c813c45"
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@types/node": "^12.12.7",
"@types/parcel-bundler": "^1.12.1",
"@types/pem": "^1.9.5",
"@types/proxy-from-env": "^1.0.1",
"@types/safe-compare": "^1.1.0",
"@types/semver": "^7.1.0",
"@types/split2": "^2.1.6",
Expand Down Expand Up @@ -82,6 +83,7 @@
"limiter": "^1.1.5",
"pem": "^1.14.2",
"proxy-agent": "^4.0.0",
"proxy-from-env": "^1.1.0",
"qs": "6.7.0",
"rotating-file-stream": "^2.1.1",
"safe-buffer": "^5.1.1",
Expand Down
91 changes: 55 additions & 36 deletions src/node/proxy_agent.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { logger } from "@coder/logger"
import * as http from "http"
import * as proxyagent from "proxy-agent"
import * as proxyAgent from "proxy-agent"
import * as proxyFromEnv from "proxy-from-env"

/**
* This file does not have anything to do with the code-server proxy.
* It's for $HTTP_PROXY support!
* 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
*
* This file exists in two locations:
* - src/node/proxy_agent.ts
Expand All @@ -15,48 +18,64 @@ import * as proxyagent from "proxy-agent"
*/

/**
* monkeyPatch patches the node HTTP/HTTPS library to route all requests through our
* custom agent from the proxyAgent package.
* 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 monkeyPatch(vscode: boolean): void {
// We do not support HTTPS_PROXY here to avoid confusion. proxy-agent will automatically
// use the correct protocol to connect to the proxy depending on the requested URL.
//
// We could implement support ourselves to allow people to configure the proxy used for
// HTTPS vs HTTP but there doesn't seem to be much value in that.
//
// At least of right now, it'd just be plain confusing to support HTTPS_PROXY when proxy-agent
// will still use HTTP to hit it for HTTP requests.
const proxyURL = process.env.HTTP_PROXY || process.env.http_proxy
if (!proxyURL) {
return
}
export function monkeyPatch(inVSCode: boolean): void {
if (shouldEnableProxy()) {
const http = require("http")
const https = require("https")

logger.debug(`using $HTTP_PROXY ${process.env.HTTP_PROXY}`)
// If we do not pass in a proxy URL, proxy-agent will get the URL from the environment.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always set it? It looks like it does the same check except the downside is that it runs for every request. The upside is we don't have to check ourselves with http://example.com.

Copy link
Member

@code-asher code-asher Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't put this on a good line, should've put it on the if. I was thinking about how it reads from the environment so I put it here on the comment. 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always set it? It looks like it does the same check except the downside is that it runs for every request.

I added the check for logging purposes. This is the log that uncovered the extension host logging bug for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logging/debugging

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

// See https://www.npmjs.com/package/proxy-from-env.
// Also see shouldEnableProxy.
const pa = newProxyAgent(inVSCode)
http.globalAgent = pa
https.globalAgent = pa
}
}

let pa: http.Agent
function newProxyAgent(inVSCode: boolean): http.Agent {
// The reasoning for this split is that VS Code's build process does not have
// esModuleInterop enabled but the code-server one does. As a result depending on where
// we execute, we either have a default attribute or we don't.
//
// I can't enable esModuleInterop in VS Code's build process as it breaks and spits out
// a huge number of errors.
if (vscode) {
pa = new (proxyagent as any)(process.env.HTTP_PROXY)
// a huge number of errors. And we can't use require as otherwise the modules won't be
// included in the final product.
if (inVSCode) {
return new (proxyAgent as any)()
} else {
pa = new (proxyagent as any).default(process.env.HTTP_PROXY)
return new (proxyAgent as any).default()
}
}

// If they have $NO_PROXY set to example.com then this check won't work!
// But that's drastically unlikely.
function shouldEnableProxy(): boolean {
let shouldEnable = false

const httpProxy = proxyFromEnv.getProxyForUrl(`http://example.com`)
if (httpProxy) {
shouldEnable = true
logger.debug(`using $HTTP_PROXY ${httpProxy}`)
}

const httpsProxy = proxyFromEnv.getProxyForUrl(`https://example.com`)
if (httpsProxy) {
shouldEnable = true
logger.debug(`using $HTTPS_PROXY ${httpsProxy}`)
}

/**
* None of our code ever passes in a explicit agent to the http modules but 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 are
* deprecated anyway.
*/
const http = require("http")
const https = require("https")
http.globalAgent = pa
https.globalAgent = pa
return shouldEnable
}
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,13 @@
dependencies:
"@types/node" "*"

"@types/proxy-from-env@^1.0.1":
version "1.0.1"
resolved "https://registry.yarnpkg.com/@types/proxy-from-env/-/proxy-from-env-1.0.1.tgz#b5f3e99230ca4518af196c18267055fc51f892b7"
integrity sha512-luG++TFHyS61eKcfkR1CVV6a1GMNXDjtqEQIIfaSHax75xp0HU3SlezjOi1yqubJwrG8e9DeW59n6wTblIDwFg==
dependencies:
"@types/node" "*"

"@types/q@^1.5.1":
version "1.5.4"
resolved "https://registry.yarnpkg.com/@types/q/-/q-1.5.4.tgz#15925414e0ad2cd765bfef58842f7e26a7accb24"
Expand Down Expand Up @@ -6333,7 +6340,7 @@ proxy-agent@^4.0.0:
proxy-from-env "^1.0.0"
socks-proxy-agent "^5.0.0"

proxy-from-env@^1.0.0:
proxy-from-env@^1.0.0, proxy-from-env@^1.1.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==
Expand Down