Merge pull request #2609 from cdr/proxy-res-d629

Fix body proxying, redirect proxying and add tests

Author: nhooyr

src/node/proxy.ts

@@ -9,6 +9,8 @@ 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.
 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

@@ -65,9 +65,6 @@ export const register = async (
   app.use(cookieParser())
   wsApp.use(cookieParser())
 
-  app.use(bodyParser.json())
-  app.use(bodyParser.urlencoded({ extended: true }))
-
   const common: express.RequestHandler = (req, _, next) => {
     // /healthz|/healthz/ needs to be excluded otherwise health checks will make
     // it look like code-server is always in use.
@@ -106,6 +103,12 @@ 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.use(bodyParser.json())
+  app.use(bodyParser.urlencoded({ extended: true }))
+
   app.use("/", vscode.router)
   wsApp.use("/", vscode.wsRouter.router)
   app.use("/vscode", vscode.router)
@@ -121,9 +124,6 @@ export const register = async (
     })
   }
 
-  app.use("/proxy", proxy.router)
-  wsApp.use("/proxy", proxy.wsRouter.router)
-
   app.use("/static", _static.router)
   app.use("/update", update.router)
 

src/node/routes/pathProxy.ts

@@ -28,8 +28,10 @@ router.all("/(:port)(/*)?", (req, res) => {
     throw new HttpError("Unauthorized", HttpCode.Unauthorized)
   }
 
-  // Absolute redirects need to be based on the subpath when rewriting.
-  ;(req as any).base = `${req.baseUrl}/${req.params.port}`
+  if (!req.args["proxy-path-passthrough"]) {
+    // Absolute redirects need to be based on the subpath when rewriting.
+    ;(req as any).base = `${req.baseUrl}/${req.params.port}`
+  }
 
   proxy.web(req, res, {
     ignorePath: true,

test/proxy.test.ts

@@ -1,28 +1,29 @@
+import bodyParser from "body-parser"
 import * as express from "express"
 import * as httpserver from "./httpserver"
 import * as integration from "./integration"
 
 describe("proxy", () => {
-  let codeServer: httpserver.HttpServer | undefined
   const nhooyrDevServer = new httpserver.HttpServer()
+  let codeServer: httpserver.HttpServer | undefined
   let proxyPath: string
+  let e: express.Express
 
   beforeAll(async () => {
-    const e = express.default()
-    await nhooyrDevServer.listen(e)
-    e.get("/wsup", (req, res) => {
-      res.json("asher is the best")
+    await nhooyrDevServer.listen((req, res) => {
+      e(req, res)
     })
     proxyPath = `/proxy/${nhooyrDevServer.port()}/wsup`
-    e.get(proxyPath, (req, res) => {
-      res.json("joe is the best")
-    })
   })
 
   afterAll(async () => {
     await nhooyrDevServer.close()
   })
 
+  beforeEach(() => {
+    e = express.default()
+  })
+
   afterEach(async () => {
     if (codeServer) {
       await codeServer.close()
@@ -31,6 +32,9 @@ describe("proxy", () => {
   })
 
   it("should rewrite the base path", async () => {
+    e.get("/wsup", (req, res) => {
+      res.json("asher is the best")
+    })
     ;[, , codeServer] = await integration.setup(["--auth=none"], "")
     const resp = await codeServer.fetch(proxyPath)
     expect(resp.status).toBe(200)
@@ -39,10 +43,61 @@ describe("proxy", () => {
   })
 
   it("should not rewrite the base path", async () => {
+    e.get(proxyPath, (req, res) => {
+      res.json("joe is the best")
+    })
     ;[, , codeServer] = await integration.setup(["--auth=none", "--proxy-path-passthrough=true"], "")
     const resp = await codeServer.fetch(proxyPath)
     expect(resp.status).toBe(200)
     const json = await resp.json()
     expect(json).toBe("joe is the best")
   })
+
+  it("should rewrite redirects", async () => {
+    e.post("/wsup", (req, res) => {
+      res.redirect(307, "/finale")
+    })
+    e.post("/finale", (req, res) => {
+      res.json("redirect success")
+    })
+    ;[, , codeServer] = await integration.setup(["--auth=none"], "")
+    const resp = await codeServer.fetch(proxyPath, {
+      method: "POST",
+    })
+    expect(resp.status).toBe(200)
+    expect(await resp.json()).toBe("redirect success")
+  })
+
+  it("should not rewrite redirects", async () => {
+    const finalePath = proxyPath.replace("/wsup", "/finale")
+    e.post(proxyPath, (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, {
+      method: "POST",
+    })
+    expect(resp.status).toBe(200)
+    expect(await resp.json()).toBe("redirect success")
+  })
+
+  it("should allow post bodies", async () => {
+    e.use(bodyParser.json({ strict: false }))
+    e.post("/wsup", (req, res) => {
+      res.json(req.body)
+    })
+    ;[, , codeServer] = await integration.setup(["--auth=none"], "")
+    const resp = await codeServer.fetch(proxyPath, {
+      method: "post",
+      body: JSON.stringify("coder is the best"),
+      headers: {
+        "Content-Type": "application/json",
+      },
+    })
+    expect(resp.status).toBe(200)
+    expect(await resp.json()).toBe("coder is the best")
+  })
 })