diff --git a/.changeset/redos-sweep-174.md b/.changeset/redos-sweep-174.md new file mode 100644 index 00000000..c493751d --- /dev/null +++ b/.changeset/redos-sweep-174.md @@ -0,0 +1,15 @@ +--- +"@cloudhal/api": patch +"@cloudhal/core-ts": patch +"@cloudhal/schedules": patch +"@cloudhal/agent-runtime": patch +"@cloudhal/cli": patch +--- + +Second ReDoS sweep (#174). Input that made a regex backtrack is now handled in linear time; results are unchanged. + +- **Unauthenticated password-reset request.** `POST /api/password-reset/request` (and invite creation) checks the address length before anything else, and the address check no longer backtracks: `a@` followed by many dots used to cost seconds of CPU per request. Public token endpoints (`/api/password-reset/*`, invite acceptance) refuse a JSON body over 64 KiB with `413 payload_too_large`, whether or not it declares a `content-length`. +- **core-ts.** Redirect path normalising and matching (on every site request): `normalisePath`, `joinTarget`, and `ancestorPaths`, which no longer lists ancestors that no redirect can match: those longer than a `from` may be (1024 code points) or holding `//`. Directive lines in search text keep their braces on one line, so a directive whose `{…}` spans a lone `\r` is no longer read as one. Search text built at publish time: fenced code, links, images and tags. SVG upload checks: CSS comments, `url(…)` references (about cubic on `url(` followed by spaces) and width/height lengths (new `css-scan.ts`). +- **schedules.** The feed check's CDATA, element and ``/`` url scans, and `urlPath`. +- **agent-runtime.** SKILL.md frontmatter lines (a line of spaces was quadratic). +- **Studio** (in `@cloudhal/cli`): the markdown body editor's trailing-newline trims. diff --git a/AGENTS.md b/AGENTS.md index 04ec7fc0..eb10029d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,6 +56,7 @@ pnpm i18n:export / pnpm i18n:import [--locale xx] # translations as nested ICU 10. **UI strings** go through `useT()` with keys in `en.ts` (only English is edited by hand; every other `.ts` is generated by `pnpm i18n:import`, ADR-0034) and every language's `.ts` (or every file of a lazy namespace, read with `useT("")`; languages are registered in `apps/studio/src/i18n/locales.ts`); no hard-coded words in `apps/studio/src` or `packages/ui/src` — visible text, `aria-label`/`title`/`placeholder`/`alt`, toasts, banners, label tables and block defaults (blocks take every word as a prop, no English fallback; API errors through `apiErrorText`). Enforced by the `i18n`, `i18n-plural`, `i18n-concat` and `i18n-format` rules (and `rtl-logical` for physical CSS utilities, ADR-0033) of `pnpm design:lint` (`apps/studio/scripts/i18n-lint.ts`, run in `pnpm check` by `test/design-rules.test.ts` and on every agent edit); legitimate literals go in its `ALLOWED` list or carry an `i18n-ignore: ` comment. 11. Conventional commits, `git commit -s` (DCO). Changeset for published-package changes. 12. **Size ratchet** ([docs/engineering/code-structure.md](docs/engineering/code-structure.md)): files ≤ 800 lines (soft 400; `.astro` 500, tests 1500; i18n dictionaries are data and exempt — the Studio bundle budget guards them), functions ≤ 150 lines, depth ≤ 5, ≤ 4 positional params (use an options object beyond 3); one responsibility per module, dispatch tables instead of giant switches. `pnpm size:check` (in `pnpm check`) fails on a new violation or when a file in `scripts/size-baseline.json` grows — put new code in a new module next to it; after shrinking one, run `node scripts/size-check.ts --update`. +13. **Regexes over external input** (request paths and bodies, agent or editor markdown, uploads, fetched feeds) must be linear (#168, #174): no unanchored or end-anchored quantified runs (`/-+$/`, `/\s+$/`, `/\/+$/`), no lazy scans that rescan to the end when the closer is missing (`/[\s\S]*?<\/x>/`, `/<[^>]*>/`), no adjacent quantifiers over overlapping classes (`\s+[^:]+`, `\s*(.*)`). Use `trimChar`/`trimEndChar` (`@cloudhal/core-ts/names`), `trimEnd()`, `indexOf`/`lastIndexOf` scanners, and check lengths before patterns. A rewrite gets an equivalence test against the old regex and a 100k-character adversarial case. ## Versions (pinned via pnpm catalog / Cargo workspace) Node 24 · pnpm 12 · TypeScript 7.0 · Biome 2.6 · Vitest 5 · Turborepo 2.11 · Rust 2024 edition · json-schema-to-typescript 16.0.0 · cargo-typify 0.8.0 · ajv 8.20 · jsonschema 0.60 · better-sqlite3 13 · miniflare 4.20260730 (last stable 4.x; its sharp overridden to 0.35.5; see ADR-0012) · wrangler 4.158 · @astrojs/cloudflare 14.4 · Astro 7.4 · Tailwind 4.3 · marked 18 · Hono 4.13 · Better Auth 1.7.12 · Kumo 2.14 · React 19.3 · TanStack Router 1.170 / Query 5.104 · Vite 8.3. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6496455f..898352cf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,6 +40,7 @@ Commit the regenerated files; CI fails on drift. Add a valid and an invalid fixt - Add a changeset (`pnpm changeset`) when a published package changes behaviour. - A PR that changes what the renderer gives themes freezes the last released starter in the same PR: copy it under `packages/cli/test/fixtures/theme-v/` and add that version to `FROZEN` in `packages/cli/test/old-theme-compat.test.ts` (see `theme-v0.5.0/README.md` there), so the release that ships the change is already tested against the theme existing sites have. - Keep modules small and single-purpose: [`docs/engineering/code-structure.md`](docs/engineering/code-structure.md) sets file/function limits that `pnpm size:check` enforces with a shrink-only baseline. +- Regexes over input from outside (requests, agent or editor content, uploads, feeds) must not backtrack: prefer `indexOf` scanners and the linear trims in `@cloudhal/core-ts/names`, check lengths first, and test a rewrite against the old regex plus a 100k-character adversarial input (AGENTS.md rule 13). - CI must be green: lint, size check, typecheck, tests (TS + Rust), contract drift. ## Releasing diff --git a/apps/docs/src/worker/paths.ts b/apps/docs/src/worker/paths.ts index fa4e7c9b..03a51195 100644 --- a/apps/docs/src/worker/paths.ts +++ b/apps/docs/src/worker/paths.ts @@ -38,5 +38,9 @@ export function pageId(input: string, selfHost?: string): string | undefined { export const markdownPath = (id: string) => (id === "index" ? "/index.md" : `/${id}.md`); -/** A path that may be a page: no file extension in its last segment. */ -export const isPagePath = (pathname: string) => !/\.[^/]*$/.test(pathname); +/** + * A path that may be a page: no file extension in its last segment, that is no "." after the last + * "/". Two `lastIndexOf`s, not `/\.[^/]*$/`, which backtracks on a long run of dots (#174). + */ +export const isPagePath = (pathname: string) => + pathname.lastIndexOf(".") <= pathname.lastIndexOf("/"); diff --git a/apps/docs/test/paths.test.ts b/apps/docs/test/paths.test.ts new file mode 100644 index 00000000..74db59fe --- /dev/null +++ b/apps/docs/test/paths.test.ts @@ -0,0 +1,28 @@ +/** #174 item 3: `isPagePath` answers as `/\.[^/]*$/` did, in linear time. */ +import { describe, expect, it } from "vitest"; +import { isPagePath } from "../src/worker/paths.ts"; + +describe("isPagePath (#174)", () => { + it("answers as the regex did", () => { + const old = (p: string) => !/\.[^/]*$/.test(p); + let s = 174; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + for (let n = 0; n < 5000; n++) { + let p = ""; + for (let i = next() % 10; i > 0; i--) p += ["/", ".", "a", "md"][next() % 4]; + expect(isPagePath(p), p).toBe(old(p)); + } + expect(isPagePath("/guides/search")).toBe(true); + expect(isPagePath("/llms.txt")).toBe(false); + expect(isPagePath("/v0.6/guide")).toBe(true); + }); + + it("stays linear on a long run of dots", () => { + const start = performance.now(); + expect(isPagePath(`/${".".repeat(200_000)}/`)).toBe(true); + expect(performance.now() - start).toBeLessThan(1500); + }); +}); diff --git a/packages/agent-runtime/src/frontmatter-lines.ts b/packages/agent-runtime/src/frontmatter-lines.ts new file mode 100644 index 00000000..efb1caaa --- /dev/null +++ b/packages/agent-runtime/src/frontmatter-lines.ts @@ -0,0 +1,42 @@ +/** + * The two line shapes of SKILL.md frontmatter, matched in one linear pass each (#174). They give + * the groups `/^\s+([^:]+):\s*(.*)$/` and `/^([A-Za-z][\w-]*):\s*(.*)$/` gave, including where + * those regexes backtracked: the first one's `\s+` and `[^:]+` both take whitespace, which made a + * line of spaces quadratic, and each `\s*` overlapped `(.*)` before a lone `\r`. + */ + +const isSpace = (ch: string | undefined) => ch !== undefined && /\s/.test(ch); +/** What `.` does not match without the `s` flag. */ +const LINE_END = /[\n\r\u2028\u2029]/; + +/** After the `:` at `colon`: whitespace skipped; undefined when the rest holds a line end. */ +function valueAfter(raw: string, colon: number): string | undefined { + let i = colon + 1; + while (isSpace(raw[i])) i++; + const rest = raw.slice(i); + return LINE_END.test(rest) ? undefined : rest; +} + +/** An indented `key: value` line (a nested map entry): `[key, value]`, or undefined. */ +export function nestedLine(raw: string): [string, string] | undefined { + let lead = 0; + while (isSpace(raw[lead])) lead++; + if (lead === 0) return undefined; + const colon = raw.indexOf(":"); + if (colon < 0) return undefined; + // The key needs a character: when the indent runs up to the `:`, its last space is the key. + const start = lead < colon ? lead : lead >= 2 ? lead - 1 : -1; + if (start < 0) return undefined; + const value = valueAfter(raw, colon); + return value === undefined ? undefined : [raw.slice(start, colon), value]; +} + +/** A top-level `key: value` line, the key a letter then letters, digits, `_` or `-`. */ +export function topLine(raw: string): [string, string] | undefined { + if (!/^[A-Za-z]/.test(raw)) return undefined; + let j = 1; + while (j < raw.length && /[\w-]/.test(raw[j] as string)) j++; + if (raw[j] !== ":") return undefined; + const value = valueAfter(raw, j); + return value === undefined ? undefined : [raw.slice(0, j), value]; +} diff --git a/packages/agent-runtime/src/skills.ts b/packages/agent-runtime/src/skills.ts index a78b109f..623a7a48 100644 --- a/packages/agent-runtime/src/skills.ts +++ b/packages/agent-runtime/src/skills.ts @@ -1,6 +1,7 @@ import { readdir, readFile, stat } from "node:fs/promises"; import path from "node:path"; import { assertValid, type SkillManifest } from "@cloudhal/core-contracts"; +import { nestedLine, topLine } from "./frontmatter-lines.ts"; export interface LoadedSkill { manifest: SkillManifest; @@ -36,14 +37,14 @@ export function parseFrontmatter(markdown: string): { let currentMap: Record | undefined; for (const raw of (m[1] as string).split(/\r?\n/)) { if (!raw.trim() || raw.trim().startsWith("#")) continue; - const nested = raw.match(/^\s+([^:]+):\s*(.*)$/); + const nested = nestedLine(raw); if (nested && currentMap) { - currentMap[nested[1] as string] = unquote((nested[2] as string).trim()); + currentMap[nested[0]] = unquote(nested[1].trim()); continue; } - const top = raw.match(/^([A-Za-z][\w-]*):\s*(.*)$/); + const top = topLine(raw); if (!top) throw new SkillError(`invalid frontmatter line: ${raw}`); - const [, key, value] = top as unknown as [string, string, string]; + const [key, value] = top; if (value.trim() === "") { currentMap = {}; fm[key] = currentMap; diff --git a/packages/agent-runtime/test/frontmatter-lines.test.ts b/packages/agent-runtime/test/frontmatter-lines.test.ts new file mode 100644 index 00000000..bb751038 --- /dev/null +++ b/packages/agent-runtime/test/frontmatter-lines.test.ts @@ -0,0 +1,49 @@ +/** + * #174 item 5: the frontmatter line parsers give the regexes' groups (seeded random lines over the + * characters that matter) and stay linear on 100k-character lines that made them backtrack. + */ +import { describe, expect, it } from "vitest"; +import { nestedLine, topLine } from "../src/frontmatter-lines.ts"; +import { parseFrontmatter } from "../src/skills.ts"; + +function strings(alphabet: readonly string[], count: number, maxLength: number, seed = 174) { + let s = seed; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + const out: string[] = []; + for (let n = 0; n < count; n++) { + let v = ""; + const len = next() % (maxLength + 1); + for (let i = 0; i < len; i++) v += alphabet[next() % alphabet.length]; + out.push(v); + } + return out; +} + +const groups = (m: RegExpMatchArray | null) => (m ? [m[1], m[2]] : undefined); + +describe("frontmatter lines (#174 item 5)", () => { + it("match exactly what the regexes matched", () => { + const parts = [" ", " ", "\t", ":", "a", "B", "-", "_", "1", "\r", "\u2028", "'"]; + for (const raw of strings(parts, 10_000, 9)) { + expect(nestedLine(raw), JSON.stringify(raw)).toEqual( + groups(raw.match(/^\s+([^:]+):\s*(.*)$/)), + ); + expect(topLine(raw), JSON.stringify(raw)).toEqual( + groups(raw.match(/^([A-Za-z][\w-]*):\s*(.*)$/)), + ); + } + }); + + it("stays linear on lines that made the regexes backtrack", () => { + const start = performance.now(); + expect(nestedLine(" ".repeat(200_000))).toBeUndefined(); + expect(nestedLine(` key:${" ".repeat(200_000)}x\r`)).toBeUndefined(); + expect(topLine(`name:${" ".repeat(200_000)}x\r`)).toBeUndefined(); + const fm = `---\nmetadata:\n${" ".repeat(200_000)}\n---\nbody`; + expect(() => parseFrontmatter(fm)).not.toThrow(); + expect(performance.now() - start).toBeLessThan(1500); + }); +}); diff --git a/packages/api/src/email.ts b/packages/api/src/email.ts new file mode 100644 index 00000000..be2905bd --- /dev/null +++ b/packages/api/src/email.ts @@ -0,0 +1,21 @@ +/** + * The address check of the invite and password-reset routes (#174). It accepts exactly what + * `/^[^\s@]+@[^\s@]+\.[^\s@]+$/` accepted, in one linear pass, and checks the length first. That + * regex backtracked quadratically on `a@` followed by many dots (the domain part is ambiguous on + * "."), and it ran before the length cap on the unauthenticated `POST /api/password-reset/request`. + */ + +/** RFC 5321's limit on a forward path: longer is never an address. */ +export const EMAIL_MAX_LENGTH = 254; + +/** + * One `@` with something before it; after it, no `@` and a `.` with something on both sides; no + * whitespace anywhere; at most `EMAIL_MAX_LENGTH` characters. + */ +export function isEmailAddress(value: string): boolean { + if (value.length > EMAIL_MAX_LENGTH || /\s/.test(value)) return false; + const at = value.indexOf("@"); + if (at < 1 || value.indexOf("@", at + 1) !== -1) return false; + // `[^\s@]+\.[^\s@]+`: a dot that is neither the domain's first nor its last character. + return value.slice(at + 2, -1).includes("."); +} diff --git a/packages/api/src/routes/invites.ts b/packages/api/src/routes/invites.ts index 53a3618d..ae66b269 100644 --- a/packages/api/src/routes/invites.ts +++ b/packages/api/src/routes/invites.ts @@ -29,6 +29,7 @@ import type { Actor } from "@cloudhal/core-contracts"; import { type Context, Hono } from "hono"; import { actorIdForUser, type StaffService } from "../auth.ts"; import type { Env } from "../context.ts"; +import { isEmailAddress } from "../email.ts"; import { apiError, badRequest, notFound } from "../errors.ts"; import { MailError } from "../mail.ts"; import { inviteMail } from "../mail-messages.ts"; @@ -82,8 +83,6 @@ export function inviteView(i: InviteRow): InviteView { }; } -export const EMAIL = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - function gone() { return apiError( 410, @@ -210,7 +209,7 @@ export function inviteRoutes(options: { const { person, actor, staff } = me(c); const body = ((await c.req.json().catch(() => ({}))) ?? {}) as Record; const email = typeof body.email === "string" ? body.email.trim().toLowerCase() : ""; - if (!EMAIL.test(email) || email.length > 254) throw badRequest("a valid email is required"); + if (!isEmailAddress(email)) throw badRequest("a valid email is required"); const { role, sites } = await inviteTarget(c, person, body); const hours = body.expiresInHours === undefined ? inviteDefaultHours(role) : body.expiresInHours; diff --git a/packages/api/src/routes/password-reset.ts b/packages/api/src/routes/password-reset.ts index ea468b26..a7895bb5 100644 --- a/packages/api/src/routes/password-reset.ts +++ b/packages/api/src/routes/password-reset.ts @@ -18,6 +18,7 @@ import type { Actor } from "@cloudhal/core-contracts"; import { type Context, Hono } from "hono"; import { actorIdForUser, type StaffService } from "../auth.ts"; import type { Env } from "../context.ts"; +import { isEmailAddress } from "../email.ts"; import { apiError, badRequest, notFound } from "../errors.ts"; import { MailError, type MailSender } from "../mail.ts"; import { passwordChangedMail, resetMail } from "../mail-messages.ts"; @@ -30,7 +31,6 @@ import { RESET_TOKEN_PREFIX, studioOrigin, } from "../tokens.ts"; -import { EMAIL } from "./invites.ts"; import { auditEvent, RESET_LINK_MINUTES } from "./members.ts"; /** RFC-0011: reset mails one address can receive per hour, whoever asks. */ @@ -120,7 +120,7 @@ export function passwordResetRoutes(options: { await limitPublic(c, "reset-request"); const raw = (await jsonBody(c)).email; const email = typeof raw === "string" ? raw.trim().toLowerCase() : ""; - if (!EMAIL.test(email) || email.length > 254) throw badRequest("a valid email is required"); + if (!isEmailAddress(email)) throw badRequest("a valid email is required"); c.get("defer")( mailResetLink({ staff, diff --git a/packages/api/src/tokens.ts b/packages/api/src/tokens.ts index 0f325ce7..2eaca5cb 100644 --- a/packages/api/src/tokens.ts +++ b/packages/api/src/tokens.ts @@ -37,7 +37,50 @@ export function hashToken(token: string): Promise { return sha256Hex(token); } -/** Public token endpoints take JSON bodies only (a cross-site form post needs a preflight). */ +/** + * The largest body a public token endpoint reads (#174): an email, a token, a password and a name + * fit many times over. Anything larger is refused before it is parsed or checked. + */ +export const JSON_BODY_MAX_BYTES = 64 * 1024; + +function tooLarge() { + return apiError( + 413, + "payload_too_large", + `the body must be at most ${JSON_BODY_MAX_BYTES} bytes`, + ); +} + +/** The body as text, or a 413 once more than `max` bytes arrive (a chunked body has no length). */ +async function cappedText(request: Request, max: number): Promise { + if (Number(request.headers.get("content-length") ?? 0) > max) throw tooLarge(); + const reader = request.body?.getReader(); + if (!reader) return ""; + const chunks: Uint8Array[] = []; + let size = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > max) { + await reader.cancel().catch(() => undefined); + throw tooLarge(); + } + chunks.push(value); + } + const all = new Uint8Array(size); + let at = 0; + for (const chunk of chunks) { + all.set(chunk, at); + at += chunk.byteLength; + } + return new TextDecoder().decode(all); +} + +/** + * Public token endpoints take JSON bodies only (a cross-site form post needs a preflight), of at + * most `JSON_BODY_MAX_BYTES`. + */ export async function jsonBody(c: Context): Promise> { if (!(c.req.header("content-type") ?? "").toLowerCase().startsWith("application/json")) throw apiError( @@ -45,7 +88,13 @@ export async function jsonBody(c: Context): Promise "unsupported_media_type", "send a JSON body (content-type: application/json)", ); - const body = (await c.req.json().catch(() => undefined)) as unknown; + const text = await cappedText(c.req.raw, JSON_BODY_MAX_BYTES); + let body: unknown; + try { + body = JSON.parse(text); + } catch { + body = undefined; + } if (!body || typeof body !== "object" || Array.isArray(body)) throw apiError(400, "bad_request", "the body must be a JSON object"); return body as Record; diff --git a/packages/api/test/public-input.test.ts b/packages/api/test/public-input.test.ts new file mode 100644 index 00000000..8fa55cfe --- /dev/null +++ b/packages/api/test/public-input.test.ts @@ -0,0 +1,141 @@ +/** + * #174 item 1: the unauthenticated reset request checks the address in linear time with the + * length first, and public token endpoints refuse bodies above `JSON_BODY_MAX_BYTES` with 413. + */ +import { SiteGraphStore } from "@cloudhal/core-ts"; +import { MemoryBlobStore } from "@cloudhal/storage"; +import { BetterSqlite3Driver } from "@cloudhal/storage/sqlite"; +import Database from "better-sqlite3"; +import { describe, expect, it } from "vitest"; +import { createApi } from "../src/app.ts"; +import { createAuth } from "../src/auth.ts"; +import { EMAIL_MAX_LENGTH, isEmailAddress } from "../src/email.ts"; +import { JSON_BODY_MAX_BYTES } from "../src/tokens.ts"; + +/** What the routes checked before #174. */ +const OLD_EMAIL = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; +const oldCheck = (s: string) => OLD_EMAIL.test(s) && s.length <= EMAIL_MAX_LENGTH; + +describe("isEmailAddress (#174)", () => { + it("accepts exactly what the old regex and length cap accepted", () => { + const samples = [ + "a@b.c", + "owner@example.com", + "first.last+tag@sub.example.co.uk", + "a@b..c", + "a@.b.c", + "a@..", + "a@...", + "a@b.", + "a@.b", + "a@bc", + "@b.c", + "a@@b.c", + "a@b@c.d", + "a b@c.d", + "a@b.c ", + "a@b.\tc", + "", + "@", + "a@", + `${"a".repeat(250)}@b.c`, + `${"a".repeat(251)}@b.c`, + ]; + for (const s of samples) expect(isEmailAddress(s), JSON.stringify(s)).toBe(oldCheck(s)); + // Every string of up to 6 characters over the characters that matter. + const alphabet = ["a", ".", "@", " "]; + let all = [""]; + for (let len = 1; len <= 6; len++) { + all = all.flatMap((s) => alphabet.map((c) => s + c)); + for (const s of all) expect(isEmailAddress(s), JSON.stringify(s)).toBe(oldCheck(s)); + } + }); + + it("answers in linear time on input that made the old regex backtrack", () => { + // The old regex took about a second on 40k dots and grew with the square of the length. + const start = performance.now(); + expect(isEmailAddress(`a@${".".repeat(200_000)}@`)).toBe(false); + expect(isEmailAddress(`a@b${".".repeat(200_000)}`)).toBe(false); + expect(isEmailAddress(`a@${".".repeat(200)}@`)).toBe(false); + expect(performance.now() - start).toBeLessThan(1500); + }); +}); + +describe("public token endpoints (#174)", () => { + const ORIGIN = "http://public.test"; + const setup = async () => { + const database = new Database(":memory:"); + const sql = new BetterSqlite3Driver(database); + const store = new SiteGraphStore({ sql, blobs: new MemoryBlobStore() }); + await store.init(); + const auth = createAuth({ + database, + sql, + baseURL: ORIGIN, + secret: "public-input-test-secret-0123456789abcde", + signUp: "first-user", + }); + return createApi({ + store, + resolvePrincipal: auth.resolvePrincipal, + authHandler: auth.handler, + staff: auth.staff, + signUp: auth.signUp, + mail: { id: "test", send: async () => undefined }, + defer: () => undefined, + }); + }; + const post = (app: Awaited>, path: string, body: string) => + app.request(`${ORIGIN}${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body, + }); + + it("refuses an adversarial address quickly, before any regex sees it", async () => { + const app = await setup(); + // A valid request first, so the timed ones measure the check and not the app's first call. + const ok = await post(app, "/api/password-reset/request", JSON.stringify({ email: "x@y.z" })); + expect(ok.status).toBe(202); + // 60k dots fit under the body cap (the old regex took about 2 s); 200k do not (it took + // about 23 s, and the old body had no cap). + for (const [dots, status] of [ + [60_000, 400], + [200_000, 413], + ] as const) { + const email = `a@${".".repeat(dots)}@`; + const start = performance.now(); + const res = await post(app, "/api/password-reset/request", JSON.stringify({ email })); + expect(res.status).toBe(status); + expect(performance.now() - start).toBeLessThan(1500); + } + }); + + it("answers 413 for a body above the cap, with or without content-length", async () => { + const app = await setup(); + const big = JSON.stringify({ email: `a@${".".repeat(JSON_BODY_MAX_BYTES)}` }); + const res = await post(app, "/api/password-reset/request", big); + expect(res.status).toBe(413); + expect(await res.json()).toMatchObject({ error: "payload_too_large" }); + // A streamed body (no content-length) is cut off once it passes the cap. + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + for (let i = 0; i < 10; i++) controller.enqueue(encoder.encode(" ".repeat(10_000))); + controller.enqueue(encoder.encode("{}")); + controller.close(); + }, + }); + const streamed = await app.request(`${ORIGIN}/api/password-reset/request`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: stream, + duplex: "half", + } as RequestInit); + expect(streamed.status).toBe(413); + // At the cap the body is read and judged as usual. + const atCap = JSON.stringify({ email: "x@y.z", pad: " ".repeat(JSON_BODY_MAX_BYTES - 40) }); + expect(new TextEncoder().encode(atCap).byteLength).toBeLessThanOrEqual(JSON_BODY_MAX_BYTES); + expect((await post(app, "/api/password-reset/request", atCap)).status).toBe(202); + }); +}); diff --git a/packages/core-ts/src/media/css-scan.ts b/packages/core-ts/src/media/css-scan.ts new file mode 100644 index 00000000..dd0fbb9c --- /dev/null +++ b/packages/core-ts/src/media/css-scan.ts @@ -0,0 +1,84 @@ +/** + * Linear scanners for the SVG checks (#174). They return what the regexes they replace returned, + * without the backtracking: `/\/\*[\s\S]*?\*\//g` rescanned to the end for every unclosed `/*`, + * and `/url\s*\(\s*(['"]?)([^'")]*)\1\s*\)/gi` was about cubic on `url(` followed by spaces + * (its two `\s*` and the value class overlap). Uploaded SVGs reach both. + */ + +const isSpace = (ch: string | undefined) => ch !== undefined && /\s/.test(ch); + +/** `css` with every `/* … *\/` comment removed, as `css.replace(/\/\*[\s\S]*?\*\//g, "")`. */ +export function stripCssComments(css: string): string { + let out = ""; + let from = 0; + for (;;) { + const open = css.indexOf("/*", from); + if (open < 0) break; + const close = css.indexOf("*/", open + 2); + // No closer after this opener means none after any later one either. + if (close < 0) break; + out += css.slice(from, open); + from = close + 2; + } + return from === 0 ? css : out + css.slice(from); +} + +/** + * The values of `url(…)` references, as group 2 of + * `css.matchAll(/url\s*\(\s*(['"]?)([^'")]*)\1\s*\)/gi)`: quoted, the text between the quotes; + * unquoted, everything after the opening spaces up to `)`. + */ +export function cssUrlValues(css: string): string[] { + const out: string[] = []; + // The first `'`, `"` or `)` at or after a position; positions only grow, so the cache holds. + let special = -1; + const nextSpecial = (from: number) => { + if (special >= from) return special; + special = css.length; + for (let i = from; i < css.length; i++) { + const ch = css[i]; + if (ch === "'" || ch === '"' || ch === ")") { + special = i; + break; + } + } + return special; + }; + let end = 0; + for (const m of css.matchAll(/url/gi)) { + const at = m.index ?? 0; + if (at < end) continue; + let i = at + 3; + while (isSpace(css[i])) i++; + if (css[i] !== "(") continue; + i++; + while (isSpace(css[i])) i++; + const q = css[i]; + if (q === "'" || q === '"') { + const close = nextSpecial(i + 1); + if (css[close] !== q) continue; + let j = close + 1; + while (isSpace(css[j])) j++; + if (css[j] !== ")") continue; + out.push(css.slice(i + 1, close)); + end = j + 1; + } else { + const close = nextSpecial(i); + if (css[close] !== ")") continue; + out.push(css.slice(i, close)); + end = close + 1; + } + } + return out; +} + +/** + * An SVG length as `/^\s*([0-9]*\.?[0-9]+)\s*(px)?\s*$/` read it: a number (`12`, `1.5`, `.5`), + * optionally followed by `px`, with whitespace around; undefined otherwise. + */ +export function svgPixels(value: string | undefined): number | undefined { + if (!value) return undefined; + let v = value.trim(); + if (v.endsWith("px")) v = v.slice(0, -2).trimEnd(); + return /^(?:[0-9]+|[0-9]*\.[0-9]+)$/.test(v) ? Number(v) : undefined; +} diff --git a/packages/core-ts/src/media/svg.ts b/packages/core-ts/src/media/svg.ts index 8bc0d9e1..f382d2be 100644 --- a/packages/core-ts/src/media/svg.ts +++ b/packages/core-ts/src/media/svg.ts @@ -4,6 +4,7 @@ * attributes is reported, and the upload is refused with the list. SVG is served under a CSP * sandbox as well (§7.2), and only people may upload it in v1 (answer 2). Worker-safe. */ +import { cssUrlValues, stripCssComments, svgPixels } from "./css-scan.ts"; const SVG_NS = "http://www.w3.org/2000/svg"; const XLINK_NS = "http://www.w3.org/1999/xlink"; @@ -141,29 +142,26 @@ function bump(problems: Map, what: string, n = 1): void { /** CSS (a `style` attribute or element) without imports, expressions or non-fragment URLs. */ function checkCss(css: string, problems: Map): void { - const flat = css.replace(/\/\*[\s\S]*?\*\//g, ""); + const flat = stripCssComments(css); if (/\\/.test(flat)) bump(problems, "CSS escapes"); if (/@import/i.test(flat)) bump(problems, "CSS @import rules"); if (/expression\s*\(|behavior\s*:|-moz-binding|javascript:/i.test(flat)) bump(problems, "CSS expressions or script URLs"); - for (const m of flat.matchAll(/url\s*\(\s*(['"]?)([^'")]*)\1\s*\)/gi)) - if (!FRAGMENT.test((m[2] ?? "").trim())) bump(problems, "external references"); + for (const url of cssUrlValues(flat)) + if (!FRAGMENT.test(url.trim())) bump(problems, "external references"); if (/@font-face|@namespace/i.test(flat)) bump(problems, "CSS @font-face or @namespace rules"); } function checkValue(name: string, value: string, problems: Map): void { if (/javascript:|vbscript:|data:text\/html/i.test(value.replace(/\s+/g, ""))) bump(problems, "script URLs"); - for (const m of value.matchAll(/url\s*\(\s*(['"]?)([^'")]*)\1\s*\)/gi)) - if (!FRAGMENT.test((m[2] ?? "").trim())) bump(problems, "external references"); + for (const url of cssUrlValues(value)) + if (!FRAGMENT.test(url.trim())) bump(problems, "external references"); if (name === "style") checkCss(value, problems); } -function length(value: string | undefined): number | undefined { - if (!value) return undefined; - const m = /^\s*([0-9]*\.?[0-9]+)\s*(px)?\s*$/.exec(value); - return m ? Number(m[1]) : undefined; -} +/** Width and height values: a number, optionally in `px`. */ +const length = svgPixels; /** Validates an SVG document. Never throws on bad input; read `ok` and `problems`. */ export function checkSvg(source: string): SvgCheck { diff --git a/packages/core-ts/src/redirects.ts b/packages/core-ts/src/redirects.ts index 1422adef..8aa6f7ab 100644 --- a/packages/core-ts/src/redirects.ts +++ b/packages/core-ts/src/redirects.ts @@ -11,13 +11,14 @@ import { } from "@cloudhal/core-contracts"; import { InvariantError } from "./errors.ts"; import { ID_PREFIX } from "./ids.ts"; +import { trimEndChar } from "./names.ts"; import type { OpBatch } from "./node-sql.ts"; import type { Creatable } from "./nodes.ts"; /** `/a/b/` → `/a/b`; the root stays `/`. Query and fragment are not part of a request path here. */ export function normalisePath(path: string): string { const p = path.split(/[?#]/, 1)[0] ?? "/"; - return p.length > 1 ? p.replace(/\/+$/, "") || "/" : p || "/"; + return p.length > 1 ? trimEndChar(p, "/") || "/" : p || "/"; } /** A target split into its site path and the `?query#fragment` suffix; `undefined` for https URLs. */ @@ -33,13 +34,36 @@ export function joinTarget(to: string, rest: string): string { const i = to.search(/[?#]/); const head = i < 0 ? to : to.slice(0, i); const tail = i < 0 ? "" : to.slice(i); - return `${head.replace(/\/+$/, "")}${rest}${tail}`; + return `${trimEndChar(head, "/")}${rest}${tail}`; } -/** The proper ancestors of a path, longest first, including `/`. */ +/** The contract's `maxLength` of a redirect's `from` (`redirect.schema.json`), in code points. */ +export const REDIRECT_FROM_MAX_LENGTH = 1024; + +/** + * The proper ancestors of a path that a redirect's `from` can be, longest first, including `/`. + * A `from` has at most `REDIRECT_FROM_MAX_LENGTH` code points, so at most twice that many UTF-16 + * units, and no empty segment: the walk starts at the longest ancestor within that many units + * and below the first `//` (#174: a long request path used to yield one ancestor per segment, + * quadratic in size, on every 404). + */ export function ancestorPaths(path: string): string[] { const out: string[] = []; + const maxUnits = 2 * REDIRECT_FROM_MAX_LENGTH; + // The longest ancestor worth walking from ends before `end`. + let end = path.length; + if (end > maxUnits) { + const slash = path.lastIndexOf("/", maxUnits); + end = slash > 0 ? slash : 0; + } + // On the whole path, so a `//` that straddles the cut is found too. + const cut = path.indexOf("//"); + if (cut >= 0 && cut < end) end = cut; let p = path; + if (end < path.length) { + p = path.slice(0, end) || "/"; + out.push(p); + } while (p.length > 1) { p = p.slice(0, p.lastIndexOf("/")) || "/"; out.push(p); diff --git a/packages/core-ts/src/search-rows.ts b/packages/core-ts/src/search-rows.ts index d1ccd91c..1ca67d6d 100644 --- a/packages/core-ts/src/search-rows.ts +++ b/packages/core-ts/src/search-rows.ts @@ -10,6 +10,7 @@ import { termName } from "./names.ts"; import { entryPath, type PermalinkSite } from "./permalinks.ts"; import { MARK_CLOSE, MARK_OPEN, SEARCH_TEXT_MAX, type SearchRow } from "./search-common.ts"; import { searchableFieldTexts } from "./search-fields.ts"; +import { linkTexts, stripFences, stripTags } from "./search-text-scan.ts"; import { termSlug } from "./term-locales.ts"; const iso = (value: string | undefined) => { @@ -30,6 +31,12 @@ const ENTITIES: Record = { const clip = (text: string) => text.length > SEARCH_TEXT_MAX ? text.slice(0, SEARCH_TEXT_MAX) : text; +/** + * A directive line (`::name{…}`, `:::name`). Its braces stay on one line: under `m` every line + * terminator starts a line, and `[^\n]*` running across `\r` was quadratic (#174). + */ +export const DIRECTIVE_LINE = /^:{2,}[a-z][\w-]*(\{[^\n\r\u2028\u2029]*\})?[ \t]*$/gim; + /** * An article body as plain text for the index: code and generated UI dropped, links and images * reduced to their words, URLs and markup removed. Directive attributes keep their quoted text @@ -38,20 +45,16 @@ const clip = (text: string) => export function searchText(body: Pick): string { let text = body.value; if (body.format === "markdown") { - text = text - // fenced code and ```surface fences - .replace(/^(`{3,}|~{3,})[^\n]*\n[\s\S]*?^\1[ \t]*$/gm, " ") + // fenced code and ```surface fences; linear scanners where a regex would backtrack (#174) + text = stripFences(text) // directive lines: keep the text of quoted attributes - .replace(/^:{2,}[a-z][\w-]*(\{[^\n]*\})?[ \t]*$/gim, (_m, attrs: string | undefined) => + .replace(DIRECTIVE_LINE, (_m, attrs: string | undefined) => attrs ? [...attrs.matchAll(/="((?:[^"\\]|\\.)*)"/g)].map((a) => a[1]).join(" ") : " ", ) - .replace(/^:{3,}[ \t]*$/gm, " ") - .replace(/!\[([^\]]*)\]\([^)]*\)/g, "$1") - .replace(/\[([^\]]*)\]\([^)]*\)/g, "$1") - .replace(/`([^`]*)`/g, "$1"); + .replace(/^:{3,}[ \t]*$/gm, " "); + text = linkTexts(linkTexts(text, true), false).replace(/`([^`]*)`/g, "$1"); } - text = text - .replace(/<[^>]*>/g, " ") + text = stripTags(text) .replace(/&(?:amp|lt|gt|quot|#39|nbsp);/g, (m) => ENTITIES[m] ?? m) .replace(/\bhttps?:\/\/\S+/g, " ") .replace(/[#>*_~|\\]+/g, " ") diff --git a/packages/core-ts/src/search-text-scan.ts b/packages/core-ts/src/search-text-scan.ts new file mode 100644 index 00000000..1b529c77 --- /dev/null +++ b/packages/core-ts/src/search-text-scan.ts @@ -0,0 +1,122 @@ +/** + * Linear scanners for `searchText` (#174). Each returns what the regex replace it stands for + * returned, without rescanning to the end of the body for every opener whose closer is missing. + * The regexes were quadratic on such input, and article bodies come from agents and editors: + * + * - `/^(`{3,}|~{3,})[^\n]*\n[\s\S]*?^\1[ \t]*$/gm` → `" "` (`stripFences`); + * - `/!\[([^\]]*)\]\([^)]*\)/g` → `"$1"` and `/\[([^\]]*)\]\([^)]*\)/g` → `"$1"` (`linkTexts`); + * - `/<[^>]*>/g` → `" "` (`stripTags`). + */ + +/** The line terminators `^` and `$` see under the `m` flag. */ +const isLineEnd = (ch: string | undefined) => + ch === "\n" || ch === "\r" || ch === "\u2028" || ch === "\u2029"; + +/** The first value in sorted `list` that is at least `min`, or undefined. */ +function firstAtLeast(list: readonly number[], min: number): number | undefined { + let lo = 0; + let hi = list.length; + while (lo < hi) { + const mid = (lo + hi) >> 1; + if ((list[mid] as number) < min) lo = mid + 1; + else hi = mid; + } + return list[lo]; +} + +/** + * Fenced blocks replaced by one space. An opening line starts with three or more backticks (or + * tildes); the block ends at the nearest later line that is exactly that fence, optionally + * followed by spaces or tabs. Like the regex, a longer opening fence falls back to shorter ones + * (```` ```` ```` may close at ```` ``` ````), longest first. + */ +export function stripFences(text: string): string { + const starts = [0]; + for (let i = 0; i < text.length; i++) if (isLineEnd(text[i])) starts.push(i + 1); + // Closing lines by fence (character and length): each line closes exactly one kind. + const closers = new Map(); + const fenceAt = (s: number) => { + const c = text[s]; + if (c !== "`" && c !== "~") return undefined; + let q = s; + while (text[q] === c) q++; + return q - s >= 3 ? { c, length: q - s } : undefined; + }; + for (const s of starts) { + const f = fenceAt(s); + if (!f) continue; + let e = s + f.length; + while (text[e] === " " || text[e] === "\t") e++; + if (e < text.length && !isLineEnd(text[e])) continue; + const key = f.c + f.length; + const list = closers.get(key); + if (list) list.push(s); + else closers.set(key, [s]); + } + let out = ""; + let from = 0; + // The opening line ends at the next "\n" (not at "\r" or U+2028, which also start lines): + // openers come in order, so the answer is kept until an opener passes it. + let nl: number | undefined; + for (const s of starts) { + if (s < from) continue; + const f = fenceAt(s); + if (!f) continue; + if (nl === undefined || nl < s) nl = text.indexOf("\n", s); + if (nl < 0) break; // no "\n" left: no later opener has a body either + for (let m = f.length; m >= 3; m--) { + const t = firstAtLeast(closers.get(f.c + m) ?? [], nl + 1); + if (t === undefined) continue; + let e = t + m; + while (text[e] === " " || text[e] === "\t") e++; + out += `${text.slice(from, s)} `; + from = e; + break; + } + } + return from === 0 ? text : out + text.slice(from); +} + +/** + * Markdown links (`[text](url)`) or, with `image`, images (`![alt](url)`) replaced by their text: + * the bracket text runs to the first `]`, which must be followed by `(`, and the target to the + * first `)` after it. + */ +export function linkTexts(text: string, image: boolean): string { + const open = image ? "![" : "["; + let out = ""; + let from = 0; + let search = 0; + for (;;) { + const j = text.indexOf(open, search); + if (j < 0) break; + const k = text.indexOf("]", j + open.length); + if (k < 0) break; // no `]` left: no later link can close either + if (text[k + 1] !== "(") { + // Every opener before this `]` reaches the same `]`, so it fails the same way. + search = k + 1; + continue; + } + const end = text.indexOf(")", k + 2); + if (end < 0) break; // no `)` left: no later link can end either + out += text.slice(from, j) + text.slice(j + open.length, k); + from = end + 1; + search = from; + } + return from === 0 ? text : out + text.slice(from); +} + +/** Every `<…>` (up to the first `>`) replaced by one space. */ +export function stripTags(text: string): string { + let out = ""; + let from = 0; + for (;;) { + const j = text.indexOf("<", from); + if (j < 0) break; + const k = text.indexOf(">", j + 1); + if (k < 0) break; // no `>` left: no later tag can close either + out += `${text.slice(from, j)} `; + from = k + 1; + } + return from === 0 ? text : out + text.slice(from); +} diff --git a/packages/core-ts/test/redos-174.test.ts b/packages/core-ts/test/redos-174.test.ts new file mode 100644 index 00000000..da496a44 --- /dev/null +++ b/packages/core-ts/test/redos-174.test.ts @@ -0,0 +1,229 @@ +/** + * #174: every linear rewrite answers like the regex it replaced (seeded random strings over the + * characters that matter, plus hand-picked cases), and stays fast on input that made the regex + * backtrack (100k characters or more). + */ +import type { Redirect } from "@cloudhal/core-contracts"; +import { describe, expect, it } from "vitest"; +import { cssUrlValues, stripCssComments, svgPixels } from "../src/media/css-scan.ts"; +import { checkSvg } from "../src/media/svg.ts"; +import { + ancestorPaths, + joinTarget, + matchRedirect, + normalisePath, + REDIRECT_FROM_MAX_LENGTH, +} from "../src/redirects.ts"; +import { DIRECTIVE_LINE, searchText } from "../src/search-rows.ts"; +import { linkTexts, stripFences, stripTags } from "../src/search-text-scan.ts"; + +/** Deterministic strings: every run of the suite checks the same inputs. */ +function strings(alphabet: readonly string[], count: number, maxLength: number, seed = 174) { + let s = seed; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + const out: string[] = []; + for (let n = 0; n < count; n++) { + let v = ""; + const len = next() % (maxLength + 1); + for (let i = 0; i < len; i++) v += alphabet[next() % alphabet.length]; + out.push(v); + } + return out; +} + +/** + * Runs `f` and fails when it takes longer than 1.5 s, the bound of the repo's ReDoS tests (#168): + * linear code answers in milliseconds, the replaced regexes took seconds on these inputs. + */ +function fast(f: () => T): T { + const start = performance.now(); + const value = f(); + expect(performance.now() - start).toBeLessThan(1500); + return value; +} + +const N = 200_000; + +describe("redirect paths (#174 item 2)", () => { + const oldNormalise = (path: string) => { + const p = path.split(/[?#]/, 1)[0] ?? "/"; + return p.length > 1 ? p.replace(/\/+$/, "") || "/" : p || "/"; + }; + const oldAncestors = (path: string) => { + const out: string[] = []; + let p = path; + while (p.length > 1) { + p = p.slice(0, p.lastIndexOf("/")) || "/"; + out.push(p); + } + return out; + }; + /** What a redirect `from` can be (the contract's pattern). */ + const FROM = /^\/(?:[^\s?#/\\]+(?:\/[^\s?#/\\]+)*)?$/; + /** The contract's `maxLength` counts code points (ajv), not UTF-16 units. */ + const validFrom = (a: string) => FROM.test(a) && [...a].length <= REDIRECT_FROM_MAX_LENGTH; + const usable = (list: string[]) => list.filter(validFrom); + + it("normalisePath and joinTarget answer as before", () => { + for (const p of strings(["/", "a", "?", "#", "b"], 3000, 12)) + expect(normalisePath(p), p).toBe(oldNormalise(p)); + for (const to of strings(["/", "x", "?", "#"], 500, 8)) + for (const rest of ["", "/r", "/r/s"]) + expect(joinTarget(to, rest)).toBe( + (() => { + if (!rest) return to; + const i = to.search(/[?#]/); + const head = i < 0 ? to : to.slice(0, i); + return `${head.replace(/\/+$/, "")}${rest}${i < 0 ? "" : to.slice(i)}`; + })(), + ); + }); + + it("ancestorPaths keeps every ancestor a redirect could match", () => { + for (const raw of strings(["/", "a", "b"], 3000, 12)) { + const p = `/${raw}`; + expect(ancestorPaths(p), p).toEqual(oldAncestors(p).filter(validFrom)); + expect( + ancestorPaths(p).every((a) => FROM.test(a)), + p, + ).toBe(true); + } + const redirects = ["/a", "/a/b", "/"].map( + (from) => + ({ from, to: `/new${from}`, match: "prefix", locale: undefined }) as unknown as Redirect, + ); + expect(matchRedirect(redirects, "/a//x")?.redirect.from).toBe("/a"); + expect(matchRedirect(redirects, "/a/b//c")?.redirect.from).toBe("/a/b"); + expect(matchRedirect(redirects, "//x")?.redirect.from).toBe("/"); + }); + + it("counts a from's length in code points, and never cuts inside a `//`", () => { + // 600 astral characters: 601 code points (a valid `from`), 1201 UTF-16 units. + const from = `/${"😀".repeat(600)}`; + const redirect = { from, to: "/new", match: "prefix" } as unknown as Redirect; + expect(matchRedirect([redirect], `${from}/x`)?.location).toBe("/new/x"); + // Long enough for the cut (more than 2 × 1024 units): the walk still starts at `from`. + const tail = `/${"x".repeat(1000)}`; + expect(matchRedirect([redirect], `${from}${tail}`)?.location).toBe(`/new${tail}`); + // A `//` right at the cut: the first ancestor stops before it, never ending in "/". + for (const p of [ + `/${"a".repeat(1022)}//b`, + `/${"a".repeat(2046)}//b`, + `/${"a".repeat(2047)}//b`, + ]) { + const list = ancestorPaths(p); + expect(list[0], `${p.length}`).toBe(p.slice(0, p.indexOf("//"))); + expect(list.every((a) => a === "/" || !a.endsWith("/"))).toBe(true); + expect(usable(list)).toEqual(usable(oldAncestors(p))); + } + }); + + it("stays linear on long runs of slashes", () => { + fast(() => expect(normalisePath(`/a${"/".repeat(N)}b/`)).toBe(`/a${"/".repeat(N)}b`)); + fast(() => expect(joinTarget(`/t${"/".repeat(N)}`, "/r")).toBe("/t/r")); + fast(() => expect(ancestorPaths(`/a${"/".repeat(N)}b`)).toEqual(["/a", "/"])); + // Many short segments: only ancestors within a `from`'s length are listed, so the output + // stays bounded (it used to be one ancestor per segment, quadratic in total size). + const long = "/a".repeat(N / 2); + const ancestors = fast(() => ancestorPaths(long)); + expect(ancestors[0]).toBe("/a".repeat(REDIRECT_FROM_MAX_LENGTH)); + expect(usable(ancestors)).toEqual(usable(oldAncestors(long.slice(0, 5000)))); + const units = 2 * REDIRECT_FROM_MAX_LENGTH; + expect(ancestors.reduce((n, a) => n + a.length, 0)).toBeLessThanOrEqual( + units * (units / 2 + 1), + ); + // Long fuzzed paths (some astral) keep every ancestor a `from` could be. + for (const raw of strings(["/", "a", "bb", "😀"], 300, 1600)) { + const p = `/${raw}`; + expect(usable(ancestorPaths(p))).toEqual(usable(oldAncestors(p))); + } + }); +}); + +describe("SVG CSS (#174 item 4)", () => { + const OLD_URL = /url\s*\(\s*(['"]?)([^'")]*)\1\s*\)/gi; + const oldLength = (value: string) => { + const m = /^\s*([0-9]*\.?[0-9]+)\s*(px)?\s*$/.exec(value); + return m ? Number(m[1]) : undefined; + }; + + it("answers as the regexes did", () => { + for (const css of strings(["/", "*", "a", " ", "\n"], 3000, 14)) + expect(stripCssComments(css), css).toBe(css.replace(/\/\*[\s\S]*?\*\//g, "")); + const urlParts = ["url", "URL", "(", ")", "'", '"', " ", "#a", "x", "\t"]; + for (const css of strings(urlParts, 5000, 9)) + expect(cssUrlValues(css), css).toEqual([...css.matchAll(OLD_URL)].map((m) => m[2])); + for (const v of strings(["1", ".", "p", "x", " ", "\t"], 3000, 8)) + expect(svgPixels(v), JSON.stringify(v)).toBe(oldLength(v)); + expect(svgPixels(undefined)).toBeUndefined(); + expect(svgPixels("")).toBeUndefined(); + }); + + it("stays linear on unclosed comments, url( runs and long lengths", () => { + fast(() => stripCssComments("/* ".repeat((2 * N) / 3))); + fast(() => cssUrlValues(`url(${" ".repeat(N)}`)); + fast(() => cssUrlValues("url(".repeat(N / 4))); + fast(() => cssUrlValues(`url('${"a".repeat(N)}`)); + fast(() => svgPixels(`${"1".repeat(N)}x`)); + fast(() => svgPixels(`1${" ".repeat(N)}x`)); + // The old `url(` check took minutes on 20k spaces; a whole document with them is quick now + // (attribute values are read from a 64 KiB window, so the document case stays below it). + const svg = ``; + expect(fast(() => checkSvg(svg)).ok).toBe(true); + }); +}); + +describe("search text (#174 item 4)", () => { + const OLD_FENCE = /^(`{3,}|~{3,})[^\n]*\n[\s\S]*?^\1[ \t]*$/gm; + + it("answers as the regexes did", () => { + const fenceParts = ["`", "`", "`", "~", "~", "a", " ", "\t", "\n", "\n", "\r"]; + for (const t of strings(fenceParts, 8000, 24)) + expect(stripFences(t), JSON.stringify(t)).toBe(t.replace(OLD_FENCE, " ")); + for (const t of [ + "```\ncode\n```", + "````\ncode\n```\nafter", + "~~~ js\na\n~~~ \nb", + "```\r\ncode\r\n```\r\n", + "```\nnever closed", + "text\n```a\n```\n```\n", + ]) + expect(stripFences(t), JSON.stringify(t)).toBe(t.replace(OLD_FENCE, " ")); + const linkParts = ["!", "[", "]", "(", ")", "a", "\n"]; + for (const t of strings(linkParts, 5000, 14)) { + expect(linkTexts(t, true), t).toBe(t.replace(/!\[([^\]]*)\]\([^)]*\)/g, "$1")); + expect(linkTexts(t, false), t).toBe(t.replace(/\[([^\]]*)\]\([^)]*\)/g, "$1")); + } + for (const t of strings(["<", ">", "a", "\n"], 3000, 12)) + expect(stripTags(t), t).toBe(t.replace(/<[^>]*>/g, " ")); + // The directive pattern now keeps its braces on one line; without "\r" or U+2028/2029 in + // the body (markdown from Studio and agents is "\n"-only) it matches as before. + const OLD_DIRECTIVE = /^:{2,}[a-z][\w-]*(\{[^\n]*\})?[ \t]*$/gim; + for (const t of strings([":", ":", "a", "{", "}", " ", "\n", "x", '"', "="], 4000, 14)) + expect(t.replace(DIRECTIVE_LINE, "<$1>"), JSON.stringify(t)).toBe( + t.replace(OLD_DIRECTIVE, "<$1>"), + ); + }); + + it("stays linear on openers without closers", () => { + fast(() => stripFences("```a\n".repeat(N / 5))); + // Line ends without "\n": every opener used to search to the end for one. + fast(() => stripFences("```a\r".repeat(N / 5))); + fast(() => stripFences("```a\u2028".repeat(N / 5))); + fast(() => stripFences(`${"```a\r".repeat(N / 5)}\n`)); + fast(() => stripFences("```a\u2029".repeat(N / 5))); + fast(() => stripFences("```a\r\n".repeat(N / 6))); + // Directive lines with CR endings: `[^\n]*` used to run across every following line. + fast(() => searchText({ format: "markdown", value: "::a{}x\r".repeat(N / 7) })); + fast(() => searchText({ format: "markdown", value: "::a{}x\u2028".repeat(N / 7) })); + fast(() => searchText({ format: "markdown", value: "::a{}x\u2029".repeat(N / 7) })); + fast(() => linkTexts("![a".repeat(N / 3), true)); + fast(() => linkTexts("[a](".repeat(N / 4), false)); + fast(() => linkTexts("[a]x".repeat(N / 4), false)); + fast(() => stripTags(" searchText({ format: "markdown", value: `${"```a\n[x](<".repeat(N / 10)}` })); + }); +}); diff --git a/packages/schedules/src/feed-check.ts b/packages/schedules/src/feed-check.ts index 698eb2da..888b7293 100644 --- a/packages/schedules/src/feed-check.ts +++ b/packages/schedules/src/feed-check.ts @@ -12,6 +12,7 @@ import type { FeedFinding, FindingSeverity, GateStateName } from "./findings.ts"; import { instantMs, MINUTE_MS } from "./instant.ts"; import { plainLine } from "./text.ts"; +import { cdataParts, elementContents, mediaUrlValues } from "./xml-scan.ts"; /** Documents larger than this are not parsed (`feed-invalid`). */ export const MAX_DOCUMENT_CHARS = 5_000_000; @@ -153,15 +154,10 @@ const ENTITIES: Record = { /** Element text: CDATA unwrapped, the five XML entities and numeric references decoded. */ export function xmlText(raw: string): string { - const parts: string[] = []; - const re = //g; - let last = 0; - for (const m of raw.matchAll(re)) { - parts.push(decode(raw.slice(last, m.index)), m[1] as string); - last = (m.index ?? 0) + m[0].length; - } - parts.push(decode(raw.slice(last))); - return parts.join("").trim(); + return cdataParts(raw) + .map((p) => decode(p.text) + (p.cdata ?? "")) + .join("") + .trim(); } function decode(s: string): string { @@ -178,30 +174,20 @@ function safeChar(code: number): string | undefined { : undefined; } -function escapeName(name: string): string { - return name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} - /** Every `…` block (non-nesting elements such as `item`, `url`). */ function blocks(text: string, name: string): string[] { - const n = escapeName(name); - return [...text.matchAll(new RegExp(`<${n}(?:\\s[^>]*)?>([\\s\\S]*?)`, "g"))].map( - (m) => m[1] as string, - ); + return elementContents(text, name); } /** The text of the first `` child, or undefined. */ function child(block: string, name: string): string | undefined { - const n = escapeName(name); - const m = new RegExp(`<${n}(?:\\s[^>]*)?>([\\s\\S]*?)`).exec(block); - return m ? xmlText(m[1] as string) : undefined; + const first = elementContents(block, name, 1)[0]; + return first === undefined ? undefined : xmlText(first); } /** `url="…"` attributes of `` and ``. */ function mediaAttributes(block: string): string[] { - return [...block.matchAll(/<(?:enclosure|media:content)\b[^>]*?\burl\s*=\s*(["'])(.*?)\1/g)].map( - (m) => decode(m[2] as string), - ); + return mediaUrlValues(block).map(decode); } /** A URL's path as the site routes it (no query, fragment or trailing slash), or undefined. */ @@ -214,7 +200,10 @@ export function urlPath(url: string): string | undefined { } catch { // keep the encoded form } - return p.length > 1 ? p.replace(/\/+$/, "") || "/" : p || "/"; + // A linear trim of trailing "/" (`/\/+$/` backtracks on a long run of "/" inside the path). + let end = p.length; + while (end > 1 && p[end - 1] === "/") end--; + return p.length > 1 ? p.slice(0, end) : p || "/"; } catch { return undefined; } diff --git a/packages/schedules/src/xml-scan.ts b/packages/schedules/src/xml-scan.ts new file mode 100644 index 00000000..ed36bcab --- /dev/null +++ b/packages/schedules/src/xml-scan.ts @@ -0,0 +1,173 @@ +/** + * Linear scanners for the feed check (#174). Each returns what the regex it replaces returned, + * without rescanning to the end of the document for every opener whose closer is missing: + * `//g`, `]*)?>([\s\S]*?)` and + * `/<(?:enclosure|media:content)\b[^>]*?\burl\s*=\s*(["'])(.*?)\1/g` were quadratic on such input. + */ + +const isSpace = (ch: string | undefined) => ch !== undefined && /\s/.test(ch); +const isWord = (ch: string | undefined) => ch !== undefined && /\w/.test(ch); +/** What `.` does not match without the `s` flag. */ +const isLineEnd = (ch: string) => ch === "\n" || ch === "\r" || ch === "\u2028" || ch === "\u2029"; + +const CDATA_OPEN = ""; + +/** `raw` split into text outside CDATA sections and their contents, in order. */ +export function cdataParts(raw: string): { text: string; cdata?: string }[] { + const out: { text: string; cdata?: string }[] = []; + let from = 0; + for (;;) { + const open = raw.indexOf(CDATA_OPEN, from); + if (open < 0) break; + const close = raw.indexOf(CDATA_CLOSE, open + CDATA_OPEN.length); + // No closer after this opener means none after any later one either. + if (close < 0) break; + out.push({ text: raw.slice(from, open), cdata: raw.slice(open + CDATA_OPEN.length, close) }); + from = close + CDATA_CLOSE.length; + } + out.push({ text: raw.slice(from) }); + return out; +} + +/** + * The contents of every `…` element (an opening tag `` or ``, the + * nearest `` closer, whitespace allowed before its `>`), as group 1 of the global regex. + * `limit` stops after that many (1 for the first, as a non-global `exec` finds it). + */ +export function elementContents(text: string, name: string, limit = Infinity): string[] { + const out: string[] = []; + const open = `<${name}`; + const close = `` left + const nextGt = (from: number) => { + if (gt === undefined || (gt >= 0 && gt < from)) gt = text.indexOf(">", from); + return gt; + }; + let closer: { at: number; end: number } | undefined | null; + const nextCloser = (from: number): { at: number; end: number } | undefined => { + if (closer === null) return undefined; + if (closer && closer.at >= from) return closer; + for (let c = text.indexOf(close, from); c >= 0; c = text.indexOf(close, c + 1)) { + let j = c + close.length; + while (isSpace(text[j])) j++; + if (text[j] === ">") { + closer = { at: c, end: j + 1 }; + return closer; + } + } + closer = null; + return undefined; + }; + for (let at = text.indexOf(open); at >= 0 && out.length < limit; ) { + const after = at + open.length; + let start: number | undefined; + if (text[after] === ">") start = after + 1; + else if (isSpace(text[after])) { + const g = nextGt(after + 1); + if (g < 0) break; // no `>` left: no later opening tag can end either + start = g + 1; + } + if (start === undefined) { + at = text.indexOf(open, at + 1); + continue; + } + const c = nextCloser(start); + if (!c) break; // no closer left: no later element can end either + out.push(text.slice(start, c.at)); + at = text.indexOf(open, c.end); + } + return out; +} + +const MEDIA_TAGS = ["(); // -1: none left + const find = (tag: string, from: number) => { + const known = next.get(tag); + if (known !== undefined && (known < 0 || known >= from)) return known; + let at = text.indexOf(tag, from); + while (at >= 0 && isWord(text[at + tag.length])) at = text.indexOf(tag, at + 1); + next.set(tag, at); + return at; + }; + return (from: number): { at: number; nameEnd: number } | undefined => { + let best: { at: number; nameEnd: number } | undefined; + for (const tag of MEDIA_TAGS) { + const at = find(tag, from); + if (at >= 0 && (!best || at < best.at)) best = { at, nameEnd: at + tag.length }; + } + return best; + }; +} + +/** + * `indexOf(needle, from)` that keeps its last answer: a query at or after the last one, and not + * past the kept position, is answered without searching again (-1 stays -1). + */ +function forwardFinder(text: string, needle: string): (from: number) => number { + let asked = -1; + let found = -1; + return (from) => { + if (asked >= 0 && from >= asked && (found < 0 || found >= from)) return found; + asked = from; + found = text.indexOf(needle, from); + return found; + }; +} + +/** At `p`: `url`, optional spaces, `=`, optional spaces, a quote; the value and where it ends. */ +function urlAttribute(text: string, p: number): { value: string; end: number } | undefined { + if (isWord(text[p - 1]) || !text.startsWith("url", p)) return undefined; + let i = p + 3; + while (isSpace(text[i])) i++; + if (text[i] !== "=") return undefined; + i++; + while (isSpace(text[i])) i++; + const q = text[i]; + if (q !== '"' && q !== "'") return undefined; + for (let j = i + 1; j < text.length; j++) { + const ch = text[j] as string; + if (ch === q) return { value: text.slice(i + 1, j), end: j + 1 }; + if (isLineEnd(ch)) return undefined; + } + return undefined; +} + +/** + * The `url="…"` values of `` and `` tags, as group 2 of the regex: the + * first `url` attribute inside the tag (before its `>`) whose quoted value closes on its line. + */ +export function mediaUrlValues(text: string): string[] { + const out: string[] = []; + const nextTag = mediaTagFinder(text); + // Tags are visited in order, so the next `>` and the next `url` are each found once and kept + // until a query passes them: a per-tag `indexOf` made `` × n quadratic. + const nextGt = forwardFinder(text, ">"); + const nextUrl = forwardFinder(text, "url"); + let from = 0; + for (let tag = nextTag(from); tag; tag = nextTag(from)) { + const gt = nextGt(tag.nameEnd); + const stop = gt < 0 ? text.length : gt; + let found: { value: string; end: number } | undefined; + for (let p = nextUrl(tag.nameEnd); p >= 0 && p < stop; p = nextUrl(p + 1)) { + found = urlAttribute(text, p); + if (found) break; + } + if (found) { + out.push(found.value); + from = found.end; + continue; + } + // Every later tag that starts before this `>` sees the same candidates, which all failed. + if (gt < 0) break; + from = gt + 1; + } + return out; +} diff --git a/packages/schedules/test/xml-scan.test.ts b/packages/schedules/test/xml-scan.test.ts new file mode 100644 index 00000000..4b45244e --- /dev/null +++ b/packages/schedules/test/xml-scan.test.ts @@ -0,0 +1,124 @@ +/** + * #174: the feed check's linear scanners answer like the regexes they replaced (seeded random + * strings plus hand-picked cases) and stay fast on 100k characters of openers without closers. + */ +import { describe, expect, it } from "vitest"; +import { urlPath, xmlText } from "../src/feed-check.ts"; +import { cdataParts, elementContents, mediaUrlValues } from "../src/xml-scan.ts"; + +function strings(alphabet: readonly string[], count: number, maxLength: number, seed = 174) { + let s = seed; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + const out: string[] = []; + for (let n = 0; n < count; n++) { + let v = ""; + const len = next() % (maxLength + 1); + for (let i = 0; i < len; i++) v += alphabet[next() % alphabet.length]; + out.push(v); + } + return out; +} + +/** Fails when `f` takes longer than 1.5 s, the bound of the repo's ReDoS tests (#168). */ +function fast(f: () => T): T { + const start = performance.now(); + const value = f(); + expect(performance.now() - start).toBeLessThan(1500); + return value; +} + +// 200k characters, more where the old regex's cost grew slowly: each case took the replaced +// regex seconds. +const N = 200_000; +const OLD_CDATA = //g; +const OLD_MEDIA = /<(?:enclosure|media:content)\b[^>]*?\burl\s*=\s*(["'])(.*?)\1/g; +const oldBlocks = (text: string, n: string) => + [...text.matchAll(new RegExp(`<${n}(?:\\s[^>]*)?>([\\s\\S]*?)`, "g"))].map( + (m) => m[1], + ); + +describe("feed-check scanners (#174 item 4)", () => { + it("cdataParts splits as the CDATA regex did", () => { + for (const raw of strings(["", "a", "]", "<"], 3000, 8)) { + const parts: { text: string; cdata?: string }[] = []; + let last = 0; + for (const m of raw.matchAll(OLD_CDATA)) { + parts.push({ text: raw.slice(last, m.index), cdata: m[1] as string }); + last = (m.index ?? 0) + m[0].length; + } + parts.push({ text: raw.slice(last) }); + expect(cdataParts(raw), raw).toEqual(parts); + } + expect(xmlText("a & &]]> c")).toBe("a & & c"); + }); + + it("elementContents finds the blocks the element regex found", () => { + const parts = ["", " ", "\n", "a", "s", "<"]; + for (const text of strings(parts, 6000, 10)) { + expect(elementContents(text, "item"), text).toEqual(oldBlocks(text, "item")); + expect(elementContents(text, "item", 1), text).toEqual(oldBlocks(text, "item").slice(0, 1)); + } + expect(elementContents("xyz", "item")).toEqual(["x", "z"]); + }); + + it("mediaUrlValues finds the url attributes the media regex found", () => { + const parts = ["", "\n", "a", "x"]; + for (const text of strings(parts, 8000, 9)) + expect(mediaUrlValues(text), text).toEqual([...text.matchAll(OLD_MEDIA)].map((m) => m[2])); + expect(mediaUrlValues('')).toEqual([ + "https://x/a.mp3", + ]); + // A feed with several tags of both kinds: `type=` before and after `url=`, the word `url` + // in text between tags, and `data-url`. The regex has a word boundary between "-" and "url", + // so it took `data-url` for the url attribute; the scanner keeps that (results unchanged). + const feed = [ + "The url of the day", + '', + "", + "see url = here", + '', + '', + "", + ].join("\n"); + expect(mediaUrlValues(feed)).toEqual([ + "https://x/1.mp3", + "https://x/2.jpg", + "https://x/no", + "https://x/no", + "https://x/4.mp3", + ]); + expect(mediaUrlValues(feed)).toEqual([...feed.matchAll(OLD_MEDIA)].map((m) => m[2])); + }); + + it("urlPath trims trailing slashes as before", () => { + const old = (url: string) => { + const p = new URL(url.trim(), "https://site.invalid").pathname; + return p.length > 1 ? p.replace(/\/+$/, "") || "/" : p || "/"; + }; + for (const raw of strings(["/", "a", "b"], 2000, 10)) { + const url = `https://x/${raw}`; + expect(urlPath(url), url).toBe(old(url)); + } + }); + + it("stays linear on openers without closers", () => { + fast(() => cdataParts(" elementContents("".repeat((2 * N) / 6), "item")); + fast(() => elementContents(" elementContents(`${" mediaUrlValues(" mediaUrlValues(''.repeat(N / 19))); + // Per-tag searches for `>` and `url` used to run to the end of the feed for every tag. + fast(() => mediaUrlValues(' mediaUrlValues("".repeat((2 * N) / 11))); + fast(() => + mediaUrlValues(`${'>`), + ); + expect(mediaUrlValues(' expect(urlPath(`https://x/a${"/".repeat(N)}b/`)).toBe(`/a${"/".repeat(N)}b`)); + }); +}); diff --git a/packages/ui/src/blocks/editor/markdown-blocks.ts b/packages/ui/src/blocks/editor/markdown-blocks.ts index 0384fff0..7766d227 100644 --- a/packages/ui/src/blocks/editor/markdown-blocks.ts +++ b/packages/ui/src/blocks/editor/markdown-blocks.ts @@ -15,6 +15,7 @@ import { type SplitMarkdown, splitMarkdown, } from "@cloudhal/core-ts/body-blocks"; +import { trimEndChar } from "@cloudhal/core-ts/names"; import { Marked } from "marked"; export type { MarkdownUnit, SplitMarkdown } from "@cloudhal/core-ts/body-blocks"; @@ -66,12 +67,13 @@ export function mergeBlocks( } // A block that was not last before keeps the blank lines it had in the middle: end it with one // newline, like a new block. An untouched body ends exactly as it did. - if (previous !== source.units.length - 1 && out !== "") out = out.replace(/\n*$/, "\n"); + if (previous !== source.units.length - 1 && out !== "") out = `${trimEndChar(out, "\n")}\n`; return out; } /** `text` ready for another block: nothing, or ending in one blank line. */ function separated(text: string): string { if (text === "") return ""; - return `${text.replace(/\n*$/, "")}\n\n`; + // `trimEndChar`, not `/\n*$/`: that backtracks on a long run of newlines inside the text (#174). + return `${trimEndChar(text, "\n")}\n\n`; } diff --git a/packages/ui/test/markdown-blocks-redos.test.tsx b/packages/ui/test/markdown-blocks-redos.test.tsx new file mode 100644 index 00000000..d72ad8c5 --- /dev/null +++ b/packages/ui/test/markdown-blocks-redos.test.tsx @@ -0,0 +1,28 @@ +/** #174 item 6: the editor's newline trims answer as `/\n*$/` did, in linear time. */ +import { trimEndChar } from "@cloudhal/core-ts/names"; +import { describe, expect, it } from "vitest"; +import { mergeBlocks, splitMarkdown } from "../src/blocks/editor/markdown-blocks.ts"; + +describe("markdown block trims (#174)", () => { + it("trimEndChar on newlines is the old /\\n*$/ replace", () => { + let s = 174; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + for (let n = 0; n < 5000; n++) { + let t = ""; + for (let i = next() % 10; i > 0; i--) t += ["\n", "\n", "a", "\r", " "][next() % 5]; + expect(`${trimEndChar(t, "\n")}\n`, JSON.stringify(t)).toBe(t.replace(/\n*$/, "\n")); + expect(trimEndChar(t, "\n"), JSON.stringify(t)).toBe(t.replace(/\n*$/, "")); + } + }); + + it("merges a block with a long run of blank lines in linear time", () => { + const tall = `x${"\n".repeat(200_000)}y`; + const start = performance.now(); + const out = mergeBlocks(splitMarkdown(""), [], ["a", tall, "b"]); + expect(performance.now() - start).toBeLessThan(1500); + expect(out).toBe(`a\n\n${tall}\n\nb\n`); + }); +});