diff --git a/src/node/http.ts b/src/node/http.ts index f006036d3..fa9a84255 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -138,6 +138,21 @@ export const relativeRoot = (originalUrl: string): string => { return normalize("./" + (depth > 1 ? "../".repeat(depth - 1) : "")) } +/** + * A helper function to construct a redirect path based on + * an Express Request, query and a path to redirect to. + * + * Redirect path is relative to `/${to}`. + */ +export const constructRedirectPath = (req: express.Request, query: qs.ParsedQs, to: string): string => { + const relativePath = normalize(`${relativeRoot(req.originalUrl)}/${to}`, true) + // %2f or %2F are both equalivent to an encoded slash / + const queryString = qs.stringify(query).replace(/%2[fF]/g, "/") + const redirectPath = `${relativePath}${queryString ? `?${queryString}` : ""}` + + return redirectPath +} + /** * Redirect relatively to `/${to}`. Query variables on the current URI will be * preserved. `to` should be a simple path without any query parameters @@ -156,9 +171,7 @@ export const redirect = ( } }) - const relativePath = normalize(`${relativeRoot(req.originalUrl)}/${to}`, true) - const queryString = qs.stringify(query) - const redirectPath = `${relativePath}${queryString ? `?${queryString}` : ""}` + const redirectPath = constructRedirectPath(req, query, to) logger.debug(`redirecting from ${req.originalUrl} to ${redirectPath}`) res.redirect(redirectPath) } diff --git a/test/package.json b/test/package.json index d50371666..726251353 100644 --- a/test/package.json +++ b/test/package.json @@ -2,6 +2,7 @@ "license": "MIT", "#": "We must put jest in a sub-directory otherwise VS Code somehow picks up the types and generates conflicts with mocha.", "devDependencies": { + "@jest-mock/express": "^1.4.5", "@playwright/test": "^1.16.3", "@types/jest": "^27.0.2", "@types/jsdom": "^16.2.13", diff --git a/test/unit/node/http.test.ts b/test/unit/node/http.test.ts index 87e8e0419..b5dc20402 100644 --- a/test/unit/node/http.test.ts +++ b/test/unit/node/http.test.ts @@ -1,4 +1,5 @@ -import { relativeRoot } from "../../../src/node/http" +import { getMockReq } from "@jest-mock/express" +import { constructRedirectPath, relativeRoot } from "../../../src/node/http" describe("http", () => { it("should construct a relative path to the root", () => { @@ -9,3 +10,46 @@ describe("http", () => { expect(relativeRoot("/foo/bar/")).toStrictEqual("./../..") }) }) + +describe("constructRedirectPath", () => { + it("should preserve slashes in queryString so they are human-readable", () => { + const mockReq = getMockReq({ + originalUrl: "localhost:8080", + }) + const mockQueryParams = { folder: "/Users/jp/dev/coder" } + const mockTo = "" + const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo) + const expected = "./?folder=/Users/jp/dev/coder" + expect(actual).toBe(expected) + }) + it("should use an empty string if no query params", () => { + const mockReq = getMockReq({ + originalUrl: "localhost:8080", + }) + const mockQueryParams = {} + const mockTo = "" + const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo) + const expected = "./" + expect(actual).toBe(expected) + }) + it("should append the 'to' path relative to the originalUrl", () => { + const mockReq = getMockReq({ + originalUrl: "localhost:8080", + }) + const mockQueryParams = {} + const mockTo = "vscode" + const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo) + const expected = "./vscode" + expect(actual).toBe(expected) + }) + it("should append append queryParams after 'to' path", () => { + const mockReq = getMockReq({ + originalUrl: "localhost:8080", + }) + const mockQueryParams = { folder: "/Users/jp/dev/coder" } + const mockTo = "vscode" + const actual = constructRedirectPath(mockReq, mockQueryParams, mockTo) + const expected = "./vscode?folder=/Users/jp/dev/coder" + expect(actual).toBe(expected) + }) +}) diff --git a/test/yarn.lock b/test/yarn.lock index 35377913d..38e8e7cd9 100644 --- a/test/yarn.lock +++ b/test/yarn.lock @@ -478,6 +478,11 @@ resolved "https://registry.yarnpkg.com/@istanbuljs/schema/-/schema-0.1.3.tgz#e45e384e4b8ec16bce2fd903af78450f6bf7ec98" integrity sha512-ZXRY4jNvVgSVQ8DL3LTcakaAtXwTVUxE81hslsyD2AtoXW/wVob10HkOJ1X/pAlcI7D+2YoZKg5do8G/w6RYgA== +"@jest-mock/express@^1.4.5": + version "1.4.5" + resolved "https://registry.yarnpkg.com/@jest-mock/express/-/express-1.4.5.tgz#437db24ccd505d88f8c0d73e8593fa3cd6eb273b" + integrity sha512-bERM1jnutyH7VMahdaOHAKy7lgX47zJ7+RTz2eMz0wlCttd9CkhsKFEyoWmJBSz/ow0nVj3lCuRqLem4QDYFkQ== + "@jest/console@^27.4.6": version "27.4.6" resolved "https://registry.yarnpkg.com/@jest/console/-/console-27.4.6.tgz#0742e6787f682b22bdad56f9db2a8a77f6a86107"