feat(http): keep slashes in queryParams in redirects (#4928)

* refactor(http): extract logic into constructRedirectPath

This allows us to easily test our redirect path construction logic where we get
the relative path, the query string and construct a redirect path.

By extracting this from `redirect`, we can easily test this logic in a unit
test.

I did this so we could test some logic where slashes in query strings should be
made human-friendly for users.

* feat(testing): add tests for constructRedirectPath

Co-authored-by: Asher <ash@coder.com>

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",

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)
+  })
+})

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"