diff --git a/src/node/routes/pathProxy.ts b/src/node/routes/pathProxy.ts index e86a430f9..a2281eab0 100644 --- a/src/node/routes/pathProxy.ts +++ b/src/node/routes/pathProxy.ts @@ -1,17 +1,14 @@ import { Request, Response } from "express" import * as path from "path" -import * as qs from "qs" import * as pluginapi from "../../../typings/pluginapi" import { HttpCode, HttpError } from "../../common/http" import { ensureProxyEnabled, authenticated, ensureAuthenticated, ensureOrigin, redirect, self } from "../http" import { proxy as _proxy } from "../proxy" -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 encodeURI(`http://0.0.0.0:${req.params.port}${req.params[0] || ""}${query ? `?${query}` : ""}`) +const getProxyTarget = (req: Request): string => { + // If there is a base path, strip it out. + const base = (req as any).base || "" + return `http://0.0.0.0:${req.params.port}/${req.originalUrl.slice(base.length)}` } export async function proxy( @@ -34,15 +31,14 @@ export async function proxy( throw new HttpError("Unauthorized", HttpCode.Unauthorized) } + // The base is used for rewriting (redirects, target). if (!opts?.passthroughPath) { - // Absolute redirects need to be based on the subpath when rewriting. - // See proxy.ts. ;(req as any).base = req.path.split(path.sep).slice(0, 3).join(path.sep) } _proxy.web(req, res, { ignorePath: true, - target: getProxyTarget(req, opts?.passthroughPath), + target: getProxyTarget(req), }) } @@ -55,8 +51,14 @@ export async function wsProxy( ensureProxyEnabled(req) ensureOrigin(req) await ensureAuthenticated(req) + + // The base is used for rewriting (redirects, target). + if (!opts?.passthroughPath) { + ;(req as any).base = req.path.split(path.sep).slice(0, 3).join(path.sep) + } + _proxy.ws(req, req.ws, req.head, { ignorePath: true, - target: getProxyTarget(req, opts?.passthroughPath), + target: getProxyTarget(req), }) } diff --git a/test/unit/node/proxy.test.ts b/test/unit/node/proxy.test.ts index 6f025a2e4..fc347ff83 100644 --- a/test/unit/node/proxy.test.ts +++ b/test/unit/node/proxy.test.ts @@ -208,6 +208,54 @@ describe("proxy", () => { const json = await resp.json() expect(json).toBe("ほげ") }) + + it("should not double-encode query variables", async () => { + const spy = jest.fn() + e.get("*", (req, res) => { + spy([req.originalUrl, req.query]) + res.end() + }) + codeServer = await integration.setup(["--auth=none"], "") + for (const test of [ + { + endpoint: proxyPath, + query: { foo: "bar with spaces" }, + expected: "/wsup?foo=bar+with+spaces", + }, + { + endpoint: absProxyPath, + query: { foo: "bar with spaces" }, + expected: absProxyPath + "?foo=bar+with+spaces", + }, + { + endpoint: proxyPath, + query: { foo: "with-&-ampersand" }, + expected: "/wsup?foo=with-%26-ampersand", + }, + { + endpoint: absProxyPath, + query: { foo: "with-&-ampersand" }, + expected: absProxyPath + "?foo=with-%26-ampersand", + }, + { + endpoint: absProxyPath, + query: { foo: "ほげ ほげ" }, + expected: absProxyPath + "?foo=%E3%81%BB%E3%81%92+%E3%81%BB%E3%81%92", + }, + { + endpoint: proxyPath, + query: { foo: "ほげ ほげ" }, + expected: "/wsup?foo=%E3%81%BB%E3%81%92+%E3%81%BB%E3%81%92", + }, + ]) { + spy.mockClear() + const resp = await codeServer.fetch(test.endpoint, undefined, test.query) + expect(resp.status).toBe(200) + await resp.text() + expect(spy).toHaveBeenCalledTimes(1) + expect(spy).toHaveBeenCalledWith([test.expected, test.query]) + } + }) }) // NOTE@jsjoeio