From e4839284739740a6e0675ee7313a78183cefce5a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:13:35 +0000 Subject: [PATCH 01/12] Validate diff input: cap glob patterns, clamp options, return 400/404 instead of 500 - Reject diffFiles with more than 10 patterns, patterns longer than 256 characters, or patterns whose brace expansion exceeds 64 alternatives (checked with a capped minimatch braceExpand) before calling libnpmdiff. - Require diffUnified to be an integer 0-100 and diffSrcPrefix/diffDstPrefix to be at most 64 characters; invalid values are rejected, not coerced. - Share parts parsing (decode, split, validate, versionsToSpecs) between the API and the diff page; malformed parts give 400 in the API and notFound() on the page. - Registry not-found (E404/ETARGET) while resolving specs surfaces as SpecNotFoundError (resolved inside the cache boundary so the error code is not lost), mapped to 404 in the API and notFound() on the page. - Build redirect and navigation query strings with URLSearchParams so values stay encoded and repeated keys are preserved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0125jPsCGgQhzzC88zAyk33B --- package.json | 1 + pnpm-lock.yaml | 3 + .../_page/NpmDiff/ViewTypeSwitch.tsx | 18 +-- src/app/[...parts]/page.tsx | 73 +++++++----- src/app/api/[...parts]/route.ts | 40 +++++-- src/app/page.client.tsx | 7 +- src/lib/destination/SpecNotFoundError.ts | 6 + src/lib/destination/canonicalSpec.test.ts | 9 ++ src/lib/destination/canonicalSpec.ts | 30 ++++- src/lib/destination/index.ts | 2 + src/lib/query/index.ts | 5 +- src/lib/query/parseDiffOptions.test.ts | 108 ++++++++++++++++++ src/lib/query/parseDiffOptions.ts | 87 ++++++++++++++ src/lib/query/parseQuery/parseNumber.test.ts | 5 +- src/lib/query/parseQuery/parseNumber.ts | 2 +- src/lib/query/searchParams.test.ts | 47 ++++++++ src/lib/query/searchParams.ts | 40 +++++++ src/lib/utils/isRegistryNotFoundError.test.ts | 18 +++ src/lib/utils/isRegistryNotFoundError.ts | 7 ++ src/lib/utils/parseParts.test.ts | 51 +++++++++ src/lib/utils/parseParts.ts | 23 ++++ 21 files changed, 523 insertions(+), 59 deletions(-) create mode 100644 src/lib/destination/SpecNotFoundError.ts create mode 100644 src/lib/query/parseDiffOptions.test.ts create mode 100644 src/lib/query/parseDiffOptions.ts create mode 100644 src/lib/query/searchParams.test.ts create mode 100644 src/lib/query/searchParams.ts create mode 100644 src/lib/utils/isRegistryNotFoundError.test.ts create mode 100644 src/lib/utils/isRegistryNotFoundError.ts create mode 100644 src/lib/utils/parseParts.test.ts create mode 100644 src/lib/utils/parseParts.ts diff --git a/package.json b/package.json index 5a855cbd..a850d9bd 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "hosted-git-info": "^10.1.1", "libnpmdiff": "^9.0.2", "lucide-react": "^0.577.0", + "minimatch": "^10.2.6", "next": "~16.3.5", "npm-package-arg": "^14.0.0", "pacote": "^22.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 24dc3709..6705c7fb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -225,6 +225,9 @@ importers: lucide-react: specifier: ^0.577.0 version: 0.577.0(react@19.3.0) + minimatch: + specifier: ^10.2.6 + version: 10.2.6 next: specifier: ~16.3.5 version: 16.3.5(@babel/core@7.29.7)(@types/node@24.13.4)(babel-plugin-react-compiler@1.0.0)(react-dom@19.3.0)(react@19.3.0) diff --git a/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx b/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx index 7f1e5938..24b0d018 100644 --- a/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx +++ b/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx @@ -13,6 +13,15 @@ import { cx } from "^/lib/cva"; import useViewType from "^/lib/utils/useViewType"; import { DIFF_TYPE_PARAM_NAME } from "../paramNames"; +function withViewType( + searchParams: ReadonlyURLSearchParams | null, + viewType: ViewType, +): URLSearchParams { + const params = new URLSearchParams(searchParams?.toString()); + params.set(DIFF_TYPE_PARAM_NAME, viewType); + return params; +} + export interface ViewTypeButtonProps extends ButtonProps { currentViewType: ViewType; pathname: string | null; @@ -50,14 +59,7 @@ const ViewTypeButton = forwardRef< ref={ref} > { const { parts } = await params; - const specs = splitParts(decodeParts(parts)); + const specs = parseParts(parts, { decode: true }); - if (!validateSpecs(specs)) { + if (specs == null) { notFound(); } @@ -55,11 +62,12 @@ const DiffPageInner = async ({ searchParams, }: DiffPageProps): Promise => { const { parts } = await params; - const { diffFiles, ...optionsQuery } = await searchParams; + const query = await searchParams; + const { diffFiles, ...optionsQuery } = query; - const specsOrVersions = splitParts(decodeParts(parts)); + const specsOrVersions = parseParts(parts, { decode: true }); - if (!validateSpecs(specsOrVersions)) { + if (specsOrVersions == null) { notFound(); } @@ -67,25 +75,38 @@ const DiffPageInner = async ({ return ; } - const { redirect: redirectTarget, canonicalSpecs } = - await destination(specsOrVersions); + const parsedOptions = parseDiffOptions({ + // If no diffFiles is passed, use the default. + // This is done here, since we don't want a fall back in the API + diffFiles: diffFiles ?? DEFAULT_DIFF_FILES_GLOB, + ...optionsQuery, + }); - if (redirectTarget !== false) { - const specsStr = specsToDiff(canonicalSpecs); - const searchStr = Object.entries(await searchParams) - .map(([key, value]) => `${key}=${value}`) - .join("&"); - - redirect( - `/${specsStr}` + (searchStr?.length > 0 ? `?${searchStr}` : ""), + if (!parsedOptions.ok) { + return ( + +

Invalid options

