Merge pull request #2674 from nhooyr/absproxy

Add /absproxy to remove --proxy-path-passthrough

Author: nhooyr

docs/FAQ.md

@@ -16,6 +16,7 @@
   - [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)
+  - [Proxying to Create React App](#proxying-to-create-react-app)
 - [Multi-tenancy](#multi-tenancy)
 - [Docker in code-server container?](#docker-in-code-server-container)
 - [How can I disable telemetry?](#how-can-i-disable-telemetry)
@@ -226,25 +227,28 @@ However many people prefer the cleaner aesthetic of no trailing slashes. This co
 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.
+For users who are ok with this tradeoff, use `/absproxy` instead and the path will be
+passed as is. e.g. `/absproxy/3000/my-app-path`
 
-This is particularly a problem with the `start` script in create-react-app. See
+### Proxying to Create React App
+
+You must use `/absproxy/<port>` with create-react-app.
+See [#2565](https://github.com/cdr/code-server/issues/2565) and
 [#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:
+create-react-app of the path at which you are serving via `$PUBLIC_URL` and webpack
+via `$WDS_SOCKET_PATH`.
 
-```json
-  "homepage": "/proxy/3000",
+e.g.
+
+```sh
+PUBLIC_URL=/absproxy/3000 \
+  WDS_SOCKET_PATH=$PUBLIC_URL/sockjs-node \
+  BROWSER=none yarn start
 ```
 
-Then visit `https://my-code-server-address.io/proxy/3000` to see your app exposed through
+Then visit `https://my-code-server-address.io/absproxy/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

src/node/cli.ts

@@ -50,7 +50,6 @@ export interface Args extends VsArgs {
   "show-versions"?: boolean
   "uninstall-extension"?: string[]
   "proxy-domain"?: string[]
-  "proxy-path-passthrough"?: boolean
   locale?: string
   _: string[]
   "reuse-window"?: boolean
@@ -173,10 +172,6 @@ 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",

src/node/proxy.ts

@@ -9,8 +9,7 @@ proxy.on("error", (error, _, res) => {
 })
 
 // Intercept the response to rewrite absolute redirects against the base path.
-// Is disabled when the request has no base path which means --proxy-path-passthrough has
-// been enabled.
+// Is disabled when the request has no base path which means /absproxy is in use.
 proxy.on("proxyRes", (res, req) => {
   if (res.headers.location && res.headers.location.startsWith("/") && (req as any).base) {
     res.headers.location = (req as any).base + res.headers.location

src/node/routes/index.ts

@@ -103,8 +103,25 @@ export const register = async (
   app.use("/", domainProxy.router)
   wsApp.use("/", domainProxy.wsRouter.router)
 
-  app.use("/proxy", proxy.router)
-  wsApp.use("/proxy", proxy.wsRouter.router)
+  app.all("/proxy/(:port)(/*)?", (req, res) => {
+    proxy.proxy(req, res)
+  })
+  wsApp.get("/proxy/(:port)(/*)?", (req, res) => {
+    proxy.wsProxy(req as WebsocketRequest)
+  })
+  // These two routes pass through the path directly.
+  // So the proxied app must be aware it is running
+  // under /absproxy/<someport>/
+  app.all("/absproxy/(:port)(/*)?", (req, res) => {
+    proxy.proxy(req, res, {
+      passthroughPath: true,
+    })
+  })
+  wsApp.get("/absproxy/(:port)(/*)?", (req, res) => {
+    proxy.wsProxy(req as WebsocketRequest, {
+      passthroughPath: true,
+    })
+  })
 
   app.use(bodyParser.json())
   app.use(bodyParser.urlencoded({ extended: true }))

src/node/routes/pathProxy.ts

@@ -1,22 +1,27 @@
-import { Request, Router } from "express"
+import { Request, Response } from "express"
+import * as path from "path"
 import qs from "qs"
 import { HttpCode, HttpError } from "../../common/http"
 import { normalize } from "../../common/util"
 import { authenticated, ensureAuthenticated, redirect } from "../http"
-import { proxy } from "../proxy"
-import { Router as WsRouter } from "../wsRouter"
+import { proxy as _proxy } from "../proxy"
+import { WebsocketRequest } from "../wsRouter"
 
-export const router = Router()
-
-const getProxyTarget = (req: Request, passthroughPath: boolean): string => {
+const getProxyTarget = (req: Request, passthroughPath?: boolean): string => {
   if (passthroughPath) {
     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) => {
+export function proxy(
+  req: Request,
+  res: Response,
+  opts?: {
+    passthroughPath?: boolean
+  },
+): void {
   if (!authenticated(req)) {
     // If visiting the root (/:port only) redirect to the login page.
     if (!req.params[0] || req.params[0] === "/") {
@@ -28,22 +33,27 @@ router.all("/(:port)(/*)?", (req, res) => {
     throw new HttpError("Unauthorized", HttpCode.Unauthorized)
   }
 
-  if (!req.args["proxy-path-passthrough"]) {
+  if (!opts?.passthroughPath) {
     // Absolute redirects need to be based on the subpath when rewriting.
-    ;(req as any).base = `${req.baseUrl}/${req.params.port}`
+    // See proxy.ts.
+    ;(req as any).base = req.path.split(path.sep).slice(0, 3).join(path.sep)
   }
 
-  proxy.web(req, res, {
+  _proxy.web(req, res, {
     ignorePath: true,
-    target: getProxyTarget(req, req.args["proxy-path-passthrough"] || false),
+    target: getProxyTarget(req, opts?.passthroughPath),
   })
-})
-
-export const wsRouter = WsRouter()
+}
 
-wsRouter.ws("/(:port)(/*)?", ensureAuthenticated, (req) => {
-  proxy.ws(req, req.ws, req.head, {
+export function wsProxy(
+  req: WebsocketRequest,
+  opts?: {
+    passthroughPath?: boolean
+  },
+): void {
+  ensureAuthenticated(req)
+  _proxy.ws(req, req.ws, req.head, {
     ignorePath: true,
-    target: getProxyTarget(req, req.args["proxy-path-passthrough"] || false),
+    target: getProxyTarget(req, opts?.passthroughPath),
   })
-})
+}

test/proxy.test.ts

@@ -7,13 +7,15 @@ describe("proxy", () => {
   const nhooyrDevServer = new httpserver.HttpServer()
   let codeServer: httpserver.HttpServer | undefined
   let proxyPath: string
+  let absProxyPath: string
   let e: express.Express
 
   beforeAll(async () => {
     await nhooyrDevServer.listen((req, res) => {
       e(req, res)
     })
     proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
+    absProxyPath = proxyPath.replace("/proxy/", "/absproxy/")
   })
 
   afterAll(async () => {
@@ -43,11 +45,11 @@ describe("proxy", () => {
   })
 
   it("should not rewrite the base path", async () => {
-    e.get(proxyPath, (req, res) => {
+    e.get(absProxyPath, (req, res) => {
       res.json("joe is the best")
     })
-    ;[, , codeServer] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
-    const resp = await codeServer.fetch(proxyPath)
+    ;[, , codeServer] = await integration.setup(["--auth=none"], "")
+    const resp = await codeServer.fetch(absProxyPath)
     expect(resp.status).toBe(200)
     const json = await resp.json()
     expect(json).toBe("joe is the best")
@@ -69,15 +71,15 @@ describe("proxy", () => {
   })
 
   it("should not rewrite redirects", async () => {
-    const finalePath = proxyPath.replace("/wsup", "/finale")
-    e.post(proxyPath, (req, res) => {
+    const finalePath = absProxyPath.replace("/wsup", "/finale")
+    e.post(absProxyPath, (req, res) => {
       res.redirect(307, finalePath)
     })
     e.post(finalePath, (req, res) => {
       res.json("redirect success")
     })
-    ;[, , codeServer] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
-    const resp = await codeServer.fetch(proxyPath, {
+    ;[, , codeServer] = await integration.setup(["--auth=none"], "")
+    const resp = await codeServer.fetch(absProxyPath, {
       method: "POST",
     })
     expect(resp.status).toBe(200)