Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions integration/fog-of-war-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,9 @@ test.describe("Fog of War", () => {
expect(await app.getHtml("#parent")).toMatch(`Parent`);
expect(await app.getHtml("#child2")).toMatch(`Child 2`);
expect(manifestRequests).toEqual([
expect.stringMatching(/\/__manifest\?paths=%2Fparent%2Fchild2&version=/),
expect.stringMatching(
/\/__manifest\?paths=%2Fparent%2C%2Fparent%2Fchild2&version=/,
),
]);
});

Expand Down Expand Up @@ -1065,7 +1067,7 @@ test.describe("Fog of War", () => {
await page.waitForSelector("#splat");
expect(await app.getHtml("#splat")).toMatch("Splat: b/c");
expect(manifestRequests).toEqual([
expect.stringMatching(/\/__manifest\?paths=%2Fb%2Fc&version=/),
expect.stringMatching(/\/__manifest\?paths=%2Fb%2C%2Fb%2Fc&version=/),
]);
});

Expand Down Expand Up @@ -1137,7 +1139,9 @@ test.describe("Fog of War", () => {
await app.clickLink("/not/a/path");
await page.waitForSelector("#error");
expect(manifestRequests).toEqual([
expect.stringMatching(/\/__manifest\?paths=%2Fnot%2Fa%2Fpath&version=/),
expect.stringMatching(
/\/__manifest\?paths=%2Fnot%2C%2Fnot%2Fa%2C%2Fnot%2Fa%2Fpath&version=/,
),
]);
manifestRequests = [];

Expand Down Expand Up @@ -1449,7 +1453,9 @@ test.describe("Fog of War", () => {
// Wait for eager discovery to kick off
await new Promise((r) => setTimeout(r, 500));
expect(manifestRequests).toEqual([
expect.stringMatching(/\/custom-manifest\?paths=%2Fa%2Fb&version=/),
expect.stringMatching(
/\/custom-manifest\?paths=%2Fa%2C%2Fa%2Fb&version=/,
),
]);

expect(wrongManifestRequests).toEqual([]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Precompute route branch matchers to avoid recompiling route path regexes during matching
25 changes: 25 additions & 0 deletions packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { getPathsWithAncestors } from "../../../lib/dom/ssr/fog-of-war";

describe("fog of war", () => {
describe("getPathsWithAncestors", () => {
test("adds parent paths", () => {
expect(getPathsWithAncestors(["/a/b/c"])).toEqual([
"/a",
"/a/b",
"/a/b/c",
]);
});

test("dedupes shared parent paths", () => {
expect(getPathsWithAncestors(["/a/b", "/a/c"])).toEqual([
"/a",
"/a/b",
"/a/c",
]);
});

test("normalizes paths without leading slashes", () => {
expect(getPathsWithAncestors(["a/b"])).toEqual(["/a", "/a/b"]);
});
});
});
43 changes: 43 additions & 0 deletions packages/react-router/__tests__/rsc/server-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import {
matchRSCServerRequest,
type RSCMatch,
type RSCRouteConfigEntry,
} from "../../lib/rsc/server.rsc";
import { URL_LIMIT } from "../../lib/dom/ssr/fog-of-war";

describe("RSC server", () => {
describe("manifest requests", () => {
test("rejects manifest requests over the URL limit", async () => {
let path = `/${"a".repeat(URL_LIMIT)}.manifest`;

let { response, match } = await matchManifestRequest(
new Request(`https://remix.run${path}`),
[],
);

expect(response.status).toBe(400);
expect(match).toBeUndefined();
});
});
});

async function matchManifestRequest(
request: Request,
routes: RSCRouteConfigEntry[],
) {
let match: RSCMatch | undefined;
let response = await matchRSCServerRequest({
createTemporaryReferenceSet: () => ({}),
request,
routes,
generateResponse(nextMatch) {
match = nextMatch;
return new Response(null, {
status: nextMatch.statusCode,
headers: nextMatch.headers,
});
},
});

return { response, match };
}
21 changes: 20 additions & 1 deletion packages/react-router/__tests__/server-runtime/server-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { createContext, type StaticHandlerContext } from "react-router";

import { createRequestHandler } from "../../lib/server-runtime/server";
import { ServerMode } from "../../lib/server-runtime/mode";
import { URL_LIMIT } from "../../lib/dom/ssr/fog-of-war";
import { mockServerBuild } from "./utils";

function spyConsole() {
Expand Down Expand Up @@ -2133,7 +2134,7 @@ describe("shared server runtime", () => {
let handler = createRequestHandler(build, ServerMode.Test);

let request = new Request(
`${baseUrl}/__manifest?paths=%2Fa%2Fb&version=${build.assets.version}`,
`${baseUrl}/__manifest?paths=%2Fa,%2Fa%2Fb&version=${build.assets.version}`,
);

let result = await handler(request);
Expand Down Expand Up @@ -2165,6 +2166,24 @@ describe("shared server runtime", () => {
});
});

test("rejects manifest requests over the URL limit", async () => {
let build = mockServerBuild({
root: {
default: {},
},
});
let handler = createRequestHandler(build, ServerMode.Test);

let request = new Request(
`${baseUrl}/__manifest?paths=${encodeURIComponent(
`/${"a".repeat(URL_LIMIT)}`,
)}&version=${build.assets.version}`,
);

let result = await handler(request);
expect(result.status).toBe(400);
});

test("disabled when route discovery is disabled", async () => {
let build = mockServerBuild(
{
Expand Down
27 changes: 27 additions & 0 deletions packages/react-router/lib/dom/ssr/fog-of-war.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,31 @@ const discoveredPaths = new Set<string>();
// https://stackoverflow.com/a/417184
export const URL_LIMIT = 7680;

export function getPathsWithAncestors(paths: string[]): string[] {
let result = new Set<string>();

paths.forEach((path) => {
if (!path.startsWith("/")) {
path = `/${path}`;
}
// In addition to the requested path, we need to include patches for each
// ancestor path so that we pick up any pathless/index routes below ancestor
// segments. So if we get a request for `/parent/child`, we need to look for
// a match on `/parent` so that if a `parent._index` route exists we return
// it and it's available for client side matching if the user routes back up
// to `/parent`. This is the same thing we do on initial load in <Scripts>
// via `getPartialManifest()`.
for (let i = 1; i < path.length; i++) {
if (path[i] === "/") {
result.add(path.slice(0, i));
}
}
result.add(path);
});

return Array.from(result);
}

export function isFogOfWarEnabled(
routeDiscovery: ServerBuild["routeDiscovery"],
ssr: boolean,
Expand Down Expand Up @@ -228,6 +253,8 @@ export async function fetchAndApplyManifestPatches(
patchRoutes: DataRouter["patchRoutes"],
signal?: AbortSignal,
): Promise<void> {
paths = getPathsWithAncestors(paths);

// NOTE: Intentionally using a standalone `URLSearchParams` instance
// instead of mutating `url.searchParams`, which is *significantly* slower:
// https://issues.chromium.org/issues/331406951
Expand Down
44 changes: 39 additions & 5 deletions packages/react-router/lib/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,8 @@ interface RouteMeta<RouteObjectType extends RouteObject = RouteObject> {
caseSensitive: boolean;
childrenIndex: number;
route: RouteObjectType;
matcher?: RegExp;
compiledParams?: CompiledPathParam[];
}

/**
Expand Down Expand Up @@ -1205,9 +1207,21 @@ function flattenRoutes<RouteObjectType extends RouteObject = RouteObject>(
branches.push({
path,
score: computeScore(path, route.index),
routesMeta,
routesMeta: routesMeta.map((meta, i) => {
let [matcher, params] = compilePath(
meta.relativePath,
meta.caseSensitive,
i === routesMeta.length - 1,
);
return {
...meta,
matcher,
compiledParams: params,
} satisfies RouteMeta<RouteObjectType>;
}),
});
};

routes.forEach((route, index) => {
// coarse-grain check for optional params
if (route.path === "" || !route.path?.includes("?")) {
Expand Down Expand Up @@ -1360,10 +1374,21 @@ function matchRouteBranch<
matchedPathname === "/"
? pathname
: pathname.slice(matchedPathname.length) || "/";
let match = matchPath(
{ path: meta.relativePath, caseSensitive: meta.caseSensitive, end },
remainingPathname,
);
let pattern = {
path: meta.relativePath,
caseSensitive: meta.caseSensitive,
end,
};
let match =
// Use precomputed matcher if it exists
meta.matcher && meta.compiledParams
? matchPathImpl(
pattern,
remainingPathname,
meta.matcher,
meta.compiledParams,
)
: matchPath(pattern, remainingPathname);

let route = meta.route;

Expand Down Expand Up @@ -1546,6 +1571,15 @@ export function matchPath<Path extends string>(
pattern.end,
);

return matchPathImpl(pattern, pathname, matcher, compiledParams);
}

function matchPathImpl<Path extends string>(
pattern: PathPattern<Path>,
pathname: string,
matcher: RegExp,
compiledParams: CompiledPathParam[],
): PathMatch<ParamParseKey<Path>> | null {
let match = pathname.match(matcher);
if (!match) return null;

Expand Down
4 changes: 3 additions & 1 deletion packages/react-router/lib/rsc/browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
import { RSCRouterGlobalErrorBoundary } from "./errorBoundaries";
import type { RouteModules } from "../dom/ssr/routeModules";
import { populateRSCRouteModules } from "./route-modules";
import { URL_LIMIT } from "../dom/ssr/fog-of-war";
import { URL_LIMIT, getPathsWithAncestors } from "../dom/ssr/fog-of-war";

const defaultManifestPath = "/__manifest";

Expand Down Expand Up @@ -1044,6 +1044,8 @@ async function fetchAndApplyManifestPatches(
fetchImplementation: (request: Request) => Promise<Response>,
signal?: AbortSignal,
) {
paths = getPathsWithAncestors(paths);

let url = getManifestUrl(paths);
if (url == null) {
return;
Expand Down
45 changes: 19 additions & 26 deletions packages/react-router/lib/rsc/server.rsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
} from "../router/utils";
import { getDocumentHeadersImpl } from "../server-runtime/headers";
import { SINGLE_FETCH_REDIRECT_STATUS } from "../dom/ssr/single-fetch";
import { URL_LIMIT, getPathsWithAncestors } from "../dom/ssr/fog-of-war";
import { throwIfPotentialCSRFAttack } from "../actions";
import invariant from "../server-runtime/invariant";

Expand Down Expand Up @@ -532,6 +533,14 @@ async function generateManifestResponse(
temporaryReferences: unknown,
routeDiscovery: RouteDiscovery | undefined,
) {
let url = new URL(request.url);
if (url.toString().length > URL_LIMIT) {
return new Response(null, {
statusText: "Bad Request",
status: 400,
});
}

if (routeDiscovery?.mode === "initial") {
let payload: RSCManifestPayload = {
type: "manifest",
Expand All @@ -550,7 +559,6 @@ async function generateManifestResponse(
);
}

let url = new URL(request.url);
let pathParam = url.searchParams.get("paths");
let pathnames = pathParam
? pathParam.split(",").filter(Boolean)
Expand Down Expand Up @@ -1193,7 +1201,7 @@ async function getRenderPayload(
),
)
: getAdditionalRoutePatches(
[staticContext.location.pathname],
getPathsWithAncestors([staticContext.location.pathname]),
routes,
basename,
staticContext.matches.map((m) => m.route.id),
Expand Down Expand Up @@ -1428,33 +1436,18 @@ async function getAdditionalRoutePatches(
let matchedPaths = new Set<string>();

for (const pathname of pathnames) {
let segments = pathname.split("/").filter(Boolean);
let paths: string[] = ["/"];

// We've already matched to the last segment
segments.pop();

// Traverse each path for our parents and match in case they have pathless/index
// children we need to include in the initial manifest
while (segments.length > 0) {
paths.push(`/${segments.join("/")}`);
segments.pop();
if (matchedPaths.has(pathname)) {
continue;
}

paths.forEach((path) => {
if (matchedPaths.has(path)) {
matchedPaths.add(pathname);
let matches = matchRoutes(routes, pathname, basename) || [];
matches.forEach((m, i) => {
if (patchRouteMatches.get(m.route.id)) {
return;
}
matchedPaths.add(path);
let matches = matchRoutes(routes, path, basename) || [];
matches.forEach((m, i) => {
if (patchRouteMatches.get(m.route.id)) {
return;
}
patchRouteMatches.set(m.route.id, {
...m.route,
parentId: matches[i - 1]?.route.id,
});
patchRouteMatches.set(m.route.id, {
...m.route,
parentId: matches[i - 1]?.route.id,
});
});
}
Expand Down
Loading