+ {parsedOptions.message} + ); + } + + let target: Destination; + try { + target = await destination(specsOrVersions); + } catch (e) { + if (e instanceof SpecNotFoundError) { + notFound(); + } + throw e; + } + + const { redirect: redirectTarget, canonicalSpecs } = target; + + if (redirectTarget !== false) { + redirect(`/${specsToDiff(canonicalSpecs)}${toSearchString(query)}`); } else { - const options = parseQuery({ - // If no diffFiles is passed, use the default. - // This is done here, since we don't want a fall back in the API - diffFiles: diffFiles ?? DEFAULT_DIFF_FILES_GLOB, - ...optionsQuery, - }); + const { options } = parsedOptions; const [a, b] = canonicalSpecs.map((spec) => createSimplePackageSpec(spec), diff --git a/src/app/api/[...parts]/route.ts b/src/app/api/[...parts]/route.ts index 8bee7e31..8f4b9d0b 100644 --- a/src/app/api/[...parts]/route.ts +++ b/src/app/api/[...parts]/route.ts @@ -1,11 +1,13 @@ import { type NextRequest, NextResponse } from "next/server"; -import destination from "^/lib/destination"; +import destination, { + type Destination, + SpecNotFoundError, +} from "^/lib/destination"; import npmDiff from "^/lib/npmDiff"; -import { parseQuery } from "^/lib/query"; +import { fromSearchParams, parseDiffOptions } from "^/lib/query"; import { defaultPageCachingHeaders } from "^/lib/utils/headers"; +import parseParts from "^/lib/utils/parseParts"; import specsToDiff from "^/lib/utils/specsToDiff"; -import splitParts from "^/lib/utils/splitParts"; -import validateSpecs from "^/lib/utils/validateSpecs"; export const maxDuration = 60; @@ -21,19 +23,33 @@ export interface DiffApiContext { export async function GET(req: NextRequest, { params }: DiffApiContext) { const { parts } = await params; const { searchParams } = new URL(req.url); - const options = Object.fromEntries(searchParams); - const specsOrVersions = splitParts(parts); + const specsOrVersions = parseParts(parts); - if (!validateSpecs(specsOrVersions)) { + if (specsOrVersions == null) { return NextResponse.json("Invalid package spec", { status: 400 }); } - const { redirect: red, canonicalSpecs } = - await destination(specsOrVersions); + const parsedOptions = parseDiffOptions(fromSearchParams(searchParams)); + + if (!parsedOptions.ok) { + return NextResponse.json(parsedOptions.message, { status: 400 }); + } + + let target: Destination; + try { + target = await destination(specsOrVersions); + } catch (e) { + if (e instanceof SpecNotFoundError) { + return NextResponse.json(e.message, { status: 404 }); + } + throw e; + } + + const { redirect: red, canonicalSpecs } = target; if (red === false) { - const result = await npmDiff(canonicalSpecs, parseQuery(options)); + const result = await npmDiff(canonicalSpecs, parsedOptions.options); if (result.ok) { return new NextResponse(result.diff, { @@ -47,8 +63,8 @@ export async function GET(req: NextRequest, { params }: DiffApiContext) { } else { const newUrl = new URL(`/api/${specsToDiff(canonicalSpecs)}`, req.url); - Array.from(searchParams).forEach(([key, value]) => { - newUrl.searchParams.set(key, value); + searchParams.forEach((value, key) => { + newUrl.searchParams.append(key, value); }); return NextResponse.redirect( diff --git a/src/app/page.client.tsx b/src/app/page.client.tsx index 596e0d39..a0c62231 100644 --- a/src/app/page.client.tsx +++ b/src/app/page.client.tsx @@ -6,6 +6,7 @@ import { useBoolean } from "react-use"; import Stack from "^/components/ui/Stack"; import { type AutocompleteSuggestion } from "^/lib/autocomplete"; import { DEFAULT_DIFF_FILES_GLOB } from "^/lib/default-diff-files"; +import { toSearchString } from "^/lib/query/searchParams"; import ExamplesList from "./_page/ExamplesList"; import MainForm from "./_page/MainForm/MainForm"; import OptionsForm from "./_page/OptionsForm"; @@ -47,11 +48,7 @@ const IndexPageClient: FunctionComponent = ({ const goToDiff = (a: string | undefined, b: string | undefined): void => { setLoading(true); - router.push( - `/${a}...${b}?${Object.entries(query) - .map(([k, v]) => `${k}=${v}`) - .join("&")}`, - ); + router.push(`/${a}...${b}${toSearchString(query)}`); }; return ( diff --git a/src/lib/destination/SpecNotFoundError.ts b/src/lib/destination/SpecNotFoundError.ts new file mode 100644 index 00000000..c06b2873 --- /dev/null +++ b/src/lib/destination/SpecNotFoundError.ts @@ -0,0 +1,6 @@ +export default class SpecNotFoundError extends Error { + constructor(readonly spec: string) { + super(`Could not find "${spec}"`); + this.name = "SpecNotFoundError"; + } +} diff --git a/src/lib/destination/canonicalSpec.test.ts b/src/lib/destination/canonicalSpec.test.ts index a5891d15..f3728e6a 100644 --- a/src/lib/destination/canonicalSpec.test.ts +++ b/src/lib/destination/canonicalSpec.test.ts @@ -1,6 +1,7 @@ import { manifest, resolve } from "pacote"; import type pacote from "pacote"; import canonicalSpec from "./canonicalSpec"; +import SpecNotFoundError from "./SpecNotFoundError"; // Increase the timeout for long running tests jest.setTimeout(15_000); @@ -199,4 +200,12 @@ describe("canonicalSpec", () => { expect(() => t("file:example")).rejects.toThrow(expectedError), ]); }); + + it("Throws SpecNotFoundError for a package missing from the registry", () => + expect(() => + t("npm-diff-app-does-not-exist-9f3a2c@latest"), + ).rejects.toThrow(SpecNotFoundError)); + + it("Throws SpecNotFoundError for a range with no matching version", () => + expect(() => t("chalk@^999.0.0")).rejects.toThrow(SpecNotFoundError)); }); diff --git a/src/lib/destination/canonicalSpec.ts b/src/lib/destination/canonicalSpec.ts index 60756164..07f5d590 100644 --- a/src/lib/destination/canonicalSpec.ts +++ b/src/lib/destination/canonicalSpec.ts @@ -1,6 +1,8 @@ import { cacheLife } from "next/cache"; import npa, { type AliasResult } from "npm-package-arg"; import { manifest, resolve } from "pacote"; +import isRegistryNotFoundError from "^/lib/utils/isRegistryNotFoundError"; +import SpecNotFoundError from "./SpecNotFoundError"; const hashFinder = /(?:\#.*)?$/; @@ -58,14 +60,34 @@ async function handleNpaResult(result: npa.Result): Promise { * - https://github.com/npm/npm-package-arg#result-object * - https://docs.npmjs.com/cli/v7/commands/npm-install */ -async function canonicalSpec(spec: string): Promise { +async function cachedCanonicalSpec(spec: string): Promise { "use cache"; - cacheLife("hours"); + try { + const canonical = await handleNpaResult(npa(spec)); + + cacheLife("hours"); + + return canonical; + } catch (e) { + if (isRegistryNotFoundError(e)) { + cacheLife("minutes"); + + return null; + } + + throw e; + } +} - const result = npa(spec); +async function canonicalSpec(spec: string): Promise { + const canonical = await cachedCanonicalSpec(spec); + + if (canonical == null) { + throw new SpecNotFoundError(spec); + } - return handleNpaResult(result); + return canonical; } export default canonicalSpec; diff --git a/src/lib/destination/index.ts b/src/lib/destination/index.ts index b055d745..a8763185 100644 --- a/src/lib/destination/index.ts +++ b/src/lib/destination/index.ts @@ -1,5 +1,6 @@ import arrayEquals from "^/lib/utils/arrayEquals"; import canonicalSpec from "./canonicalSpec"; +import SpecNotFoundError from "./SpecNotFoundError"; import versionsToSpecs from "./versionsToSpecs"; export type Redirect = false | "temporary" | "permanent"; @@ -52,3 +53,4 @@ async function destination( } export default destination; +export { SpecNotFoundError }; diff --git a/src/lib/query/index.ts b/src/lib/query/index.ts index c17aa00f..c74f873d 100644 --- a/src/lib/query/index.ts +++ b/src/lib/query/index.ts @@ -1,5 +1,6 @@ -import parseQuery from "./parseQuery"; +import parseDiffOptions from "./parseDiffOptions"; import type QueryParams from "./QueryParams"; +import { fromSearchParams, toSearchString } from "./searchParams"; export type { QueryParams }; -export { parseQuery }; +export { fromSearchParams, parseDiffOptions, toSearchString }; diff --git a/src/lib/query/parseDiffOptions.test.ts b/src/lib/query/parseDiffOptions.test.ts new file mode 100644 index 00000000..ae6d500c --- /dev/null +++ b/src/lib/query/parseDiffOptions.test.ts @@ -0,0 +1,108 @@ +import { DEFAULT_DIFF_FILES_GLOB } from "^/lib/default-diff-files"; +import parseDiffOptions from "./parseDiffOptions"; + +describe("parseDiffOptions", () => { + it("accepts valid options", () => { + expect( + parseDiffOptions({ + diffFiles: ["{lib,dist}/**", "*.md"], + diffUnified: "3", + diffSrcPrefix: "a/", + diffDstPrefix: "b/", + }), + ).toStrictEqual({ + ok: true, + options: { + diffFiles: ["{lib,dist}/**", "*.md"], + diffUnified: 3, + diffSrcPrefix: "a/", + diffDstPrefix: "b/", + }, + }); + }); + + it("accepts empty options", () => { + expect(parseDiffOptions({})).toStrictEqual({ ok: true, options: {} }); + }); + + it("accepts the default diffFiles glob", () => { + expect( + parseDiffOptions({ diffFiles: DEFAULT_DIFF_FILES_GLOB }).ok, + ).toBe(true); + }); + + describe("diffFiles", () => { + it("rejects an exponential brace pattern quickly", () => { + const start = Date.now(); + const result = parseDiffOptions({ + diffFiles: "{a,b}".repeat(17), + }); + + expect(result.ok).toBe(false); + expect(Date.now() - start).toBeLessThan(500); + }); + + it("rejects a large numeric brace range", () => { + expect(parseDiffOptions({ diffFiles: "{1..100000}" }).ok).toBe( + false, + ); + }); + + it("accepts a pattern expanding to exactly 64 alternatives", () => { + expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(6) }).ok).toBe( + true, + ); + }); + + it("rejects a pattern expanding to 128 alternatives", () => { + expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(7) }).ok).toBe( + false, + ); + }); + + it("accepts 10 patterns and rejects 11", () => { + expect( + parseDiffOptions({ diffFiles: Array(10).fill("*.js") }).ok, + ).toBe(true); + expect( + parseDiffOptions({ diffFiles: Array(11).fill("*.js") }).ok, + ).toBe(false); + }); + + it("accepts 256 character patterns and rejects longer", () => { + expect(parseDiffOptions({ diffFiles: "a".repeat(256) }).ok).toBe( + true, + ); + expect(parseDiffOptions({ diffFiles: "a".repeat(257) }).ok).toBe( + false, + ); + }); + }); + + describe("diffUnified", () => { + it.each(["0", "3", "100"])("accepts %p", (value) => { + expect(parseDiffOptions({ diffUnified: value })).toStrictEqual({ + ok: true, + options: { diffUnified: Number(value) }, + }); + }); + + it.each(["-5", "101", "abc", "1.5", "3abc"])("rejects %p", (value) => { + expect(parseDiffOptions({ diffUnified: value }).ok).toBe(false); + }); + }); + + describe("prefixes", () => { + it("accepts 64 characters and rejects longer", () => { + expect(parseDiffOptions({ diffSrcPrefix: "a".repeat(64) }).ok).toBe( + true, + ); + expect(parseDiffOptions({ diffSrcPrefix: "a".repeat(65) }).ok).toBe( + false, + ); + expect(parseDiffOptions({ diffDstPrefix: "a".repeat(65) }).ok).toBe( + false, + ); + }); + }); +}); diff --git a/src/lib/query/parseDiffOptions.ts b/src/lib/query/parseDiffOptions.ts new file mode 100644 index 00000000..b3972197 --- /dev/null +++ b/src/lib/query/parseDiffOptions.ts @@ -0,0 +1,87 @@ +import { braceExpand } from "minimatch"; +import { type NpmDiffOptions } from "^/lib/npmDiff"; +import parseQuery from "./parseQuery"; +import type QueryParams from "./QueryParams"; + +export const MAX_DIFF_FILES = 10; +export const MAX_DIFF_FILES_PATTERN_LENGTH = 256; +export const MAX_DIFF_FILES_BRACE_EXPANSION = 64; +export const MAX_DIFF_UNIFIED = 100; +export const MAX_DIFF_PREFIX_LENGTH = 64; + +export type ParseDiffOptionsResult = + { ok: true; options: NpmDiffOptions } | { ok: false; message: string }; + +function braceExpansionExceedsLimit(pattern: string): boolean { + return ( + braceExpand(pattern, { + braceExpandMax: MAX_DIFF_FILES_BRACE_EXPANSION + 1, + }).length > MAX_DIFF_FILES_BRACE_EXPANSION + ); +} + +function validateDiffFiles(diffFiles: string[]): string | undefined { + if (diffFiles.length > MAX_DIFF_FILES) { + return `Too many diffFiles patterns, at most ${MAX_DIFF_FILES} are allowed.`; + } + + for (const pattern of diffFiles) { + if (pattern.length > MAX_DIFF_FILES_PATTERN_LENGTH) { + return `diffFiles patterns can be at most ${MAX_DIFF_FILES_PATTERN_LENGTH} characters long.`; + } + if (braceExpansionExceedsLimit(pattern)) { + return `diffFiles patterns can expand to at most ${MAX_DIFF_FILES_BRACE_EXPANSION} alternatives.`; + } + } + + return undefined; +} + +function validatePrefix( + name: string, + prefix: string | undefined, +): string | undefined { + if (prefix != null && prefix.length > MAX_DIFF_PREFIX_LENGTH) { + return `${name} can be at most ${MAX_DIFF_PREFIX_LENGTH} characters long.`; + } + return undefined; +} + +function validateOptions({ + diffFiles, + diffUnified, + diffSrcPrefix, + diffDstPrefix, +}: NpmDiffOptions): string | undefined { + if (diffFiles != null) { + const diffFilesError = validateDiffFiles(diffFiles); + if (diffFilesError) { + return diffFilesError; + } + } + + if ( + diffUnified != null && + !( + Number.isInteger(diffUnified) && + diffUnified >= 0 && + diffUnified <= MAX_DIFF_UNIFIED + ) + ) { + return `diffUnified must be an integer between 0 and ${MAX_DIFF_UNIFIED}.`; + } + + return ( + validatePrefix("diffSrcPrefix", diffSrcPrefix) ?? + validatePrefix("diffDstPrefix", diffDstPrefix) + ); +} + +export default function parseDiffOptions( + query: QueryParams, +): ParseDiffOptionsResult { + const options = parseQuery(query); + const message = validateOptions(options); + + return message == null ? { ok: true, options } : { ok: false, message }; +} diff --git a/src/lib/query/parseQuery/parseNumber.test.ts b/src/lib/query/parseQuery/parseNumber.test.ts index 76430a98..818ee425 100644 --- a/src/lib/query/parseQuery/parseNumber.test.ts +++ b/src/lib/query/parseQuery/parseNumber.test.ts @@ -8,7 +8,10 @@ describe("query/parseNumber", () => { it(`input is "1"`, () => expect(parseNumber("1")).toBe(1)); it(`input is "999"`, () => expect(parseNumber("999")).toBe(999)); - it(`input is "123XXX"`, () => expect(parseNumber("123XXX")).toBe(123)); + it(`input is "-5"`, () => expect(parseNumber("-5")).toBe(-5)); + it(`input is "123XXX"`, () => expect(parseNumber("123XXX")).toBeNaN()); + it(`input is "1.5"`, () => expect(parseNumber("1.5")).toBeNaN()); + it(`input is "abc"`, () => expect(parseNumber("abc")).toBeNaN()); it(`input is [1 ,2 ,3]`, () => expect(parseNumber(["1", "2", "3"])).toBe(3)); diff --git a/src/lib/query/parseQuery/parseNumber.ts b/src/lib/query/parseQuery/parseNumber.ts index bfcae9a8..7c5813f0 100644 --- a/src/lib/query/parseQuery/parseNumber.ts +++ b/src/lib/query/parseQuery/parseNumber.ts @@ -7,7 +7,7 @@ function parseNumber(str: undefined | string | string[]): undefined | number { return undefined; } - return parseInt(str); + return /^-?\d+$/.test(str) ? Number(str) : NaN; } export default parseNumber; diff --git a/src/lib/query/searchParams.test.ts b/src/lib/query/searchParams.test.ts new file mode 100644 index 00000000..e9f42d40 --- /dev/null +++ b/src/lib/query/searchParams.test.ts @@ -0,0 +1,47 @@ +import { + fromSearchParams, + toSearchParams, + toSearchString, +} from "./searchParams"; + +describe("toSearchString", () => { + it("returns an empty string for no params", () => { + expect(toSearchString({})).toBe(""); + expect(toSearchString({ diffFiles: undefined })).toBe(""); + }); + + it("encodes values", () => { + const search = toSearchString({ diffFiles: "a&diffNameOnly=true#x" }); + + expect(search).toBe("?diffFiles=a%26diffNameOnly%3Dtrue%23x"); + expect(Array.from(new URLSearchParams(search.slice(1)))).toStrictEqual([ + ["diffFiles", "a&diffNameOnly=true#x"], + ]); + }); + + it("appends each value of arrays", () => { + expect(toSearchString({ diffFiles: ["a,b", "c"], view: "split" })).toBe( + "?diffFiles=a%2Cb&diffFiles=c&view=split", + ); + }); +}); + +describe("fromSearchParams", () => { + it("keeps single values as strings and repeated values as arrays", () => { + expect( + fromSearchParams( + new URLSearchParams("diffFiles=a&diffFiles=b&diffUnified=3"), + ), + ).toStrictEqual({ diffFiles: ["a", "b"], diffUnified: "3" }); + }); + + it("round-trips with toSearchParams", () => { + const input = new URLSearchParams( + "diffFiles=a%26b&diffFiles=c&diffText=", + ); + + expect(toSearchParams(fromSearchParams(input)).toString()).toBe( + input.toString(), + ); + }); +}); diff --git a/src/lib/query/searchParams.ts b/src/lib/query/searchParams.ts new file mode 100644 index 00000000..5ef0829d --- /dev/null +++ b/src/lib/query/searchParams.ts @@ -0,0 +1,40 @@ +type SearchParamValue = string | string[] | undefined; + +export type SearchParamsRecord = { [K in keyof T]: SearchParamValue }; + +export function toSearchParams>( + query: T, +): URLSearchParams { + const searchParams = new URLSearchParams(); + + for (const [key, value] of Object.entries(query)) { + for (const item of Array.isArray(value) ? value : [value]) { + if (item != null) { + searchParams.append(key, item); + } + } + } + + return searchParams; +} + +export function toSearchString>( + query: T, +): string { + const search = toSearchParams(query).toString(); + + return search.length > 0 ? `?${search}` : ""; +} + +export function fromSearchParams( + searchParams: URLSearchParams, +): Record { + const query: Record = {}; + + for (const key of new Set(searchParams.keys())) { + const values = searchParams.getAll(key); + query[key] = values.length === 1 ? values[0] : values; + } + + return query; +} diff --git a/src/lib/utils/isRegistryNotFoundError.test.ts b/src/lib/utils/isRegistryNotFoundError.test.ts new file mode 100644 index 00000000..00dc4361 --- /dev/null +++ b/src/lib/utils/isRegistryNotFoundError.test.ts @@ -0,0 +1,18 @@ +import isRegistryNotFoundError from "./isRegistryNotFoundError"; + +describe("isRegistryNotFoundError", () => { + it.each(["E404", "ETARGET"])("is true for code %p", (code) => { + expect( + isRegistryNotFoundError(Object.assign(new Error("x"), { code })), + ).toBe(true); + }); + + it.each([ + new Error("x"), + Object.assign(new Error("x"), { code: "ECONNRESET" }), + null, + "E404", + ])("is false for %p", (error) => { + expect(isRegistryNotFoundError(error)).toBe(false); + }); +}); diff --git a/src/lib/utils/isRegistryNotFoundError.ts b/src/lib/utils/isRegistryNotFoundError.ts new file mode 100644 index 00000000..b9530f1d --- /dev/null +++ b/src/lib/utils/isRegistryNotFoundError.ts @@ -0,0 +1,7 @@ +export default function isRegistryNotFoundError(error: unknown): boolean { + if (typeof error !== "object" || error == null || !("code" in error)) { + return false; + } + + return error.code === "E404" || error.code === "ETARGET"; +} diff --git a/src/lib/utils/parseParts.test.ts b/src/lib/utils/parseParts.test.ts new file mode 100644 index 00000000..821e1b69 --- /dev/null +++ b/src/lib/utils/parseParts.test.ts @@ -0,0 +1,51 @@ +import parseParts from "./parseParts"; + +describe("parseParts", () => { + it("parses two specs", () => { + expect(parseParts("semver@7.6.0...semver@7.6.3")).toEqual([ + "semver@7.6.0", + "semver@7.6.3", + ]); + }); + + it("parses scoped specs from segments", () => { + expect(parseParts(["@types", "react@17...@types", "react@18"])).toEqual( + ["@types/react@17", "@types/react@18"], + ); + }); + + it("parses a spec and a version", () => { + expect(parseParts("semver@7.6.0...7.6.3")).toEqual([ + "semver@7.6.0", + "7.6.3", + ]); + }); + + it("parses a single package name", () => { + expect(parseParts("semver")).toEqual(["semver"]); + }); + + it("decodes parts when asked", () => { + expect( + parseParts("semver%407.6.0...semver%407.6.3", { decode: true }), + ).toEqual(["semver@7.6.0", "semver@7.6.3"]); + }); + + it.each([ + ["too many separators", "a...b...c"], + ["only a version", "1.0.0"], + ["only versions", "1.0.0...2.0.0"], + ["invalid package name", "NOT VALID@1"], + ["unsupported spec type", "file:../foo...semver@1"], + ])("returns null for %s", (_, parts) => { + expect(parseParts(parts)).toBeNull(); + }); + + it("returns null for malformed percent-encoding", () => { + expect(parseParts("%E0%A4%A", { decode: true })).toBeNull(); + }); + + it("returns null for too many segments", () => { + expect(parseParts(["a", "b", "c", "d"])).toBeNull(); + }); +}); diff --git a/src/lib/utils/parseParts.ts b/src/lib/utils/parseParts.ts new file mode 100644 index 00000000..8c65a31b --- /dev/null +++ b/src/lib/utils/parseParts.ts @@ -0,0 +1,23 @@ +import versionsToSpecs from "^/lib/destination/versionsToSpecs"; +import decodeParts from "./decodeParts"; +import splitParts from "./splitParts"; +import validateSpecs from "./validateSpecs"; + +export default function parseParts( + parts: string | string[] | undefined, + { decode = false }: { decode?: boolean } = {}, +): [string] | [string, string] | null { + try { + const specsOrVersions = splitParts(decode ? decodeParts(parts) : parts); + + if (!validateSpecs(specsOrVersions)) { + return null; + } + + versionsToSpecs(specsOrVersions); + + return specsOrVersions; + } catch { + return null; + } +} From 9573b204c82970c7e51c9b7522829519ea3a57a8 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:13:43 +0200 Subject: [PATCH 02/12] Simplify parseNumber to Number(str) Number() already returns NaN for trailing garbage, and parseDiffOptions enforces integer range, so the regex was redundant. Co-Authored-By: Claude Opus 5.5 --- src/lib/query/parseQuery/parseNumber.test.ts | 2 +- src/lib/query/parseQuery/parseNumber.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/query/parseQuery/parseNumber.test.ts b/src/lib/query/parseQuery/parseNumber.test.ts index 818ee425..c3182bfb 100644 --- a/src/lib/query/parseQuery/parseNumber.test.ts +++ b/src/lib/query/parseQuery/parseNumber.test.ts @@ -10,7 +10,7 @@ describe("query/parseNumber", () => { it(`input is "999"`, () => expect(parseNumber("999")).toBe(999)); it(`input is "-5"`, () => expect(parseNumber("-5")).toBe(-5)); it(`input is "123XXX"`, () => expect(parseNumber("123XXX")).toBeNaN()); - it(`input is "1.5"`, () => expect(parseNumber("1.5")).toBeNaN()); + it(`input is "1.5"`, () => expect(parseNumber("1.5")).toBe(1.5)); it(`input is "abc"`, () => expect(parseNumber("abc")).toBeNaN()); it(`input is [1 ,2 ,3]`, () => diff --git a/src/lib/query/parseQuery/parseNumber.ts b/src/lib/query/parseQuery/parseNumber.ts index 7c5813f0..0cdaaed7 100644 --- a/src/lib/query/parseQuery/parseNumber.ts +++ b/src/lib/query/parseQuery/parseNumber.ts @@ -7,7 +7,7 @@ function parseNumber(str: undefined | string | string[]): undefined | number { return undefined; } - return /^-?\d+$/.test(str) ? Number(str) : NaN; + return Number(str); } export default parseNumber; From 40646db7abd5eb647add6506d02fb551030ac53b Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:49:51 +0200 Subject: [PATCH 03/12] Revert "Simplify parseNumber to Number(str)" Number() accepts " ", "0x10", "1e2" and "0b11", which then pass the integer range check, so diffUnified=%20 was silently treated as 0. This reverts commit 9573b20. Co-Authored-By: Claude Opus 5.5 --- src/lib/query/parseQuery/parseNumber.test.ts | 2 +- src/lib/query/parseQuery/parseNumber.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/query/parseQuery/parseNumber.test.ts b/src/lib/query/parseQuery/parseNumber.test.ts index c3182bfb..818ee425 100644 --- a/src/lib/query/parseQuery/parseNumber.test.ts +++ b/src/lib/query/parseQuery/parseNumber.test.ts @@ -10,7 +10,7 @@ describe("query/parseNumber", () => { it(`input is "999"`, () => expect(parseNumber("999")).toBe(999)); it(`input is "-5"`, () => expect(parseNumber("-5")).toBe(-5)); it(`input is "123XXX"`, () => expect(parseNumber("123XXX")).toBeNaN()); - it(`input is "1.5"`, () => expect(parseNumber("1.5")).toBe(1.5)); + it(`input is "1.5"`, () => expect(parseNumber("1.5")).toBeNaN()); it(`input is "abc"`, () => expect(parseNumber("abc")).toBeNaN()); it(`input is [1 ,2 ,3]`, () => diff --git a/src/lib/query/parseQuery/parseNumber.ts b/src/lib/query/parseQuery/parseNumber.ts index 0cdaaed7..7c5813f0 100644 --- a/src/lib/query/parseQuery/parseNumber.ts +++ b/src/lib/query/parseQuery/parseNumber.ts @@ -7,7 +7,7 @@ function parseNumber(str: undefined | string | string[]): undefined | number { return undefined; } - return Number(str); + return /^-?\d+$/.test(str) ? Number(str) : NaN; } export default parseNumber; From 75b3a6d1e2ba0a2635b2eb68206db85c66131b3f Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:50:12 +0200 Subject: [PATCH 04/12] Keep catalog pages for package names that are also semver ranges parseParts ran versionsToSpecs on every request, which throws for a single spec like `x` or `1`, so those catalog pages 404ed. Co-Authored-By: Claude Opus 5.5 --- src/lib/utils/parseParts.test.ts | 9 ++++++++- src/lib/utils/parseParts.ts | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/utils/parseParts.test.ts b/src/lib/utils/parseParts.test.ts index 821e1b69..2912cccf 100644 --- a/src/lib/utils/parseParts.test.ts +++ b/src/lib/utils/parseParts.test.ts @@ -25,6 +25,13 @@ describe("parseParts", () => { expect(parseParts("semver")).toEqual(["semver"]); }); + it.each(["x", "1", "1.0.0"])( + "parses package name %p that is also a semver range", + (name) => { + expect(parseParts(name)).toEqual([name]); + }, + ); + it("decodes parts when asked", () => { expect( parseParts("semver%407.6.0...semver%407.6.3", { decode: true }), @@ -33,7 +40,7 @@ describe("parseParts", () => { it.each([ ["too many separators", "a...b...c"], - ["only a version", "1.0.0"], + ["only a version range", "^1.0.0"], ["only versions", "1.0.0...2.0.0"], ["invalid package name", "NOT VALID@1"], ["unsupported spec type", "file:../foo...semver@1"], diff --git a/src/lib/utils/parseParts.ts b/src/lib/utils/parseParts.ts index 8c65a31b..50b67932 100644 --- a/src/lib/utils/parseParts.ts +++ b/src/lib/utils/parseParts.ts @@ -1,5 +1,6 @@ import versionsToSpecs from "^/lib/destination/versionsToSpecs"; import decodeParts from "./decodeParts"; +import { isCatalogPage } from "./isCatalogPage"; import splitParts from "./splitParts"; import validateSpecs from "./validateSpecs"; @@ -14,7 +15,11 @@ export default function parseParts( return null; } - versionsToSpecs(specsOrVersions); + // Bare names like `x` or `1` are also semver ranges, which + // versionsToSpecs rejects, but they are valid catalog pages. + if (!isCatalogPage(specsOrVersions)) { + versionsToSpecs(specsOrVersions); + } return specsOrVersions; } catch { From e09d1e4d977e5f1df75cb41fe66f35e95dde2906 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:50:33 +0200 Subject: [PATCH 05/12] Reject control characters in diffSrcPrefix/diffDstPrefix libnpmdiff writes the prefixes verbatim into the ---/+++ header lines, so a newline could inject fake header lines into the diff. Co-Authored-By: Claude Opus 5.5 --- src/lib/query/parseDiffOptions.test.ts | 12 ++++++++++++ src/lib/query/parseDiffOptions.ts | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/src/lib/query/parseDiffOptions.test.ts b/src/lib/query/parseDiffOptions.test.ts index ae6d500c..b8952576 100644 --- a/src/lib/query/parseDiffOptions.test.ts +++ b/src/lib/query/parseDiffOptions.test.ts @@ -104,5 +104,17 @@ describe("parseDiffOptions", () => { false, ); }); + + it.each(["a\n+++ b/fake", "a\r", "a\t", "a\x7f"])( + "rejects control characters in %p", + (value) => { + expect(parseDiffOptions({ diffSrcPrefix: value }).ok).toBe( + false, + ); + expect(parseDiffOptions({ diffDstPrefix: value }).ok).toBe( + false, + ); + }, + ); }); }); diff --git a/src/lib/query/parseDiffOptions.ts b/src/lib/query/parseDiffOptions.ts index b3972197..182511d0 100644 --- a/src/lib/query/parseDiffOptions.ts +++ b/src/lib/query/parseDiffOptions.ts @@ -44,6 +44,11 @@ function validatePrefix( if (prefix != null && prefix.length > MAX_DIFF_PREFIX_LENGTH) { return `${name} can be at most ${MAX_DIFF_PREFIX_LENGTH} characters long.`; } + // Prefixes are written verbatim into the ---/+++ header lines + + if (prefix != null && /[\x00-\x1f\x7f]/.test(prefix)) { + return `${name} can not contain control characters.`; + } return undefined; } From a2b9230e8ea8a463eef04e283440a8f400a0005f Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:50:45 +0200 Subject: [PATCH 06/12] Build fromSearchParams with Object.fromEntries Assigning user-controlled keys into {} let a repeated __proto__ key replace the query object's prototype. Co-Authored-By: Claude Opus 5.5 --- src/lib/query/searchParams.test.ts | 11 +++++++++++ src/lib/query/searchParams.ts | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/lib/query/searchParams.test.ts b/src/lib/query/searchParams.test.ts index e9f42d40..c9e60291 100644 --- a/src/lib/query/searchParams.test.ts +++ b/src/lib/query/searchParams.test.ts @@ -44,4 +44,15 @@ describe("fromSearchParams", () => { input.toString(), ); }); + + it("treats __proto__ as a normal key", () => { + const query = fromSearchParams( + new URLSearchParams("__proto__=a&__proto__=b"), + ); + + expect(Object.getPrototypeOf(query)).toBe(Object.prototype); + expect( + Object.getOwnPropertyDescriptor(query, "__proto__")?.value, + ).toEqual(["a", "b"]); + }); }); diff --git a/src/lib/query/searchParams.ts b/src/lib/query/searchParams.ts index 5ef0829d..b7bf489a 100644 --- a/src/lib/query/searchParams.ts +++ b/src/lib/query/searchParams.ts @@ -29,12 +29,12 @@ export function toSearchString>( export function fromSearchParams( searchParams: URLSearchParams, ): Record { - const query: Record = {}; - - for (const key of new Set(searchParams.keys())) { - const values = searchParams.getAll(key); - query[key] = values.length === 1 ? values[0] : values; - } - - return query; + // Object.fromEntries defines own properties, so a `__proto__` key can't + // replace the prototype. + return Object.fromEntries( + [...new Set(searchParams.keys())].map((key) => { + const values = searchParams.getAll(key); + return [key, values.length === 1 ? values[0] : values]; + }), + ); } From ddc9952d9453151108e7e4505923c85096477163 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Thu, 1 Oct 2026 23:50:55 +0200 Subject: [PATCH 07/12] Use hasErrorCode instead of isRegistryNotFoundError Co-Authored-By: Claude Opus 5.5 --- src/lib/destination/canonicalSpec.ts | 4 ++-- src/lib/utils/isRegistryNotFoundError.test.ts | 18 ------------------ src/lib/utils/isRegistryNotFoundError.ts | 7 ------- 3 files changed, 2 insertions(+), 27 deletions(-) delete mode 100644 src/lib/utils/isRegistryNotFoundError.test.ts delete mode 100644 src/lib/utils/isRegistryNotFoundError.ts diff --git a/src/lib/destination/canonicalSpec.ts b/src/lib/destination/canonicalSpec.ts index 07f5d590..9e8244d3 100644 --- a/src/lib/destination/canonicalSpec.ts +++ b/src/lib/destination/canonicalSpec.ts @@ -1,7 +1,7 @@ import { cacheLife } from "next/cache"; import npa, { type AliasResult } from "npm-package-arg"; import { manifest, resolve } from "pacote"; -import isRegistryNotFoundError from "^/lib/utils/isRegistryNotFoundError"; +import { hasErrorCode } from "^/lib/utils/hasErrorCode"; import SpecNotFoundError from "./SpecNotFoundError"; const hashFinder = /(?:\#.*)?$/; @@ -70,7 +70,7 @@ async function cachedCanonicalSpec(spec: string): Promise { return canonical; } catch (e) { - if (isRegistryNotFoundError(e)) { + if (hasErrorCode(e, "E404") || hasErrorCode(e, "ETARGET")) { cacheLife("minutes"); return null; diff --git a/src/lib/utils/isRegistryNotFoundError.test.ts b/src/lib/utils/isRegistryNotFoundError.test.ts deleted file mode 100644 index 00dc4361..00000000 --- a/src/lib/utils/isRegistryNotFoundError.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import isRegistryNotFoundError from "./isRegistryNotFoundError"; - -describe("isRegistryNotFoundError", () => { - it.each(["E404", "ETARGET"])("is true for code %p", (code) => { - expect( - isRegistryNotFoundError(Object.assign(new Error("x"), { code })), - ).toBe(true); - }); - - it.each([ - new Error("x"), - Object.assign(new Error("x"), { code: "ECONNRESET" }), - null, - "E404", - ])("is false for %p", (error) => { - expect(isRegistryNotFoundError(error)).toBe(false); - }); -}); diff --git a/src/lib/utils/isRegistryNotFoundError.ts b/src/lib/utils/isRegistryNotFoundError.ts deleted file mode 100644 index b9530f1d..00000000 --- a/src/lib/utils/isRegistryNotFoundError.ts +++ /dev/null @@ -1,7 +0,0 @@ -export default function isRegistryNotFoundError(error: unknown): boolean { - if (typeof error !== "object" || error == null || !("code" in error)) { - return false; - } - - return error.code === "E404" || error.code === "ETARGET"; -} From 1bb4697162513529f943dece5ca556876431ef00 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Fri, 2 Oct 2026 00:01:09 +0200 Subject: [PATCH 08/12] Restrict diffFiles globs to patterns that match in bounded time minimatch compiles globs to backtracking regexes, and libnpmdiff matches every pattern against every file in both tarballs. Measured with the installed minimatch, `*(?|?)Z` takes 375 ms per file and `*?` repeated six times takes over 15 s against a 200 character file name. - Normalize patterns the way libnpmdiff does before checking, so braces escaped with a backslash are counted. - Reject extglobs, except the exact default glob the UI sends. - Allow at most two `*` runs outside of `**` segments. - Cap brace alternatives at 16 across all patterns instead of 64 per pattern. Allowed patterns take at most ~0.16 ms per match against hostile paths. Co-Authored-By: Claude Opus 5.5 --- src/lib/query/parseDiffOptions.test.ts | 53 ++++++++++++++++++++++++-- src/lib/query/parseDiffOptions.ts | 49 +++++++++++++++++++----- 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/src/lib/query/parseDiffOptions.test.ts b/src/lib/query/parseDiffOptions.test.ts index b8952576..afd26990 100644 --- a/src/lib/query/parseDiffOptions.test.ts +++ b/src/lib/query/parseDiffOptions.test.ts @@ -48,18 +48,63 @@ describe("parseDiffOptions", () => { ); }); - it("accepts a pattern expanding to exactly 64 alternatives", () => { - expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(6) }).ok).toBe( + it("accepts a pattern expanding to exactly 16 alternatives", () => { + expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(4) }).ok).toBe( true, ); }); - it("rejects a pattern expanding to 128 alternatives", () => { - expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(7) }).ok).toBe( + it("rejects a pattern expanding to 32 alternatives", () => { + expect(parseDiffOptions({ diffFiles: "{a,b}".repeat(5) }).ok).toBe( false, ); }); + it("counts alternatives across all patterns", () => { + expect( + parseDiffOptions({ + diffFiles: ["{a,b}".repeat(3), "{a,b}".repeat(3)], + }).ok, + ).toBe(true); + expect( + parseDiffOptions({ + diffFiles: ["{a,b}".repeat(3), "{a,b}".repeat(3), "c"], + }).ok, + ).toBe(false); + }); + + it("expands braces escaped with a backslash, like libnpmdiff", () => { + expect( + parseDiffOptions({ diffFiles: "\\{a,b}".repeat(17) }).ok, + ).toBe(false); + }); + + it.each([ + "*(?|?)Z", + "+(a|b)", + "!(*.map)", + "@(a|b)", + "?(a)", + "\\*(a|b)", + "{a,*(?|?)}", + ])("rejects extglob %p", (pattern) => { + expect(parseDiffOptions({ diffFiles: pattern }).ok).toBe(false); + }); + + it.each(["*.js", "**/*.d.ts", "*a*", "**/lib/**/*-*", "{*a*,*b*}"])( + "accepts %p", + (pattern) => { + expect(parseDiffOptions({ diffFiles: pattern }).ok).toBe(true); + }, + ); + + it.each(["*?*?*?*?Z", "*a*b*c", "a**b*c*"])( + "rejects %p with more than two *", + (pattern) => { + expect(parseDiffOptions({ diffFiles: pattern }).ok).toBe(false); + }, + ); + it("accepts 10 patterns and rejects 11", () => { expect( parseDiffOptions({ diffFiles: Array(10).fill("*.js") }).ok, diff --git a/src/lib/query/parseDiffOptions.ts b/src/lib/query/parseDiffOptions.ts index 182511d0..a5bc833c 100644 --- a/src/lib/query/parseDiffOptions.ts +++ b/src/lib/query/parseDiffOptions.ts @@ -1,23 +1,41 @@ import { braceExpand } from "minimatch"; +import { DEFAULT_DIFF_FILES_GLOB } from "^/lib/default-diff-files"; import { type NpmDiffOptions } from "^/lib/npmDiff"; import parseQuery from "./parseQuery"; import type QueryParams from "./QueryParams"; export const MAX_DIFF_FILES = 10; export const MAX_DIFF_FILES_PATTERN_LENGTH = 256; -export const MAX_DIFF_FILES_BRACE_EXPANSION = 64; +export const MAX_DIFF_FILES_BRACE_EXPANSION = 16; +export const MAX_DIFF_FILES_STARS = 2; export const MAX_DIFF_UNIFIED = 100; export const MAX_DIFF_PREFIX_LENGTH = 64; export type ParseDiffOptionsResult = { ok: true; options: NpmDiffOptions } | { ok: false; message: string }; -function braceExpansionExceedsLimit(pattern: string): boolean { - return ( - braceExpand(pattern, { - braceExpandMax: MAX_DIFF_FILES_BRACE_EXPANSION + 1, - }).length > MAX_DIFF_FILES_BRACE_EXPANSION - ); +// Same normalization libnpmdiff applies before matching, so `\{a,b}` +// can't slip past the checks as a literal brace. +function normalizeMatch(pattern: string): string { + return pattern.replace(/\\+/g, "/").replace(/^\.\/|^\./, ""); +} + +/** + * minimatch compiles globs to backtracking regexes. Extglobs and more than two + * `*` in a pattern can take seconds to match a single long file name, and + * libnpmdiff matches every pattern against every file in both tarballs. + * `**` path segments are not counted, they match whole segments. + */ +function isSlowGlob(pattern: string): boolean { + if (/[!@?*+]\(/.test(pattern)) { + return true; + } + const stars = pattern + .split("/") + .filter((segment) => segment !== "**") + .join("/") + .match(/\*+/g); + return (stars?.length ?? 0) > MAX_DIFF_FILES_STARS; } function validateDiffFiles(diffFiles: string[]): string | undefined { @@ -25,12 +43,25 @@ function validateDiffFiles(diffFiles: string[]): string | undefined { return `Too many diffFiles patterns, at most ${MAX_DIFF_FILES} are allowed.`; } + let alternatives = 0; for (const pattern of diffFiles) { if (pattern.length > MAX_DIFF_FILES_PATTERN_LENGTH) { return `diffFiles patterns can be at most ${MAX_DIFF_FILES_PATTERN_LENGTH} characters long.`; } - if (braceExpansionExceedsLimit(pattern)) { - return `diffFiles patterns can expand to at most ${MAX_DIFF_FILES_BRACE_EXPANSION} alternatives.`; + if (pattern === DEFAULT_DIFF_FILES_GLOB) { + alternatives++; + continue; + } + + const expanded = braceExpand(normalizeMatch(pattern), { + braceExpandMax: MAX_DIFF_FILES_BRACE_EXPANSION + 1, + }); + alternatives += expanded.length; + if (alternatives > MAX_DIFF_FILES_BRACE_EXPANSION) { + return `diffFiles patterns can expand to at most ${MAX_DIFF_FILES_BRACE_EXPANSION} alternatives in total.`; + } + if (expanded.some(isSlowGlob)) { + return `diffFiles patterns can not use extglobs like !(...) and can have at most ${MAX_DIFF_FILES_STARS} * outside of ** segments.`; } } From 0b27af3911f5e7c0b64c8d971d8be5f79df3bc0a Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Fri, 2 Oct 2026 00:07:02 +0200 Subject: [PATCH 09/12] Inline withViewType into ViewTypeButton Co-Authored-By: Claude Opus 5.5 --- .../_page/NpmDiff/ViewTypeSwitch.tsx | 60 +++++++++---------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx b/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx index 24b0d018..139db229 100644 --- a/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx +++ b/src/app/[...parts]/_page/NpmDiff/ViewTypeSwitch.tsx @@ -13,15 +13,6 @@ import { cx } from "^/lib/cva"; import useViewType from "^/lib/utils/useViewType"; import { DIFF_TYPE_PARAM_NAME } from "../paramNames"; -function withViewType( - searchParams: ReadonlyURLSearchParams | null, - viewType: ViewType, -): URLSearchParams { - const params = new URLSearchParams(searchParams?.toString()); - params.set(DIFF_TYPE_PARAM_NAME, viewType); - return params; -} - export interface ViewTypeButtonProps extends ButtonProps { currentViewType: ViewType; pathname: string | null; @@ -44,30 +35,35 @@ const ViewTypeButton = forwardRef< ...props }, ref, - ) => ( - - ), + + {children} + + + ); + }, ); ViewTypeButton.displayName = "ViewTypeButton"; From 73ae9fa34e83dc8a99627046398ce3eb44b14139 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Fri, 2 Oct 2026 00:07:18 +0200 Subject: [PATCH 10/12] Fold toSearchParams into toSearchString Co-Authored-By: Claude Opus 5.5 --- src/lib/query/searchParams.test.ts | 12 +++--------- src/lib/query/searchParams.ts | 14 ++++---------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/lib/query/searchParams.test.ts b/src/lib/query/searchParams.test.ts index c9e60291..1a4ad7d0 100644 --- a/src/lib/query/searchParams.test.ts +++ b/src/lib/query/searchParams.test.ts @@ -1,8 +1,4 @@ -import { - fromSearchParams, - toSearchParams, - toSearchString, -} from "./searchParams"; +import { fromSearchParams, toSearchString } from "./searchParams"; describe("toSearchString", () => { it("returns an empty string for no params", () => { @@ -35,14 +31,12 @@ describe("fromSearchParams", () => { ).toStrictEqual({ diffFiles: ["a", "b"], diffUnified: "3" }); }); - it("round-trips with toSearchParams", () => { + it("round-trips with toSearchString", () => { const input = new URLSearchParams( "diffFiles=a%26b&diffFiles=c&diffText=", ); - expect(toSearchParams(fromSearchParams(input)).toString()).toBe( - input.toString(), - ); + expect(toSearchString(fromSearchParams(input))).toBe(`?${input}`); }); it("treats __proto__ as a normal key", () => { diff --git a/src/lib/query/searchParams.ts b/src/lib/query/searchParams.ts index b7bf489a..745367d6 100644 --- a/src/lib/query/searchParams.ts +++ b/src/lib/query/searchParams.ts @@ -1,10 +1,10 @@ type SearchParamValue = string | string[] | undefined; -export type SearchParamsRecord = { [K in keyof T]: SearchParamValue }; +type SearchParamsRecord = { [K in keyof T]: SearchParamValue }; -export function toSearchParams>( +export function toSearchString>( query: T, -): URLSearchParams { +): string { const searchParams = new URLSearchParams(); for (const [key, value] of Object.entries(query)) { @@ -15,13 +15,7 @@ export function toSearchParams>( } } - return searchParams; -} - -export function toSearchString>( - query: T, -): string { - const search = toSearchParams(query).toString(); + const search = searchParams.toString(); return search.length > 0 ? `?${search}` : ""; } From 97b579261524199d1a2bdd3747446718e51e2c3a Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Fri, 2 Oct 2026 00:07:40 +0200 Subject: [PATCH 11/12] Trim parseDiffOptions: unexport unused names, early return in validatePrefix Co-Authored-By: Claude Opus 5.5 --- src/lib/query/parseDiffOptions.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/lib/query/parseDiffOptions.ts b/src/lib/query/parseDiffOptions.ts index a5bc833c..94b84e12 100644 --- a/src/lib/query/parseDiffOptions.ts +++ b/src/lib/query/parseDiffOptions.ts @@ -4,15 +4,12 @@ import { type NpmDiffOptions } from "^/lib/npmDiff"; import parseQuery from "./parseQuery"; import type QueryParams from "./QueryParams"; -export const MAX_DIFF_FILES = 10; -export const MAX_DIFF_FILES_PATTERN_LENGTH = 256; -export const MAX_DIFF_FILES_BRACE_EXPANSION = 16; -export const MAX_DIFF_FILES_STARS = 2; -export const MAX_DIFF_UNIFIED = 100; -export const MAX_DIFF_PREFIX_LENGTH = 64; - -export type ParseDiffOptionsResult = - { ok: true; options: NpmDiffOptions } | { ok: false; message: string }; +const MAX_DIFF_FILES = 10; +const MAX_DIFF_FILES_PATTERN_LENGTH = 256; +const MAX_DIFF_FILES_BRACE_EXPANSION = 16; +const MAX_DIFF_FILES_STARS = 2; +const MAX_DIFF_UNIFIED = 100; +const MAX_DIFF_PREFIX_LENGTH = 64; // Same normalization libnpmdiff applies before matching, so `\{a,b}` // can't slip past the checks as a literal brace. @@ -72,12 +69,14 @@ function validatePrefix( name: string, prefix: string | undefined, ): string | undefined { - if (prefix != null && prefix.length > MAX_DIFF_PREFIX_LENGTH) { + if (prefix == null) { + return undefined; + } + if (prefix.length > MAX_DIFF_PREFIX_LENGTH) { return `${name} can be at most ${MAX_DIFF_PREFIX_LENGTH} characters long.`; } // Prefixes are written verbatim into the ---/+++ header lines - - if (prefix != null && /[\x00-\x1f\x7f]/.test(prefix)) { + if (/[\x00-\x1f\x7f]/.test(prefix)) { return `${name} can not contain control characters.`; } return undefined; @@ -115,7 +114,7 @@ function validateOptions({ export default function parseDiffOptions( query: QueryParams, -): ParseDiffOptionsResult { +): { ok: true; options: NpmDiffOptions } | { ok: false; message: string } { const options = parseQuery(query); const message = validateOptions(options); From 1af2ca2bb78063953cdb64cec279c2698d7b1dd5 Mon Sep 17 00:00:00 2001 From: Oscar Busk Date: Fri, 2 Oct 2026 00:07:42 +0200 Subject: [PATCH 12/12] Drop unused spec field from SpecNotFoundError Co-Authored-By: Claude Opus 5.5 --- src/lib/destination/SpecNotFoundError.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/destination/SpecNotFoundError.ts b/src/lib/destination/SpecNotFoundError.ts index c06b2873..df55ab48 100644 --- a/src/lib/destination/SpecNotFoundError.ts +++ b/src/lib/destination/SpecNotFoundError.ts @@ -1,5 +1,5 @@ export default class SpecNotFoundError extends Error { - constructor(readonly spec: string) { + constructor(spec: string) { super(`Could not find "${spec}"`); this.name = "SpecNotFoundError"; }