From f169e3ac665c9180455f66a5acea0f2257837c66 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 11 Jan 2021 13:17:45 -0500
Subject: [PATCH 01/12] pathProxy.ts: Implement --proxy-path-passthrough

Closes #2222
---
 src/node/cli.ts              |  5 +++++
 src/node/routes/pathProxy.ts | 14 +++++++-------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/node/cli.ts b/src/node/cli.ts
index 1653e87a7..5a0e4473e 100644
--- a/src/node/cli.ts
+++ b/src/node/cli.ts
@@ -50,6 +50,7 @@ export interface Args extends VsArgs {
   "show-versions"?: boolean
   "uninstall-extension"?: string[]
   "proxy-domain"?: string[]
+  "proxy-path-passthrough"?: boolean
   locale?: string
   _: string[]
   "reuse-window"?: boolean
@@ -172,6 +173,10 @@ const options: Options<Required<Args>> = {
   "uninstall-extension": { type: "string[]", description: "Uninstall a VS Code extension by id." },
   "show-versions": { type: "boolean", description: "Show VS Code extension versions." },
   "proxy-domain": { type: "string[]", description: "Domain used for proxying ports." },
+  "proxy-path-passthrough": {
+    type: "boolean",
+    description: "Whether the path proxy should leave the /proxy/<port> in the request path when proxying.",
+  },
   "ignore-last-opened": {
     type: "boolean",
     short: "e",
diff --git a/src/node/routes/pathProxy.ts b/src/node/routes/pathProxy.ts
index 152402195..9554a006d 100644
--- a/src/node/routes/pathProxy.ts
+++ b/src/node/routes/pathProxy.ts
@@ -8,12 +8,12 @@ import { Router as WsRouter } from "../wsRouter"
 
 export const router = Router()
 
-const getProxyTarget = (req: Request, rewrite: boolean): string => {
-  if (rewrite) {
-    const query = qs.stringify(req.query)
-    return `http://0.0.0.0:${req.params.port}/${req.params[0] || ""}${query ? `?${query}` : ""}`
+const getProxyTarget = (req: Request, passthroughPath: boolean): string => {
+  if (passthroughPath) {
+    return `http://0.0.0.0:${req.params.port}/${req.originalUrl}`
   }
-  return `http://0.0.0.0:${req.params.port}/${req.originalUrl}`
+  const query = qs.stringify(req.query)
+  return `http://0.0.0.0:${req.params.port}/${req.params[0] || ""}${query ? `?${query}` : ""}`
 }
 
 router.all("/(:port)(/*)?", (req, res) => {
@@ -33,7 +33,7 @@ router.all("/(:port)(/*)?", (req, res) => {
 
   proxy.web(req, res, {
     ignorePath: true,
-    target: getProxyTarget(req, true),
+    target: getProxyTarget(req, req.args["proxy-path-passthrough"] || false),
   })
 })
 
@@ -42,6 +42,6 @@ export const wsRouter = WsRouter()
 wsRouter.ws("/(:port)(/*)?", ensureAuthenticated, (req) => {
   proxy.ws(req, req.ws, req.head, {
     ignorePath: true,
-    target: getProxyTarget(req, true),
+    target: getProxyTarget(req, req.args["proxy-path-passthrough"] || false),
   })
 })

From 497b01bffe176453ffc25cf83854180377d8077e Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 11 Jan 2021 13:18:07 -0500
Subject: [PATCH 02/12] FAQ.md: Document --proxy-path-passthrough

And the concerns surrounding it.

Closes #2485
---
 doc/FAQ.md | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/doc/FAQ.md b/doc/FAQ.md
index f3c58db84..19a4ff370 100644
--- a/doc/FAQ.md
+++ b/doc/FAQ.md
@@ -15,6 +15,7 @@
 - [How do I securely access web services?](#how-do-i-securely-access-web-services)
   - [Sub-paths](#sub-paths)
   - [Sub-domains](#sub-domains)
+- [Why does the code-server proxy strip `/proxy/<port>` from the request path?](#why-does-the-code-server-proxy-strip-proxyport-from-the-request-path)
 - [Multi-tenancy](#multi-tenancy)
 - [Docker in code-server container?](#docker-in-code-server-container)
 - [How can I disable telemetry?](#how-can-i-disable-telemetry)
@@ -208,6 +209,36 @@ code-server --proxy-domain <domain>
 Now you can browse to `<port>.<domain>`. Note that this uses the host header so
 ensure your reverse proxy forwards that information if you are using one.
 
+## Why does the code-server proxy strip `/proxy/<port>` from the request path?
+
+HTTP servers should strive to use relative URLs to avoid needed to be coupled to the
+absolute path at which they are served. This means you must use trailing slashes on all
+paths with subpaths. See https://blog.cdivilly.com/2019/02/28/uri-trailing-slashes
+
+This is really the "correct" way things work and why the striping of the base path is the
+default. If your application uses relative URLs and does not assume the absolute path at
+which it is being served, it will just work no matter what port you decide to serve it off
+or if you put it in behind code-server or any other proxy!
+
+However many people prefer the cleaner aesthetic of no trailing slashes. This couples you
+to the base path as you cannot use relative redirects correctly anymore. See the above
+link.
+
+For users who are ok with this tradeoff, pass `--proxy-path-passthrough` to code-server
+and the path will be passed as is.
+
+This is particularly a problem with the `start` script in create-react-app. See
+[#2222](https://github.com/cdr/code-server/issues/2222). You will need to inform
+create-react-app of the path at which you are serving via `homepage` field in your
+`package.json`. e.g. you'd add the following for the default CRA port:
+
+```json
+  "homepage": "/proxy/3000",
+```
+
+Then visit `https://my-code-server-address.io/proxy/3000` to see your app exposed through
+code-server!
+
 ## Multi-tenancy
 
 If you want to run multiple code-servers on shared infrastructure, we recommend using virtual

From ba4a24809c6273a3c449c1608193e3e1523bcedc Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 11 Jan 2021 14:33:46 -0500
Subject: [PATCH 03/12] routes/index.ts: Correctly register wsErrorHandler

express requires all 4 arguments to be declared for a error handler.
It's very unfortunate that our types do not handle this.
---
 src/node/routes/index.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts
index a7b7c1853..4688d6a2e 100644
--- a/src/node/routes/index.ts
+++ b/src/node/routes/index.ts
@@ -165,7 +165,7 @@ export const register = async (
 
   app.use(errorHandler)
 
-  const wsErrorHandler: express.ErrorRequestHandler = async (err, req) => {
+  const wsErrorHandler: express.ErrorRequestHandler = async (err, req, res, next) => {
     logger.error(`${err.message} ${err.stack}`)
     ;(req as WebsocketRequest).ws.end()
   }

From 9efcf7f3ce62c90fa7ed52d1a9c5e000c10091cb Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 11 Jan 2021 14:36:34 -0500
Subject: [PATCH 04/12] FAQ.md: Document wds problem with create-react-app and
 pathProxy.ts

---
 doc/FAQ.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/FAQ.md b/doc/FAQ.md
index 19a4ff370..5d14a3129 100644
--- a/doc/FAQ.md
+++ b/doc/FAQ.md
@@ -239,6 +239,12 @@ create-react-app of the path at which you are serving via `homepage` field in yo
 Then visit `https://my-code-server-address.io/proxy/3000` to see your app exposed through
 code-server!
 
+Unfortunately `webpack-dev-server`'s websocket connections will not go through as it
+always uses `/sockjs-node`. So hot reloading will not work until the `create-react-app`
+team fix this bug.
+
+Highly recommend using the subdomain approach instead to avoid this class of issue.
+
 ## Multi-tenancy
 
 If you want to run multiple code-servers on shared infrastructure, we recommend using virtual

From ea1949e4406ed4ecaeacdf4b45ee8fb8e54f2747 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 09:34:51 -0500
Subject: [PATCH 05/12] test: Add testutil.HttpServer

The goal is to remove supertest as it does not support typescript well
and there's really no good reason for the dependency. Also no websocket
testing support.
---
 package.json     |  4 +--
 test/testutil.ts | 65 ++++++++++++++++++++++++++++++++++++
 yarn.lock        | 87 +++++++++---------------------------------------
 3 files changed, 82 insertions(+), 74 deletions(-)
 create mode 100644 test/testutil.ts

diff --git a/package.json b/package.json
index aab6a111f..f3e368c36 100644
--- a/package.json
+++ b/package.json
@@ -38,13 +38,13 @@
     "@types/js-yaml": "^3.12.3",
     "@types/mocha": "^8.0.3",
     "@types/node": "^12.12.7",
+    "@types/node-fetch": "^2.5.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",
-    "@types/supertest": "^2.0.10",
     "@types/tar-fs": "^2.0.0",
     "@types/tar-stream": "^2.1.0",
     "@types/ws": "^7.2.6",
@@ -61,7 +61,6 @@
     "prettier": "^2.0.5",
     "stylelint": "^13.0.0",
     "stylelint-config-recommended": "^3.0.0",
-    "supertest": "^6.0.1",
     "ts-node": "^9.0.0",
     "typescript": "4.0.2"
   },
@@ -81,6 +80,7 @@
     "httpolyglot": "^0.1.2",
     "js-yaml": "^3.13.1",
     "limiter": "^1.1.5",
+    "node-fetch": "^2.6.1",
     "pem": "^1.14.2",
     "proxy-agent": "^4.0.0",
     "proxy-from-env": "^1.1.0",
diff --git a/test/testutil.ts b/test/testutil.ts
new file mode 100644
index 000000000..561662758
--- /dev/null
+++ b/test/testutil.ts
@@ -0,0 +1,65 @@
+import * as http from "http"
+import { logger } from "@coder/logger"
+import { ensureAddress } from "../src/node/app"
+import * as nodeFetch from "node-fetch"
+
+export class HttpServer {
+  private hs = http.createServer()
+
+  /**
+   * listen starts the server on a random localhost port.
+   * Use close to cleanup when done.
+   */
+  public listen(fn: http.RequestListener): Promise<void> {
+    this.hs.on("request", fn)
+
+    let resolved = false
+    return new Promise((res, rej) => {
+      this.hs.listen(0, "localhost", () => {
+        res()
+        resolved = true
+      })
+
+      this.hs.on("error", (err) => {
+        if (!resolved) {
+          rej(err)
+        } else {
+          // Promise resolved earlier so this is some other error.
+          logError("server error", err)
+        }
+      })
+    })
+  }
+
+  /**
+   * close cleans up the server.
+   */
+  public close(): Promise<void> {
+    return new Promise((res, rej) => {
+      this.hs.close((err) => {
+        if (err) {
+          rej(err)
+          return
+        }
+        res()
+      })
+    })
+  }
+
+  /**
+   * fetch fetches the request path.
+   * The request path must be rooted!
+   */
+  public fetch(requestPath: string, opts?: nodeFetch.RequestInit): Promise<nodeFetch.Response> {
+    return nodeFetch.default(`${ensureAddress(this.hs)}${requestPath}`, opts)
+  }
+}
+
+
+export function logError(prefix: string, err: any): void {
+  if (err instanceof Error) {
+    logger.error(`${prefix}: ${err.message} ${err.stack}`)
+  } else {
+    logger.error(`${prefix}: ${err}`)
+  }
+}
diff --git a/yarn.lock b/yarn.lock
index e0e3fa62c..afbef22ce 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1040,11 +1040,6 @@
   dependencies:
     "@types/express" "*"
 
-"@types/cookiejar@*":
-  version "2.1.2"
-  resolved "https://registry.yarnpkg.com/@types/cookiejar/-/cookiejar-2.1.2.tgz#66ad9331f63fe8a3d3d9d8c6e3906dd10f6446e8"
-  integrity sha512-t73xJJrvdTjXrn4jLS9VSGRbz0nUY3cl2DMGDU48lKl+HR9dbbjW2A9r3g40VA++mQpy6uuHg33gy7du2BKpog==
-
 "@types/express-serve-static-core@*":
   version "4.17.13"
   resolved "https://registry.yarnpkg.com/@types/express-serve-static-core/-/express-serve-static-core-4.17.13.tgz#d9af025e925fc8b089be37423b8d1eac781be084"
@@ -1108,6 +1103,14 @@
   resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-8.0.3.tgz#51b21b6acb6d1b923bbdc7725c38f9f455166402"
   integrity sha512-vyxR57nv8NfcU0GZu8EUXZLTbCMupIUwy95LJ6lllN+JRPG25CwMHoB1q5xKh8YKhQnHYRAn4yW2yuHbf/5xgg==
 
+"@types/node-fetch@^2.5.7":
+  version "2.5.7"
+  resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.5.7.tgz#20a2afffa882ab04d44ca786449a276f9f6bbf3c"
+  integrity sha512-o2WVNf5UhWRkxlf6eq+jMZDu7kjgpgJfl4xVNlvryc95O/6F2ld8ztKX+qu+Rjyet93WAWm5LjeX9H5FGkODvw==
+  dependencies:
+    "@types/node" "*"
+    form-data "^3.0.0"
+
 "@types/node@*", "@types/node@^12.12.7":
   version "12.12.67"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-12.12.67.tgz#4f86badb292e822e3b13730a1f9713ed2377f789"
@@ -1184,21 +1187,6 @@
   dependencies:
     "@types/node" "*"
 
-"@types/superagent@*":
-  version "4.1.10"
-  resolved "https://registry.yarnpkg.com/@types/superagent/-/superagent-4.1.10.tgz#5e2cc721edf58f64fe9b819f326ee74803adee86"
-  integrity sha512-xAgkb2CMWUMCyVc/3+7iQfOEBE75NvuZeezvmixbUw3nmENf2tCnQkW5yQLTYqvXUQ+R6EXxdqKKbal2zM5V/g==
-  dependencies:
-    "@types/cookiejar" "*"
-    "@types/node" "*"
-
-"@types/supertest@^2.0.10":
-  version "2.0.10"
-  resolved "https://registry.yarnpkg.com/@types/supertest/-/supertest-2.0.10.tgz#630d79b4d82c73e043e43ff777a9ca98d457cab7"
-  integrity sha512-Xt8TbEyZTnD5Xulw95GLMOkmjGICrOQyJ2jqgkSjAUR3mm7pAIzSR0NFBaMcwlzVvlpCjNwbATcWWwjNiZiFrQ==
-  dependencies:
-    "@types/superagent" "*"
-
 "@types/tar-fs@^2.0.0":
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/@types/tar-fs/-/tar-fs-2.0.0.tgz#db94cb4ea1cccecafe3d1a53812807efb4bbdbc1"
@@ -2255,7 +2243,7 @@ commander@^5.0.0:
   resolved "https://registry.yarnpkg.com/commander/-/commander-5.1.0.tgz#46abbd1652f8e059bddaef99bbdcb2ad9cf179ae"
   integrity sha512-P0CysNDQ7rtVw4QIQtm+MRxV66vKFSvlsQvGYXZWR3qFU0jlMKHZZZgw8e+8DSah4UDKMqnknRDQz+xuQXQ/Zg==
 
-component-emitter@^1.2.1, component-emitter@^1.3.0:
+component-emitter@^1.2.1:
   version "1.3.0"
   resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-1.3.0.tgz#16e4070fba8ae29b679f2215853ee181ab2eabc0"
   integrity sha512-Rd3se6QB+sO1TwqZjscQrurpEPIfO0/yYnSin6Q/rD3mOutHvUrCAhJub3r90uNb+SESBuE0QYoB90YdfatsRg==
@@ -2327,11 +2315,6 @@ cookie@0.4.0:
   resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.4.0.tgz#beb437e7022b3b6d49019d088665303ebe9c14ba"
   integrity sha512-+Hp8fLp57wnUSt0tY0tHEXh4voZRDnoIrZPqlo3DPiI4y9lwg/jqx+1Om94/W6ZaPDOUbnjOt/99w66zk+l1Xg==
 
-cookiejar@^2.1.2:
-  version "2.1.2"
-  resolved "https://registry.yarnpkg.com/cookiejar/-/cookiejar-2.1.2.tgz#dd8a235530752f988f9a0844f3fc589e3111125c"
-  integrity sha512-Mw+adcfzPxcPeI+0WlvRrr/3lGVO0bD75SxX6811cxSh1Wbxx7xZBGK1eVtDf6si8rg2lhnUjsVLMFMfbRIuwA==
-
 copy-descriptor@^0.1.0:
   version "0.1.1"
   resolved "https://registry.yarnpkg.com/copy-descriptor/-/copy-descriptor-0.1.1.tgz#676f6eb3c39997c2ee1ac3a924fd6124748f578d"
@@ -3428,11 +3411,6 @@ fast-levenshtein@^2.0.6, fast-levenshtein@~2.0.6:
   resolved "https://registry.yarnpkg.com/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz#3d8a5c66883a16a30ca8643e851f19baa7797917"
   integrity sha1-PYpcZog6FqMMqGQ+hR8Zuqd5eRc=
 
-fast-safe-stringify@^2.0.7:
-  version "2.0.7"
-  resolved "https://registry.yarnpkg.com/fast-safe-stringify/-/fast-safe-stringify-2.0.7.tgz#124aa885899261f68aedb42a7c080de9da608743"
-  integrity sha512-Utm6CdzT+6xsDk2m8S6uL8VHxNwI6Jub+e9NYTcAms28T84pTa25GJQV9j0CY0N1rM8hK4x6grpF2BQf+2qwVA==
-
 fastest-levenshtein@^1.0.12:
   version "1.0.12"
   resolved "https://registry.yarnpkg.com/fastest-levenshtein/-/fastest-levenshtein-1.0.12.tgz#9990f7d3a88cc5a9ffd1f1745745251700d497e2"
@@ -3603,11 +3581,6 @@ format@^0.2.0:
   resolved "https://registry.yarnpkg.com/format/-/format-0.2.2.tgz#d6170107e9efdc4ed30c9dc39016df942b5cb58b"
   integrity sha1-1hcBB+nv3E7TDJ3DkBbflCtctYs=
 
-formidable@^1.2.2:
-  version "1.2.2"
-  resolved "https://registry.yarnpkg.com/formidable/-/formidable-1.2.2.tgz#bf69aea2972982675f00865342b982986f6b8dd9"
-  integrity sha512-V8gLm+41I/8kguQ4/o1D3RIHRmhYFG4pnNyonvua+40rqcEmT4+V71yaZ3B457xbbgCsCfjSPi65u/W6vK1U5Q==
-
 forwarded@~0.1.2:
   version "0.1.2"
   resolved "https://registry.yarnpkg.com/forwarded/-/forwarded-0.1.2.tgz#98c23dab1175657b8c0573e8ceccd91b0ff18c84"
@@ -4978,7 +4951,7 @@ merge2@^1.2.3, merge2@^1.3.0:
   resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.4.1.tgz#4368892f885e907455a6fd7dc55c0c9d404990ae"
   integrity sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==
 
-methods@1.1.2, methods@^1.1.2, methods@~1.1.2:
+methods@~1.1.2:
   version "1.1.2"
   resolved "https://registry.yarnpkg.com/methods/-/methods-1.1.2.tgz#5529a4d67654134edcc5266656835b0f851afcee"
   integrity sha1-VSmk1nZUE07cxSZmVoNbD4Ua/O4=
@@ -5035,11 +5008,6 @@ mime@1.6.0:
   resolved "https://registry.yarnpkg.com/mime/-/mime-1.6.0.tgz#32cd9e5c64553bd58d19a568af452acff04981b1"
   integrity sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg==
 
-mime@^2.4.6:
-  version "2.4.6"
-  resolved "https://registry.yarnpkg.com/mime/-/mime-2.4.6.tgz#e5b407c90db442f2beb5b162373d07b69affa4d1"
-  integrity sha512-RZKhC3EmpBchfTGBVb8fb+RL2cWyw/32lshnsETttkBAyAUXSGHxbEJWWRXc751DrIxG1q04b8QwMbAwkRPpUA==
-
 mimic-fn@^1.0.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/mimic-fn/-/mimic-fn-1.2.0.tgz#820c86a39334640e99516928bd03fca88057d022"
@@ -5214,6 +5182,11 @@ node-addon-api@^1.7.1:
   resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-1.7.2.tgz#3df30b95720b53c24e59948b49532b662444f54d"
   integrity sha512-ibPK3iA+vaY1eEjESkQkM0BbCqFOaZMiXRTtdB0u7b4djtY6JnsjvPdUHVMg6xQt3B8fpTTWHI9A+ADjM9frzg==
 
+node-fetch@^2.6.1:
+  version "2.6.1"
+  resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052"
+  integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==
+
 node-forge@^0.7.1:
   version "0.7.6"
   resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.7.6.tgz#fdf3b418aee1f94f0ef642cd63486c77ca9724ac"
@@ -6405,11 +6378,6 @@ qs@6.7.0:
   resolved "https://registry.yarnpkg.com/qs/-/qs-6.7.0.tgz#41dc1a015e3d581f1621776be31afb2876a9b1bc"
   integrity sha512-VCdBRNFTX1fyE7Nb6FYoURo/SPe62QCaAyzJvUjwRaIsc+NePBEniHlvxFmmX56+HZphIGtV0XeCirBtpDrTyQ==
 
-qs@^6.9.4:
-  version "6.9.4"
-  resolved "https://registry.yarnpkg.com/qs/-/qs-6.9.4.tgz#9090b290d1f91728d3c22e54843ca44aea5ab687"
-  integrity sha512-A1kFqHekCTM7cz0udomYUoYNWjBebHm/5wzU/XqrBRBNWectVH0QIiN+NEcZ0Dte5hvzHwbr8+XQmguPhJ6WdQ==
-
 qs@~6.5.2:
   version "6.5.2"
   resolved "https://registry.yarnpkg.com/qs/-/qs-6.5.2.tgz#cb3ae806e8740444584ef154ce8ee98d403f3e36"
@@ -7522,31 +7490,6 @@ sugarss@^2.0.0:
   dependencies:
     postcss "^7.0.2"
 
-superagent@6.1.0:
-  version "6.1.0"
-  resolved "https://registry.yarnpkg.com/superagent/-/superagent-6.1.0.tgz#09f08807bc41108ef164cfb4be293cebd480f4a6"
-  integrity sha512-OUDHEssirmplo3F+1HWKUrUjvnQuA+nZI6i/JJBdXb5eq9IyEQwPyPpqND+SSsxf6TygpBEkUjISVRN4/VOpeg==
-  dependencies:
-    component-emitter "^1.3.0"
-    cookiejar "^2.1.2"
-    debug "^4.1.1"
-    fast-safe-stringify "^2.0.7"
-    form-data "^3.0.0"
-    formidable "^1.2.2"
-    methods "^1.1.2"
-    mime "^2.4.6"
-    qs "^6.9.4"
-    readable-stream "^3.6.0"
-    semver "^7.3.2"
-
-supertest@^6.0.1:
-  version "6.0.1"
-  resolved "https://registry.yarnpkg.com/supertest/-/supertest-6.0.1.tgz#f6b54370de85c45d6557192c8d7df604ca2c9e18"
-  integrity sha512-8yDNdm+bbAN/jeDdXsRipbq9qMpVF7wRsbwLgsANHqdjPsCoecmlTuqEcLQMGpmojFBhxayZ0ckXmLXYq7e+0g==
-  dependencies:
-    methods "1.1.2"
-    superagent "6.1.0"
-
 supports-color@7.1.0:
   version "7.1.0"
   resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-7.1.0.tgz#68e32591df73e25ad1c4b49108a2ec507962bfd1"

From 8acb2aec113edfce0e9734dd463b361a6d00be20 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 09:37:41 -0500
Subject: [PATCH 06/12] plugin.test.ts: Switch to testutil.HttpServer

---
 test/plugin.test.ts | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/test/plugin.test.ts b/test/plugin.test.ts
index 305cf041a..167804bbd 100644
--- a/test/plugin.test.ts
+++ b/test/plugin.test.ts
@@ -1,11 +1,12 @@
 import { logger } from "@coder/logger"
+import * as assert from "assert"
 import * as express from "express"
 import * as fs from "fs"
 import { describe } from "mocha"
 import * as path from "path"
-import * as supertest from "supertest"
 import { PluginAPI } from "../src/node/plugin"
 import * as apps from "../src/node/routes/apps"
+import * as testutil from "./testutil"
 const fsp = fs.promises
 
 /**
@@ -13,23 +14,30 @@ const fsp = fs.promises
  */
 describe("plugin", () => {
   let papi: PluginAPI
-  let app: express.Application
-  let agent: supertest.SuperAgentTest
+  let s: testutil.HttpServer
 
   before(async () => {
-    papi = new PluginAPI(logger, path.resolve(__dirname, "test-plugin") + ":meow")
+    papi = new PluginAPI(logger, `${path.resolve(__dirname, "test-plugin")}:meow`)
     await papi.loadPlugins()
 
-    app = express.default()
+    const app = express.default()
     papi.mount(app)
-
     app.use("/api/applications", apps.router(papi))
 
-    agent = supertest.agent(app)
+    s = new testutil.HttpServer()
+    await s.listen(app)
+  })
+
+  after(async () => {
+    await s.close()
   })
 
   it("/api/applications", async () => {
-    await agent.get("/api/applications").expect(200, [
+    const resp = await s.fetch("/api/applications")
+    assert.equal(200, resp.status)
+    const body = await resp.json()
+    logger.debug(`${JSON.stringify(body)}`)
+    assert.deepEqual(body, [
       {
         name: "Test App",
         version: "4.0.0",
@@ -57,6 +65,9 @@ describe("plugin", () => {
     const indexHTML = await fsp.readFile(path.join(__dirname, "test-plugin/public/index.html"), {
       encoding: "utf8",
     })
-    await agent.get("/test-plugin/test-app").expect(200, indexHTML)
+    const resp = await s.fetch("/test-plugin/test-app")
+    assert.equal(200, resp.status)
+    const body = await resp.text()
+    assert.equal(body, indexHTML)
   })
 })

From d3074278ca26102650e728fd4102b5b1cc330997 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 09:49:23 -0500
Subject: [PATCH 07/12] app.ts: Fix createApp to log all http server errors

cc @code-asher
---
 src/common/util.ts |  8 ++++++++
 src/node/app.ts    | 18 ++++++++++++++++--
 test/testutil.ts   | 15 +++------------
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/src/common/util.ts b/src/common/util.ts
index b4f66be24..87ca6f596 100644
--- a/src/common/util.ts
+++ b/src/common/util.ts
@@ -112,3 +112,11 @@ export const getFirstString = (value: string | string[] | object | undefined): s
 
   return typeof value === "string" ? value : undefined
 }
+
+export function logError(prefix: string, err: any): void {
+  if (err instanceof Error) {
+    logger.error(`${prefix}: ${err.message} ${err.stack}`)
+  } else {
+    logger.error(`${prefix}: ${err}`)
+  }
+}
diff --git a/src/node/app.ts b/src/node/app.ts
index 448ec9660..1d9ce9d4a 100644
--- a/src/node/app.ts
+++ b/src/node/app.ts
@@ -3,6 +3,7 @@ import express, { Express } from "express"
 import { promises as fs } from "fs"
 import http from "http"
 import * as httpolyglot from "httpolyglot"
+import * as util from "../common/util"
 import { DefaultedArgs } from "./cli"
 import { handleUpgrade } from "./wsRouter"
 
@@ -22,8 +23,21 @@ export const createApp = async (args: DefaultedArgs): Promise<[Express, Express,
       )
     : http.createServer(app)
 
-  await new Promise<http.Server>(async (resolve, reject) => {
-    server.on("error", reject)
+  let resolved = false
+  await new Promise<http.Server>(async (resolve2, reject) => {
+    const resolve = () => {
+      resolved = true
+      resolve2()
+    }
+    server.on("error", (err) => {
+      if (!resolved) {
+        reject(err)
+      } else {
+        // Promise resolved earlier so this is an unrelated error.
+        util.logError("http server error", err)
+      }
+    })
+
     if (args.socket) {
       try {
         await fs.unlink(args.socket)
diff --git a/test/testutil.ts b/test/testutil.ts
index 561662758..dc88da3db 100644
--- a/test/testutil.ts
+++ b/test/testutil.ts
@@ -1,7 +1,7 @@
 import * as http from "http"
-import { logger } from "@coder/logger"
-import { ensureAddress } from "../src/node/app"
 import * as nodeFetch from "node-fetch"
+import * as util from "../src/common/util"
+import { ensureAddress } from "../src/node/app"
 
 export class HttpServer {
   private hs = http.createServer()
@@ -25,7 +25,7 @@ export class HttpServer {
           rej(err)
         } else {
           // Promise resolved earlier so this is some other error.
-          logError("server error", err)
+          util.logError("http server error", err)
         }
       })
     })
@@ -54,12 +54,3 @@ export class HttpServer {
     return nodeFetch.default(`${ensureAddress(this.hs)}${requestPath}`, opts)
   }
 }
-
-
-export function logError(prefix: string, err: any): void {
-  if (err instanceof Error) {
-    logger.error(`${prefix}: ${err.message} ${err.stack}`)
-  } else {
-    logger.error(`${prefix}: ${err}`)
-  }
-}

From 64e915de4a8431932da71c3530dbfefdfa14347b Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 09:53:34 -0500
Subject: [PATCH 08/12] test: Rename testutil.ts to httpserver.ts

---
 test/{testutil.ts => httpserver.ts} | 0
 test/plugin.test.ts                 | 6 +++---
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename test/{testutil.ts => httpserver.ts} (100%)

diff --git a/test/testutil.ts b/test/httpserver.ts
similarity index 100%
rename from test/testutil.ts
rename to test/httpserver.ts
diff --git a/test/plugin.test.ts b/test/plugin.test.ts
index 167804bbd..1259a91ee 100644
--- a/test/plugin.test.ts
+++ b/test/plugin.test.ts
@@ -6,7 +6,7 @@ import { describe } from "mocha"
 import * as path from "path"
 import { PluginAPI } from "../src/node/plugin"
 import * as apps from "../src/node/routes/apps"
-import * as testutil from "./testutil"
+import * as httpserver from "./httpserver"
 const fsp = fs.promises
 
 /**
@@ -14,7 +14,7 @@ const fsp = fs.promises
  */
 describe("plugin", () => {
   let papi: PluginAPI
-  let s: testutil.HttpServer
+  let s: httpserver.HttpServer
 
   before(async () => {
     papi = new PluginAPI(logger, `${path.resolve(__dirname, "test-plugin")}:meow`)
@@ -24,7 +24,7 @@ describe("plugin", () => {
     papi.mount(app)
     app.use("/api/applications", apps.router(papi))
 
-    s = new testutil.HttpServer()
+    s = new httpserver.HttpServer()
     await s.listen(app)
   })
 

From 240c8e266ed0e18517e5c98e0ab270ce63b6e446 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 10:53:58 -0500
Subject: [PATCH 09/12] test: Implement integration.ts for near full stack
 integration testing

---
 src/node/cli.ts     | 16 ++++++++++++++--
 test/httpserver.ts  | 16 ++++++++++++++++
 test/integration.ts | 16 ++++++++++++++++
 3 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 test/integration.ts

diff --git a/src/node/cli.ts b/src/node/cli.ts
index 5a0e4473e..2469bedbf 100644
--- a/src/node/cli.ts
+++ b/src/node/cli.ts
@@ -244,7 +244,7 @@ export const optionDescriptions = (): string[] => {
 export const parse = (
   argv: string[],
   opts?: {
-    configFile: string
+    configFile?: string
   },
 ): Args => {
   const error = (msg: string): Error => {
@@ -521,7 +521,19 @@ export async function readConfigFile(configPath?: string): Promise<ConfigArgs> {
   }
 
   const configFile = await fs.readFile(configPath)
-  const config = yaml.safeLoad(configFile.toString(), {
+  return parseConfigFile(configFile.toString(), configPath)
+}
+
+/**
+ * parseConfigFile parses configFile into ConfigArgs.
+ * configPath is used as the filename in error messages
+ */
+export function parseConfigFile(configFile: string, configPath: string): ConfigArgs {
+  if (configFile == "") {
+    return  { _: [], config: configPath }
+  }
+
+  const config = yaml.safeLoad(configFile, {
     filename: configPath,
   })
   if (!config || typeof config === "string") {
diff --git a/test/httpserver.ts b/test/httpserver.ts
index dc88da3db..570adf13d 100644
--- a/test/httpserver.ts
+++ b/test/httpserver.ts
@@ -3,9 +3,17 @@ import * as nodeFetch from "node-fetch"
 import * as util from "../src/common/util"
 import { ensureAddress } from "../src/node/app"
 
+// Perhaps an abstraction similar to this should be used in app.ts as well.
 export class HttpServer {
   private hs = http.createServer()
 
+  public constructor(hs?: http.Server) {
+    // See usage in test/integration.ts
+    if (hs) {
+      this.hs = hs
+    }
+  }
+
   /**
    * listen starts the server on a random localhost port.
    * Use close to cleanup when done.
@@ -53,4 +61,12 @@ export class HttpServer {
   public fetch(requestPath: string, opts?: nodeFetch.RequestInit): Promise<nodeFetch.Response> {
     return nodeFetch.default(`${ensureAddress(this.hs)}${requestPath}`, opts)
   }
+
+  public port(): number {
+    const addr = this.hs.address()
+    if (addr && typeof addr == "object") {
+      return addr.port
+    }
+    throw new Error("server not listening or listening on unix socket")
+  }
 }
diff --git a/test/integration.ts b/test/integration.ts
new file mode 100644
index 000000000..a4cc7586d
--- /dev/null
+++ b/test/integration.ts
@@ -0,0 +1,16 @@
+import { createApp } from "../src/node/app"
+import { register } from "../src/node/routes"
+import { parse, setDefaults, parseConfigFile, DefaultedArgs } from  "../src/node/cli"
+import * as httpserver from "./httpserver"
+import * as express from "express"
+
+export async function setup(argv: string[], configFile?: string): Promise<[express.Application, express.Application, httpserver.HttpServer, DefaultedArgs]> {
+  const cliArgs = parse(argv)
+  let configArgs = parseConfigFile(configFile || "", "test/integration.ts")
+  const args = await setDefaults(cliArgs, configArgs)
+
+  const [app, wsApp, server] = await createApp(args)
+  await register(app, wsApp, server, args)
+
+  return [app, wsApp, new httpserver.HttpServer(server), args]
+}

From 60233d09746b5e8792bc94440d0d0eddf3b69abf Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 10:54:34 -0500
Subject: [PATCH 10/12] test/proxy.test.ts: Implement

---
 test/proxy.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 test/proxy.test.ts

diff --git a/test/proxy.test.ts b/test/proxy.test.ts
new file mode 100644
index 000000000..defb99166
--- /dev/null
+++ b/test/proxy.test.ts
@@ -0,0 +1,47 @@
+import * as integration from "./integration"
+import * as httpserver from "./httpserver"
+import * as express from "express"
+import * as assert from "assert"
+
+describe("proxy", () => {
+  let codeServer: httpserver.HttpServer | undefined
+  let nhooyrDevServer = new httpserver.HttpServer()
+  let proxyPath: string
+
+  before(async () => {
+    const e = express.default()
+    await nhooyrDevServer.listen(e)
+    e.get("/wsup", (req, res) => {
+      res.json("asher is the best")
+    })
+    proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
+    e.get(proxyPath, (req, res) => {
+      res.json("joe is the best")
+    })
+  })
+
+  after(async () => {
+    await nhooyrDevServer.close()
+  })
+
+  afterEach(async () => {
+    if (codeServer) {
+      await codeServer.close()
+      codeServer = undefined
+    }
+  })
+
+  it("should rewrite the base path", async () => {
+    ;[,, codeServer,] = await integration.setup(["--auth=none"], "")
+    const resp = await codeServer.fetch(proxyPath)
+    assert.equal(resp.status, 200)
+    assert.equal(await resp.json(), "asher is the best")
+  })
+
+  it("should not rewrite the base path", async () => {
+    ;[,,codeServer,] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
+    const resp = await codeServer.fetch(proxyPath)
+    assert.equal(resp.status, 200)
+    assert.equal(await resp.json(), "joe is the best")
+  })
+})

From 5c06646f5804603ee6ef7104ac858872a3fbda35 Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Thu, 14 Jan 2021 10:55:19 -0500
Subject: [PATCH 11/12] Formatting and linting fixes

---
 src/node/cli.ts     |  4 ++--
 test/httpserver.ts  |  2 +-
 test/integration.ts | 15 +++++++++------
 test/proxy.test.ts  | 12 ++++++------
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/node/cli.ts b/src/node/cli.ts
index 2469bedbf..c8ca387a9 100644
--- a/src/node/cli.ts
+++ b/src/node/cli.ts
@@ -529,8 +529,8 @@ export async function readConfigFile(configPath?: string): Promise<ConfigArgs> {
  * configPath is used as the filename in error messages
  */
 export function parseConfigFile(configFile: string, configPath: string): ConfigArgs {
-  if (configFile == "") {
-    return  { _: [], config: configPath }
+  if (!configFile) {
+    return { _: [], config: configPath }
   }
 
   const config = yaml.safeLoad(configFile, {
diff --git a/test/httpserver.ts b/test/httpserver.ts
index 570adf13d..50f887863 100644
--- a/test/httpserver.ts
+++ b/test/httpserver.ts
@@ -64,7 +64,7 @@ export class HttpServer {
 
   public port(): number {
     const addr = this.hs.address()
-    if (addr && typeof addr == "object") {
+    if (addr && typeof addr === "object") {
       return addr.port
     }
     throw new Error("server not listening or listening on unix socket")
diff --git a/test/integration.ts b/test/integration.ts
index a4cc7586d..6a321cae0 100644
--- a/test/integration.ts
+++ b/test/integration.ts
@@ -1,12 +1,15 @@
-import { createApp } from "../src/node/app"
-import { register } from "../src/node/routes"
-import { parse, setDefaults, parseConfigFile, DefaultedArgs } from  "../src/node/cli"
-import * as httpserver from "./httpserver"
 import * as express from "express"
+import { createApp } from "../src/node/app"
+import { parse, setDefaults, parseConfigFile, DefaultedArgs } from "../src/node/cli"
+import { register } from "../src/node/routes"
+import * as httpserver from "./httpserver"
 
-export async function setup(argv: string[], configFile?: string): Promise<[express.Application, express.Application, httpserver.HttpServer, DefaultedArgs]> {
+export async function setup(
+  argv: string[],
+  configFile?: string,
+): Promise<[express.Application, express.Application, httpserver.HttpServer, DefaultedArgs]> {
   const cliArgs = parse(argv)
-  let configArgs = parseConfigFile(configFile || "", "test/integration.ts")
+  const configArgs = parseConfigFile(configFile || "", "test/integration.ts")
   const args = await setDefaults(cliArgs, configArgs)
 
   const [app, wsApp, server] = await createApp(args)
diff --git a/test/proxy.test.ts b/test/proxy.test.ts
index defb99166..88948fa10 100644
--- a/test/proxy.test.ts
+++ b/test/proxy.test.ts
@@ -1,11 +1,11 @@
-import * as integration from "./integration"
-import * as httpserver from "./httpserver"
-import * as express from "express"
 import * as assert from "assert"
+import * as express from "express"
+import * as httpserver from "./httpserver"
+import * as integration from "./integration"
 
 describe("proxy", () => {
   let codeServer: httpserver.HttpServer | undefined
-  let nhooyrDevServer = new httpserver.HttpServer()
+  const nhooyrDevServer = new httpserver.HttpServer()
   let proxyPath: string
 
   before(async () => {
@@ -32,14 +32,14 @@ describe("proxy", () => {
   })
 
   it("should rewrite the base path", async () => {
-    ;[,, codeServer,] = await integration.setup(["--auth=none"], "")
+    ;[, , codeServer] = await integration.setup(["--auth=none"], "")
     const resp = await codeServer.fetch(proxyPath)
     assert.equal(resp.status, 200)
     assert.equal(await resp.json(), "asher is the best")
   })
 
   it("should not rewrite the base path", async () => {
-    ;[,,codeServer,] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
+    ;[, , codeServer] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
     const resp = await codeServer.fetch(proxyPath)
     assert.equal(resp.status, 200)
     assert.equal(await resp.json(), "joe is the best")

From c32d8b155ff0395ccee10275b88dec935736e4ea Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 18 Jan 2021 08:30:00 -0500
Subject: [PATCH 12/12] heart.ts: Fix leak when server closes

This had me very confused for quite a while until I did a binary search
inspection on route/index.ts. Only with the heart.beat line commented
out did my tests pass without leaking.

They weren't leaking fds but just this heartbeat timer and node of
course prints just fds that are active when it detects some sort of leak
I guess and that made the whole thing very confusing. These fds are not
leaked and will close when node's event loop detects there are no more
callbacks to run.

no of handles 3

tcp stream {
  fd: 20,
  readable: false,
  writable: true,
  address: {},
  serverAddr: null
}

tcp stream {
  fd: 22,
  readable: false,
  writable: true,
  address: {},
  serverAddr: null
}

tcp stream {
  fd: 23,
  readable: true,
  writable: false,
  address: {},
  serverAddr: null
}

It kept printing the above text again and again for 60s and then the
test binary times out I think. I'm not sure if it was node printing the
stuff above or if it was a mocha thing. But it was really confusing...

cc @code-asher for thoughts on what was going on.

edit: It was the leaked-handles import in socket.test.ts!!!
Not sure if we should keep it, this was really confusing and misleading.
---
 src/node/heart.ts        | 9 +++++++++
 src/node/routes/index.ts | 3 +++
 test/integration.ts      | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/src/node/heart.ts b/src/node/heart.ts
index eed070e4e..1eada79b3 100644
--- a/src/node/heart.ts
+++ b/src/node/heart.ts
@@ -45,4 +45,13 @@ export class Heart {
         })
     }, this.heartbeatInterval)
   }
+
+  /**
+   * Call to clear any heartbeatTimer for shutdown.
+   */
+  public dispose(): void {
+    if (typeof this.heartbeatTimer !== "undefined") {
+      clearTimeout(this.heartbeatTimer)
+    }
+  }
 }
diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts
index 4688d6a2e..006543679 100644
--- a/src/node/routes/index.ts
+++ b/src/node/routes/index.ts
@@ -55,6 +55,9 @@ export const register = async (
       })
     })
   })
+  server.on("close", () => {
+    heart.dispose()
+  })
 
   app.disable("x-powered-by")
   wsApp.disable("x-powered-by")
diff --git a/test/integration.ts b/test/integration.ts
index 6a321cae0..f829fe5de 100644
--- a/test/integration.ts
+++ b/test/integration.ts
@@ -8,6 +8,8 @@ export async function setup(
   argv: string[],
   configFile?: string,
 ): Promise<[express.Application, express.Application, httpserver.HttpServer, DefaultedArgs]> {
+  argv = ["--bind-addr=localhost:0", ...argv]
+
   const cliArgs = parse(argv)
   const configArgs = parseConfigFile(configFile || "", "test/integration.ts")
   const args = await setDefaults(cliArgs, configArgs)