Skip to content

Commit 71c6d3c

Browse files
committed
Match a deeplink's prefix and page name the way the router does
React Router compiles route paths with the `i` flag unless a route opts into `caseSensitive`, so /Deeplink/apikeys reaches this loader. The prefix strip required the literal lowercase /deeplink/, so the suffix came back empty and the link landed on the environment root instead of the page. The page-name lookup had the same problem one level down: /deeplink/APIKeys fell through even though /env/{env}/APIKeys would have matched. Fold the case of the prefix and of the first segment only, and resolve the name to the map's own spelling. Everything after the first segment is left exactly as written, since task and run ids are case-sensitive.
1 parent 1660450 commit 71c6d3c

2 files changed

Lines changed: 74 additions & 7 deletions

File tree

apps/webapp/app/utils/deeplinkPages.test.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1+
import { matchPath } from "@remix-run/router";
12
import { existsSync, readdirSync, statSync } from "node:fs";
23
import { join } from "node:path";
34
import { describe, expect, it } from "vitest";
4-
import { deeplinkSuffix, ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages";
5+
import {
6+
DEEPLINK_PATH_PREFIX,
7+
deeplinkSuffix,
8+
ENV_PAGE_TARGETS,
9+
resolveDeeplinkPage,
10+
} from "./deeplinkPages";
511

612
const ROUTES_DIR = join(__dirname, "../routes");
713

@@ -181,6 +187,35 @@ describe("resolveDeeplinkPage", () => {
181187
expect(resolveDeeplinkPage("metrics")).toBeUndefined();
182188
});
183189

190+
it("matches the page name whatever its case, and resolves it to the map's spelling", () => {
191+
// `/env/{env}/APIKeys` matches its route, so the short link has to agree rather than falling
192+
// through to the environment root.
193+
expect(resolveDeeplinkPage("APIKeys")).toBe("apikeys");
194+
expect(resolveDeeplinkPage("Waitpoints")).toBe("waitpoints/tokens");
195+
expect(resolveDeeplinkPage("TASKS")).toBe("");
196+
expect(resolveDeeplinkPage("Bulk-Actions")).toBe("bulk-actions");
197+
// Case doesn't turn a non-page into a page.
198+
expect(resolveDeeplinkPage("Nonsense")).toBeUndefined();
199+
expect(resolveDeeplinkPage("Metrics")).toBeUndefined();
200+
});
201+
202+
it("leaves the case of everything after the name alone", () => {
203+
// Only the name is folded. Ids are case-sensitive, so lowercasing one would break the link far
204+
// more thoroughly than the miss the folding fixes.
205+
expect(resolveDeeplinkPage("runs/run_ABC123")).toBe("runs/run_ABC123");
206+
expect(resolveDeeplinkPage("Runs/run_ABC123")).toBe("runs/run_ABC123");
207+
expect(resolveDeeplinkPage("TASKS/standard/My-Task")).toBe("tasks/standard/My-Task");
208+
// Grafted onto the prefix and already written out under it, both with the id untouched.
209+
expect(resolveDeeplinkPage("Waitpoints/waitpoint_ABC")).toBe("waitpoints/tokens/waitpoint_ABC");
210+
expect(resolveDeeplinkPage("Waitpoints/tokens/waitpoint_ABC")).toBe(
211+
"waitpoints/tokens/waitpoint_ABC"
212+
);
213+
// An escaped slash inside a capitalised id survives as one segment, as it does in lower case.
214+
expect(resolveDeeplinkPage("Tasks/standard/Group%2FMy-Task")).toBe(
215+
"tasks/standard/Group%2FMy-Task"
216+
);
217+
});
218+
184219
it("drops traversal segments, in plain and escaped spellings", () => {
185220
expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd");
186221
expect(resolveDeeplinkPage("../runs")).toBe("runs");
@@ -216,6 +251,26 @@ describe("deeplinkSuffix", () => {
216251
);
217252
});
218253

254+
it("strips the prefix whatever its case, and only the prefix", () => {
255+
expect(deeplinkSuffix("/Deeplink/apikeys")).toBe("apikeys");
256+
expect(deeplinkSuffix("/DEEPLINK/runs/run_ABC123")).toBe("runs/run_ABC123");
257+
// The remainder comes back as it was written, capitals and all.
258+
expect(deeplinkSuffix("/DeepLink/tasks/standard/My-Task")).toBe("tasks/standard/My-Task");
259+
expect(deeplinkSuffix("/Deeplink")).toBe("");
260+
expect(deeplinkSuffix("/Deeplink/")).toBe("");
261+
});
262+
263+
it("folds case because the route it is mounted on does", () => {
264+
// The assertion the test above rests on: React Router compiles a route path with the `i` flag
265+
// unless it opts into `caseSensitive`, so a capitalised prefix really does reach this loader
266+
// instead of 404ing before it. If that ever changed, the folding would be dead weight.
267+
const route = `${DEEPLINK_PATH_PREFIX}/*`;
268+
expect(matchPath(route, "/deeplink/apikeys")?.params["*"]).toBe("apikeys");
269+
expect(matchPath(route, "/Deeplink/apikeys")?.params["*"]).toBe("apikeys");
270+
// And the splat keeps the case it was given, which is why only the first segment is folded.
271+
expect(matchPath(route, "/DEEPLINK/APIKeys")?.params["*"]).toBe("APIKeys");
272+
});
273+
219274
it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => {
220275
expect(deeplinkSuffix("/deeplink")).toBe("");
221276
expect(deeplinkSuffix("/deeplink/")).toBe("");

apps/webapp/app/utils/deeplinkPages.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,15 @@ export const DEEPLINK_PATH_PREFIX = "/deeplink";
6969
* Returns "" for anything that is not under the prefix. That includes a pathname the URL parser has
7070
* already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can
7171
* leave the prefix entirely before this ever sees it.
72+
*
73+
* The prefix is matched case-insensitively because React Router's route matching is: it compiles
74+
* every path with the `i` flag unless the route opts into `caseSensitive`, so `/Deeplink/apikeys`
75+
* reaches this loader too. Only the prefix is folded — the remainder is returned as it was written,
76+
* since the ids after the first segment are case-sensitive.
7277
*/
7378
export function deeplinkSuffix(pathname: string): string {
7479
const withSlash = `${DEEPLINK_PATH_PREFIX}/`;
75-
if (!pathname.startsWith(withSlash)) return "";
80+
if (!pathname.toLowerCase().startsWith(withSlash)) return "";
7681

7782
return pathname.slice(withSlash.length);
7883
}
@@ -105,18 +110,25 @@ function isUsableSegment(segment: string): boolean {
105110
*
106111
* `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an
107112
* `encodeURIComponent` pass here would double-encode every id that contains an escape.
113+
*
114+
* Only the first segment is matched case-insensitively, to the same end as the prefix in
115+
* `deeplinkSuffix`: `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should
116+
* reach it rather than falling through to the environment root. The name resolves to the map's own
117+
* spelling, and every segment after it is left exactly as written — folding the case of a task or
118+
* run id would break the link far more thoroughly than the miss this fixes.
108119
*/
109120
export function resolveDeeplinkPage(suffix: string): string | undefined {
110-
const segments = suffix.split("/").filter(isUsableSegment);
121+
const [first = "", ...rest] = suffix.split("/").filter(isUsableSegment);
111122

112-
const target = ENV_PAGE_TARGETS.get(segments[0] ?? "");
123+
const name = first.toLowerCase();
124+
const target = ENV_PAGE_TARGETS.get(name);
113125
if (target === undefined) return undefined;
114126

115-
if (segments.length === 1) return target.landing;
127+
if (rest.length === 0) return target.landing;
116128

117-
const written = segments.join("/");
129+
const written = [name, ...rest].join("/");
118130
//already written out under the prefix, so grafting would duplicate it
119131
if (written === target.prefix || written.startsWith(`${target.prefix}/`)) return written;
120132

121-
return [target.prefix, ...segments.slice(1)].join("/");
133+
return [target.prefix, ...rest].join("/");
122134
}

0 commit comments

Comments
 (0)