diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-field.tsx b/frontend/src/components/shared/markdown-editor/markdown-editor-field.tsx index caf50f70..a26e6e66 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-field.tsx +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-field.tsx @@ -65,8 +65,10 @@ export function MarkdownEditorField({ cycleToVariable: (variable) => rawRef.current ? cycleTextareaToVariable(rawRef.current.textarea, variable) : false, focus: () => rawRef.current?.focus(), + // A disabled field must not be mutated through the handle (a variable click mid-save + // would splice the value and dirty the form); the rich branch guards on editor.isEditable. insertAtCursor: (text) => { - if (rawRef.current) { + if (!disabled && rawRef.current) { insertTextareaText(rawRef.current.textarea, text, onChange); } }, @@ -76,7 +78,7 @@ export function MarkdownEditorField({ focus: () => richRef.current?.focus(), insertAtCursor: (text) => richRef.current?.insertAtCursor(text), }, - [mode, onChange], + [mode, onChange, disabled], ); if (mode === 'raw') { diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-paste.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-paste.ts index 955f1631..1ab5bde2 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-paste.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-paste.ts @@ -20,7 +20,7 @@ const MARKDOWN_CUES = [ /^>\s/m, // blockquote ]; -const looksLikeMarkdown = (text: string): boolean => MARKDOWN_CUES.some((cue) => cue.test(text)); +const isMarkdownLike = (text: string): boolean => MARKDOWN_CUES.some((cue) => cue.test(text)); export const shouldParseMarkdownOnPaste = (text: string, html: string, isCodeContext: boolean): boolean => { if (!text.trim() || isCodeContext) { @@ -36,7 +36,7 @@ export const shouldParseMarkdownOnPaste = (text: string, html: string, isCodeCon // through the schema, so bold/italic/links survive as marks. Markdown-looking text still wins the // markdown parse: a VS Code copy of markdown source arrives wrapped in syntax-color spans, and parsing // its text/plain (not the span noise) is the point of this plugin. - if (html && !looksLikeMarkdown(text)) { + if (html && !isMarkdownLike(text)) { return false; } diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts index eb83c06f..6ffe5925 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.test.ts @@ -114,3 +114,44 @@ describe('pipe-less GFM tables (no outer pipe) — cells survive too', () => { expect(escapeTablePipes(src)).toBe(src); }); }); + +describe('table cell with a pipe inside a URL — content survives load and converges', () => { + it('escapes a pipe inside a link destination', () => { + expect(escapeTablePipes('| A | B |\n| --- | --- |\n| [x](https://h/?a=1|2) | end |')).toBe( + '| A | B |\n| --- | --- |\n| [x](https://h/?a=1\\|2) | end |', + ); + }); + + it('escapes a pipe inside a bare autolink and an image src', () => { + expect(escapeTablePipes('| A | B |\n| --- | --- |\n| https://h/?a=1|2 | ![p](https://c/i.png?w=1|2) |')).toBe( + '| A | B |\n| --- | --- |\n| https://h/?a=1\\|2 | ![p](https://c/i.png?w=1\\|2) |', + ); + }); + + it('leaves a real column delimiter (spaced pipe, no scheme run) untouched', () => { + const table = '| A | B |\n| --- | --- |\n| http://h/x | plain |'; + + expect(escapeTablePipes(table)).toBe(table); + }); + + it('keeps the URL and the trailing cell on round-trip, and converges', () => { + const out = roundTrip('| A | B |\n| --- | --- |\n| [go](https://h/?x=1|2) | TRAILING |'); + + expect(out).toContain('TRAILING'); + expect(out).toContain('x=1\\|2'); + expect(roundTrip(out)).toBe(out); + }); +}); + +describe('TABLE_DELIMITER_LINE is linear (ReDoS guard)', () => { + it('scans a crafted delimiter-looking line with a long trailing space run in linear time', () => { + // A `|`-line followed by "dashes + many spaces + non-matching tail" was O(n²) on the old regex + // (~0.6s at 32k). The linear rewrite stays sub-millisecond; assert a generous budget. + const evil = `x|y\n${'-'.repeat(50)}${' '.repeat(60000)}z\n`; + const started = performance.now(); + + escapeTablePipes(evil); + + expect(performance.now() - started).toBeLessThan(100); + }); +}); diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts index b6126a2d..39f2b7b7 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-table-pipes.ts @@ -1,8 +1,8 @@ // marked's GFM table tokenizer splits every row on raw `|` BEFORE inline tokenization, so a pipe inside a -// code span (`` `x | y` ``) or a Go-template action (`{{.X | upper}}`) in a cell creates a phantom -// column and the trailing cells are silently DROPPED on load. The splitter does honor `\|` — and unescapes -// it in the cell text — so pre-escaping those pipes before marked lexes protects the load side the same way -// TunedTable's renderChildren escape protects the save side. +// code span (`` `x | y` ``), a Go-template action (`{{.X | upper}}`), or a URL (`[x](http://a|b)`, a bare +// `http://a|b`, an image src) in a cell creates a phantom column and the trailing cells are silently DROPPED +// on load. The splitter does honor `\|` — and unescapes it in the cell text — so pre-escaping those pipes +// before marked lexes protects the load side the same way TunedTable's renderChildren escape protects save. // // Scope must match EXACTLY the rows marked itself treats as the table — no more, no less: // • the header/delimiter pair must already parse as a table (matching cell counts, delimiter has |/:) — @@ -18,8 +18,14 @@ // content, and our html tokenizers render such lines as literal text anyway). const FENCE_LINE = /^ {0,3}(```|~~~)/; -const TABLE_DELIMITER_LINE = /^ {0,3}\|? *:?-+:? *(?:\| *:?-+:? *)*\|? *$/; +// Written to be linear: the trailing `(?: *\|)? *$` (not `\|? *$`) plus per-cell spacing keep any two space +// runs from competing for the same characters, so a crafted delimiter-looking line can't force O(n²) backtracking. +const TABLE_DELIMITER_LINE = /^ {0,3}\|? *:?-+:?(?: *\| *:?-+:?)*(?: *\|)? *$/; const TEMPLATE_ACTION = /\{\{[^{}]*\}\}/g; +// A maximal non-space run containing a `scheme://` — a link/image destination or a bare autolink. It has no +// spaces by construction, so any `|` in it is URL content (never a real cell separator, which needs spaces +// around it or sits outside the run), and escaping it is always correct. +const URL_RUN = /[^\s]*:\/\/[^\s]*/g; // A table's body ends at a blank line or the first line that starts a different block — the same interrupts // marked's gfmTable body-row negative lookahead lists (heading, blockquote, fences, list, hr, indented code). @@ -33,7 +39,7 @@ const ENDS_TABLE_BODY = [ /^(?: {4}| {0,3}\t)/, ]; -const endsTableBody = (line: string): boolean => ENDS_TABLE_BODY.some((rule) => rule.test(line)); +const isTableBodyEnd = (line: string): boolean => ENDS_TABLE_BODY.some((rule) => rule.test(line)); const isEscapedAt = (text: string, offset: number): boolean => { let isEscaped = false; @@ -121,7 +127,9 @@ const escapeRowPipes = (row: string): string => { index = closerStart + run.length; } - return result.replace(TEMPLATE_ACTION, (action) => escapeUnescapedPipes(action)); + return result + .replace(TEMPLATE_ACTION, (action) => escapeUnescapedPipes(action)) + .replace(URL_RUN, (url) => escapeUnescapedPipes(url)); }; export const escapeTablePipes = (markdown: string): string => { @@ -170,7 +178,7 @@ export const escapeTablePipes = (markdown: string): string => { // content. escapeRowPipes leaves structural pipes alone, so escaping the header can't skew its cell count. escapeRow(index); - for (let row = index + 2; row < lines.length && !endsTableBody(lines[row]!); row++) { + for (let row = index + 2; row < lines.length && !isTableBodyEnd(lines[row]!); row++) { escapeRow(row); } } diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor-textarea.ts b/frontend/src/components/shared/markdown-editor/markdown-editor-textarea.ts index 2e7d591d..46f92ade 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor-textarea.ts +++ b/frontend/src/components/shared/markdown-editor/markdown-editor-textarea.ts @@ -41,20 +41,20 @@ export function insertTextareaText( // Pixel offset of `position` from the textarea content top, measured via a hidden mirror so soft-wrapped // lines count — a logical-line count undershoots the scroll badly for wrapped templates. function caretOffsetTop(textarea: HTMLTextAreaElement, position: number): number { - const cs = getComputedStyle(textarea); + const computedStyle = getComputedStyle(textarea); const mirror = document.createElement('div'); - mirror.style.fontFamily = cs.fontFamily; - mirror.style.fontSize = cs.fontSize; - mirror.style.fontWeight = cs.fontWeight; - mirror.style.fontStyle = cs.fontStyle; - mirror.style.lineHeight = cs.lineHeight; - mirror.style.letterSpacing = cs.letterSpacing; - mirror.style.wordSpacing = cs.wordSpacing; - mirror.style.paddingTop = cs.paddingTop; - mirror.style.paddingRight = cs.paddingRight; - mirror.style.paddingBottom = cs.paddingBottom; - mirror.style.paddingLeft = cs.paddingLeft; + mirror.style.fontFamily = computedStyle.fontFamily; + mirror.style.fontSize = computedStyle.fontSize; + mirror.style.fontWeight = computedStyle.fontWeight; + mirror.style.fontStyle = computedStyle.fontStyle; + mirror.style.lineHeight = computedStyle.lineHeight; + mirror.style.letterSpacing = computedStyle.letterSpacing; + mirror.style.wordSpacing = computedStyle.wordSpacing; + mirror.style.paddingTop = computedStyle.paddingTop; + mirror.style.paddingRight = computedStyle.paddingRight; + mirror.style.paddingBottom = computedStyle.paddingBottom; + mirror.style.paddingLeft = computedStyle.paddingLeft; mirror.style.width = `${textarea.clientWidth}px`; mirror.style.boxSizing = 'border-box'; mirror.style.whiteSpace = 'pre-wrap'; diff --git a/frontend/src/components/shared/markdown-editor/markdown-editor.tsx b/frontend/src/components/shared/markdown-editor/markdown-editor.tsx index bad8ecc9..f350a4b0 100644 --- a/frontend/src/components/shared/markdown-editor/markdown-editor.tsx +++ b/frontend/src/components/shared/markdown-editor/markdown-editor.tsx @@ -163,7 +163,10 @@ function MarkdownEditor({ editor?.commands.focus(); }, insertAtCursor: (text: string) => { - if (!editor) { + // A disabled field must not be mutated through the handle — `dispatch` bypasses the editable + // gate (which only blocks user input), so without this a variable click mid-save would edit + // the doc and dirty the form. + if (!editor || !editor.isEditable) { return; }