Archived
1
0

Fix query string being double-encoding over path proxy

Instead of trying to piece together the original URL and re-encode what
needs to be re-encoded, strip out the base from the original URL.

Fixes #6307.
This commit is contained in:
Asher 2024-01-11 11:21:08 -09:00
parent e87499c301
commit d49b3bf159
No known key found for this signature in database
GPG Key ID: D63C1EF81242354A
2 changed files with 61 additions and 11 deletions

View File

@ -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),
})
}

View File

@ -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