diff --git a/CHANGELOG.md b/CHANGELOG.md index b99f757aa7..f1ea7c64cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,15 @@ between releases do not reach them. Remote-PC runtime bundles are versioned sepa time took it for a shell: it opened the terminal instead of the chat, and resized the terminal shared with the other devices. ([#537](https://github.com/devswha/herdr-web-ui/pull/537)) +- A message can no longer freeze or crash the chat by its shape alone. A long run of underscores + nested emphasis until the page ran out of stack; a URL followed by thousands of closing + parentheses, a formula opened on many lines and never closed, a long blank line under a table + header, a list item of thousands of lines, and a heading or list item holding a line-separator + character after a long run of spaces each took seconds to read; a line of plus signs took half + a second, and a formula of thousands of nested fractions a second more. Each is read in a few + milliseconds now. Emphasis stops nesting after sixteen levels, and a formula nested deeper than + a hundred braces shows as its source. + ([#547](https://github.com/devswha/herdr-web-ui/pull/547)) - A workspace row's state rolls up as herdr's does: a workspace with one finished agent and one still running shows DONE, where it showed RUN. ([#521](https://github.com/devswha/herdr-web-ui/pull/521)) diff --git a/src/components/Markdown.tsx b/src/components/Markdown.tsx index d2d71cdbd7..644ac53173 100644 --- a/src/components/Markdown.tsx +++ b/src/components/Markdown.tsx @@ -1,7 +1,7 @@ import { useContext, useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { Check, Copy } from "lucide-react"; -import { foldCode, parseMarkdown, type InlineNode, type ListBlock, type MarkdownBlock } from "../lib/markdown.ts"; +import { foldCode, mathNestsTooDeep, parseMarkdown, type InlineNode, type ListBlock, type MarkdownBlock } from "../lib/markdown.ts"; import { codeIsFilePath, OpenFileContext, splitFilePaths } from "../lib/filePaths.ts"; import { fileUriPath } from "../lib/terminalFileLinks.ts"; import { useT } from "../lib/i18n.ts"; @@ -33,7 +33,7 @@ function useKatex(): Katex | null { function MathExpression({ value, displayMode = false }: { value: string; displayMode?: boolean }) { const katex = useKatex(); const source = displayMode ? `\\[${value}\\]` : `\\(${value}\\)`; - if (!katex) return {source}; + if (!katex || mathNestsTooDeep(value)) return {source}; try { // KaTeX escapes text and rejects untrusted commands by default. const html = katex.renderToString(value, { displayMode, strict: "ignore" }); diff --git a/src/lib/filePaths.test.ts b/src/lib/filePaths.test.ts index d46f8611d3..5be8e91dfa 100644 --- a/src/lib/filePaths.test.ts +++ b/src/lib/filePaths.test.ts @@ -17,6 +17,17 @@ describe("file paths in chat text", () => { expect(paths("and/or, e.g. v1.2.3, 1.0/2.5, README.md, TCP/IP")).toEqual([]); }); + it("starts a path once in a run of plus signs, not at every one", () => { + // each `+` is a character a path may hold: tried from every one of them, a line of 48,000 takes half a second + const start = performance.now(); + expect(splitFilePaths("+".repeat(48_000))).toEqual(["+".repeat(48_000)]); + expect(splitFilePaths("+/".repeat(24_000))).toHaveLength(1); + expect(performance.now() - start).toBeLessThan(250); + // a plus sign before or inside a path reads as it did + expect(paths("+src/app.ts and c++/x/y.cc, a+b/c.ts")).toEqual(["+src/app.ts", "c++/x/y.cc", "a+b/c.ts"]); + expect(splitFilePaths("+~/notes/a.md")).toEqual(["+", { path: "~/notes/a.md" }]); + }); + it("takes a code span that is one file name or path", () => { for (const code of ["README.md", "src/app.ts", "~/x/y.png", "/tmp/a.log", "docs/demo.mp4"]) expect(codeIsFilePath(code)).toBe(true); for (const code of ["bun test", "1.2.3", "git log --oneline", "a/b", "foo()", "x.y()", "tool.monitor", "Math.random", "process.env", "os.path", "process.env.HOME", "console.log", "JSON.parse"]) expect(codeIsFilePath(code)).toBe(false); diff --git a/src/lib/filePaths.ts b/src/lib/filePaths.ts index d8871e8848..1992bb657d 100644 --- a/src/lib/filePaths.ts +++ b/src/lib/filePaths.ts @@ -6,7 +6,10 @@ import { createContext } from "react"; * like "and/or" or "e.g." stay text; code spans need only look like one file name. */ -const BARE_PATH = /(? { it("renders inline and display math while leaving fenced code untouched", async () => { @@ -218,6 +218,142 @@ describe("foldCode", () => { }); }); +describe("text no one wrote by hand", () => { + // what an agent prints, or a changelog holds, is not always prose: the page parses it while it + // draws, so a line that takes seconds freezes the tab, and one that nests without end crashes it + const within = (milliseconds: number, run: () => void): void => { + const start = performance.now(); + run(); + expect(performance.now() - start).toBeLessThan(milliseconds); + }; + /** every string of up to `length` of the `parts`, in order */ + const everyLine = (parts: string[], length: number): string[] => { + let lines = [""]; + const all = [""]; + for (let step = 0; step < length; step += 1) { + lines = lines.flatMap((line) => parts.map((part) => line + part)); + all.push(...lines); + } + return all; + }; + + it("cuts a URL's closing parentheses as it did, counting them once", () => { + // the rule as it was written first: both counts taken again at every closing parenthesis + const asBefore = (url: string): string => { + let end = url.length; + for (;;) { + const last = url[end - 1]; + if (last !== undefined && ".,:;!?'\"*_~".includes(last)) { end -= 1; continue; } + if (last === ")") { + const text = url.slice(0, end); + if (text.split(")").length > text.split("(").length) { end -= 1; continue; } + } + break; + } + return url.slice(0, end); + }; + const urls = everyLine(["a", "(", ")", ".", "_"], 6).map((tail) => `https://x.dev/${tail}`); + expect(urls.length).toBeGreaterThan(19_000); + for (const url of urls) expect(trimUrl(url)).toBe(asBefore(url)); + within(1000, () => expect(trimUrl(`https://x.dev/${")".repeat(48_000)}`)).toBe("https://x.dev/")); + within(1000, () => parseMarkdown(`https://x.dev/${")".repeat(48_000)}`)); + }); + + it("knows a table's separator line as it did, cell by cell", () => { + // the one pattern it was: it shares a run of spaces between its parts in every way there is + const asBefore = /^\s*\|?\s*:?-{3,}:?\s*(?:\|\s*:?-{3,}:?\s*)+\|?\s*$/; + const lines = everyLine([" ", "|", "---", "--", ":", "x"], 6); + expect(lines.length).toBeGreaterThan(55_000); + let separators = 0; + for (const line of lines) { + expect(`${line}: ${isTableSeparator(line)}`).toBe(`${line}: ${asBefore.test(line)}`); + if (isTableSeparator(line)) separators += 1; + } + expect(separators).toBeGreaterThan(100); + for (const line of ["|---|---|", "---|---", " | :--- | ---: | :---: | ", "\t|---\t|\t---|\t"]) expect(isTableSeparator(line)).toBe(true); + for (const line of ["", "---", "|---|", "||---|---", "|---|---||", "|--|--|", "|---|-x-|"]) expect(isTableSeparator(line)).toBe(false); + within(1000, () => parseMarkdown(`a|b\n${" ".repeat(96_000)}x`)); + within(1000, () => parseMarkdown(`a|b\n---|---${" ".repeat(96_000)}x`)); + }); + + it("looks once for the end of a formula that never closes", () => { + within(1000, () => { + const blocks = parseMarkdown("\\[\n".repeat(32_000)); + expect(blocks).toHaveLength(32_000); + expect(blocks.every((block) => block.type === "paragraph")).toBe(true); + }); + // the search ends at a fence: what opens after it is looked for again + expect(parseMarkdown("\\[ a\n\\[ b\n```\ncode\n```\n\\[ c \\]\n\\[ d").map((block) => block.type)) + .toEqual(["paragraph", "paragraph", "code", "math", "paragraph"]); + }); + + it("adds an item's lines to it without copying the item at every line", () => { + within(1000, () => { + const [list] = parseMarkdown(`- a\n${" x\n".repeat(48_000)}`) as [ListBlock]; + expect(list.items).toHaveLength(1); + expect(list.items[0]?.content).toHaveLength(96_001); + }); + }); + + it("stops nesting emphasis after a few levels, and keeps what is inside as text", () => { + const depth = (nodes: InlineNode[]): number => { + let levels = 0; + for (let level = nodes; level.length > 0; levels += 1) { + level = level.flatMap((node) => "children" in node ? node.children : []); + } + return levels; + }; + within(1000, () => { + const nodes = parseInline(`${"_".repeat(48_000)} x`); + expect(depth(nodes)).toBeLessThan(20); + // nothing is lost: what the levels left over hold is still there, as text + expect(JSON.stringify(nodes)).toContain("_".repeat(40_000)); + }); + // what people write nests as before + expect(parseInline("**a _b ~~c~~_**")).toEqual([{ type: "strong", children: [{ type: "text", value: "a " }, + { type: "em", children: [{ type: "text", value: "b " }, { type: "del", children: [{ type: "text", value: "c" }] }] }] }]); + expect(depth(parseInline(`${"__".repeat(8)}a${"__".repeat(8)}`))).toBe(9); + // a link around it is not a level: sixteen of emphasis read whole inside a label too + const sixteen = `${"__".repeat(16)}a\`x\`b${"__".repeat(16)}`; + expect(JSON.stringify(parseInline(sixteen))).toContain('{"type":"code","value":"x"}'); + expect(JSON.stringify(parseInline(`[${sixteen}](https://x.dev)`))).toContain('{"type":"code","value":"x"}'); + }); + + it("reads a heading or an item whose text holds a line separator, without trying every split of its spaces", () => { + // U+2028 is not a line here (only \n is), and `.` stops at it unless told otherwise + within(1000, () => { + const [heading] = parseMarkdown(`# ${" ".repeat(48_000)}a\u2028b`); + expect(heading).toEqual({ type: "heading", level: 1, content: [{ type: "text", value: "a\u2028b" }] }); + }); + within(1000, () => { + const [list] = parseMarkdown(`- ${" ".repeat(48_000)}a\u2028b`) as [ListBlock]; + expect(list.items).toEqual([{ content: [{ type: "text", value: "a\u2028b" }] }]); + }); + within(1000, () => parseMarkdown(`1. ${" ".repeat(48_000)}\u2029`)); + }); + + it("shows a formula nested beyond reason as its source, without asking KaTeX", async () => { + for (const formula of ["x", "\\frac{1}{2}", "{".repeat(100) + "}".repeat(100), "\\{".repeat(500), "{}".repeat(5000)]) expect(mathNestsTooDeep(formula)).toBe(false); + expect(mathNestsTooDeep("{".repeat(101))).toBe(true); + const fractions = `${"\\frac{1}{".repeat(4000)}x${"}".repeat(4000)}`; + expect(mathNestsTooDeep(fractions)).toBe(true); + await loadKatex(); + const languages = Object.getOwnPropertyDescriptor(navigator, "languages"); + Object.defineProperty(navigator, "languages", { configurable: true, value: ["en"] }); + try { + within(1000, () => { + const html = renderToStaticMarkup(createElement(SettingsProvider, { children: createElement(Markdown, { children: `\\[${fractions}\\]\n\n\\(\\frac{1}{2}\\)` }) })); + expect(html).toContain("\\frac{1}{\\frac{1}{"); + // the formula beside it is still drawn + expect(html).toContain("katex-html"); + }); + } finally { + if (languages) Object.defineProperty(navigator, "languages", languages); + else Reflect.deleteProperty(navigator, "languages"); + } + }); +}); + describe("numbered lists as agents write them", () => { const render = (source: string): string => { const languages = Object.getOwnPropertyDescriptor(navigator, "languages"); diff --git a/src/lib/markdown.ts b/src/lib/markdown.ts index e01aa4ef50..9eac35d0c2 100644 --- a/src/lib/markdown.ts +++ b/src/lib/markdown.ts @@ -72,21 +72,29 @@ export function markdownFileTarget(href: string): string | null { /** A bare URL without the punctuation that closes the sentence around it (GFM's autolink rule). */ export function trimUrl(url: string): string { let end = url.length; + // counted once: an opening one is never cut, a closing one only from the end + let opened = 0; + let closed = 0; + for (let index = 0; index < end; index += 1) { + if (url[index] === "(") opened += 1; + else if (url[index] === ")") closed += 1; + } for (;;) { const last = url[end - 1]; if (last !== undefined && ".,:;!?'\"*_~".includes(last)) { end -= 1; continue; } // a closing parenthesis stays only while it closes one opened inside the URL - if (last === ")") { - const text = url.slice(0, end); - if (text.split(")").length > text.split("(").length) { end -= 1; continue; } - } + if (last === ")" && closed > opened) { end -= 1; closed -= 1; continue; } break; } return url.slice(0, end); } +/** Emphasis nests by one call each: past this many levels, what is inside is read as plain text. */ +const MAX_INLINE_DEPTH = 16; + /** Dependency-free inline markdown scanner. Unknown or malformed markup remains text. */ -export function parseInline(source: string, links = true): InlineNode[] { +export function parseInline(source: string, links = true, depth = 0): InlineNode[] { + if (depth > MAX_INLINE_DEPTH) return source === "" ? [] : [{ type: "text", value: source }]; const nodes: InlineNode[] = []; // Underscores inside identifiers are literal: MAC_QA_CHAT_OK must survive // rendering exactly as it appears in the terminal and native transcript. @@ -120,17 +128,18 @@ export function parseInline(source: string, links = true): InlineNode[] { const split = token.lastIndexOf("]("); const label = token.slice(1, split); const target = token.slice(split + 2, -1); + // a label holds no `]`, so no link nests in one: only emphasis counts toward the depth const href = safeMarkdownHref(target) ?? webLikeHref(target); const file = href === null ? markdownFileTarget(target) : null; - nodes.push(href !== null ? { type: "link", href, children: parseInline(label, false) } - : file !== null ? { type: "file", path: file, children: parseInline(label, false) } + nodes.push(href !== null ? { type: "link", href, children: parseInline(label, false, depth) } + : file !== null ? { type: "file", path: file, children: parseInline(label, false, depth) } : { type: "text", value: label }); } else if (token.startsWith("**") || token.startsWith("__")) { - nodes.push({ type: "strong", children: parseInline(token.slice(2, -2), links) }); + nodes.push({ type: "strong", children: parseInline(token.slice(2, -2), links, depth + 1) }); } else if (token.startsWith("~~")) { - nodes.push({ type: "del", children: parseInline(token.slice(2, -2), links) }); + nodes.push({ type: "del", children: parseInline(token.slice(2, -2), links, depth + 1) }); } else { - nodes.push({ type: "em", children: parseInline(token.slice(1, -1), links) }); + nodes.push({ type: "em", children: parseInline(token.slice(1, -1), links, depth + 1) }); } offset = index + token.length; } @@ -138,8 +147,21 @@ export function parseInline(source: string, links = true): InlineNode[] { return nodes; } -const listLine = /^(\s*)([-*]|\d+\.)\s+(.+)$/; -const tableSeparator = /^\s*\|?\s*:?-{3,}:?\s*(?:\|\s*:?-{3,}:?\s*)+\|?\s*$/; +// `s`: the text may hold a line separator (U+2028) that `.` would stop at, and the pattern then +// tries every split of the spaces before it; with it the line is an item, as it reads +const listLine = /^(\s*)([-*]|\d+\.)\s+(.+)$/s; +/** + * The line under a table's header: two or more cells of dashes (`---`, `:---:`), with or without + * the outer pipes. Read cell by cell: one pattern for the whole line tries every way to share + * a run of spaces between its parts, and a long blank line takes seconds. + */ +export function isTableSeparator(line: string): boolean { + let row = line.trim(); + if (row.startsWith("|")) row = row.slice(1); + if (row.endsWith("|")) row = row.slice(0, -1); + const columns = row.split("|"); + return columns.length >= 2 && columns.every((column) => /^:?-{3,}:?$/.test(column.trim())); +} function cells(line: string): string[] { const trimmed = line.trim().replace(/^\|/, "").replace(/\|$/, ""); @@ -149,7 +171,7 @@ function lineAt(lines: string[], index: number): string { return lines[index] ?? ""; } function startsTable(lines: string[], index: number): boolean { - return lineAt(lines, index).includes("|") && tableSeparator.test(lineAt(lines, index + 1)); + return lineAt(lines, index).includes("|") && isTableSeparator(lineAt(lines, index + 1)); } /** A table in a list item (`within` > 0) ends at a line outside the item or at the next item. */ function parseTable(lines: string[], start: number, within = 0): { block: MarkdownBlock; next: number } { @@ -203,7 +225,9 @@ function parseList(lines: string[], start: number): { block: ListBlock; next: nu const item = block.items.at(-1)!; const last = item.blocks?.at(-1); if (last === undefined) { - item.content = [...item.content, { type: "text", value: " " }, ...parseInline(line.trim())]; + // added to, not copied: an item of thousands of lines copies itself at every one + item.content.push({ type: "text", value: " " }); + for (const node of parseInline(line.trim())) item.content.push(node); } else { // after a table or nested list the item's text goes on below it; a quote or rule // there ends the list and shows as its own block, as it did before tables nested @@ -233,6 +257,24 @@ function parseList(lines: string[], start: number): { block: ListBlock; next: nu return { block, next: index }; } +/** KaTeX reads a formula one call per group: nested deeper than this, it is shown as its source. */ +const MAX_MATH_DEPTH = 100; + +/** + * Whether a formula's braces nest deeper than anyone writes them. KaTeX takes over a second on + * thousands of nested fractions before it gives up, with the page waiting on it. + */ +export function mathNestsTooDeep(value: string): boolean { + let depth = 0; + for (let index = 0; index < value.length; index += 1) { + const char = value[index]; + if (char === "\\") index += 1; + else if (char === "{") { depth += 1; if (depth > MAX_MATH_DEPTH) return true; } + else if (char === "}" && depth > 0) depth -= 1; + } + return false; +} + /** Code blocks longer than this open folded to their first FOLDED_CODE_LINES lines. */ export const FOLD_CODE_AFTER_LINES = 30; export const FOLDED_CODE_LINES = 20; @@ -251,6 +293,8 @@ export function parseMarkdown(source: string, depth = 0): MarkdownBlock[] { const lines = source.replace(/\r\n?/g, "\n").split("\n"); const blocks: MarkdownBlock[] = []; let index = 0; + // no formula opened before this line closes: the search that found so is not made again + let unclosedMath = 0; while (index < lines.length) { const line = lineAt(lines, index); if (line.trim() === "") { index += 1; continue; } @@ -270,7 +314,7 @@ export function parseMarkdown(source: string, depth = 0): MarkdownBlock[] { continue; } - const display = /^\s*\\\[(.*)$/.exec(line); + const display = index < unclosedMath ? null : /^\s*\\\[(.*)$/.exec(line); if (display !== null) { const body: string[] = []; let next = index; @@ -292,9 +336,11 @@ export function parseMarkdown(source: string, depth = 0): MarkdownBlock[] { } // An incomplete formula stays prose; later headings and fences still parse normally. if (complete) continue; + // every `\[` up to where the search ended (the end, or a fence) would search the same lines + unclosedMath = next; } - const heading = /^(#{1,6})\s+(.+)$/.exec(line); + const heading = /^(#{1,6})\s+(.+)$/s.exec(line); if (heading !== null) { blocks.push({ type: "heading", level: (heading[1] ?? "#").length as 1 | 2 | 3 | 4 | 5 | 6, content: parseInline(heading[2] ?? "") }); index += 1;