diff --git a/internal/materialize/testdata/corpus.json b/internal/materialize/testdata/corpus.json index 7982b222a..8d02c75be 100644 --- a/internal/materialize/testdata/corpus.json +++ b/internal/materialize/testdata/corpus.json @@ -192,7 +192,7 @@ } ], "workspace_slug": "ws", - "expected": "title: \"Synthetic\"\\\ntags:` [a, `b\\]\n\n1. \n2. second\n3. third\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)\n\n![diagram](pad-attachment:5c6d7e8f-9a0b-4c1d-8e2f-3a4b5c6d7e8f)\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)\n\nParagr\n\nParagraph li\\\nnking [[other::TASK-9|shown]] \n\n- i~~nlin~~g [external](https://example.com/a?b=c) inline.\n\nPpng [[Legacy\\\\` https://x.tes]][[t/pd]][[oc]] i\\[[other::TASK-9|nline.\n\nParagraph li\\|nking [shown]][[ i]]nlin\\\ne.\n\nParagraph linking [[PLAN-7|custom text]] inline." + "expected": "title: \"Synthetic\"\\\ntags:` [a, `b\\]\n\n1. \n2. second\n3. third\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)\n\n![diagram](pad-attachment:5c6d7e8f-9a0b-4c1d-8e2f-3a4b5c6d7e8f)\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)\n\nParagr\n\nParagraph li\\\nnking [[other::TASK-9|shown]] \n\n- i~~nlin~~g [external](https://example.com/a?b=c) inline.\n\nPpng [[Legacy` https://x.tes]][[t/pd]][[oc]] i\\[[other::TASK-9|nline.\n\nParagraph li\\|nking [shown]][[ i]]nlin\\\ne.\n\nParagraph linking [[PLAN-7|custom text]] inline." }, { "name": "seed-31676", @@ -2326,7 +2326,7 @@ } ], "workspace_slug": "ws", - "expected": "| a | b |\n| --- | --- |\n| 1 | 2 |\n| x | |\n\ninking [[PLAN-7|custom text]] inline.\n\n```go\nfunc main() {\n\tprintln(\"hi\")\n}\n```\n\nParag|raph linking [[other::TASK-9|shown]] inl\\\nine.\n\n| a | b |\n| --- | --- |\n| 1 | 2 |\n| x | y |\n\n> #quoted line emoji 🎉second line\n\n---\n\n> quoted line\n> second line\n\n# He\\\nading one\n\n\n\n

TAS<b>K-3]] and [[

\n\n> Paragr\\\n> aph linking [[Legac<b>y doc]] inline.\n\nParagraph linking [[other::TASK-9]] inline.\n\nRoadma\n\n~~p\\]\\]~~\n\n#Paragraph linking [unknown](/alice/ws/tasks/TASK-999) inline.\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)" + "expected": "| a | b |\n| --- | --- |\n| 1 | 2 |\n| x | |\n\ninking [[PLAN-7|custom text]] inline.\n\n```go\nfunc main() {\n\tprintln(\"hi\")\n}\n```\n\nParag|raph linking [[other::TASK-9|shown]] inl\\\nine.\n\n| a | b |\n| --- | --- |\n| 1 | 2 |\n| x | y |\n\n> #quoted line emoji 🎉second line\n\n---\n\n> quoted line\n> second line\n\n# He\\\nading one\n\n\n\n

TAS<b>K-3]] and [[

\n\n> Paragr\\\n> aph linking [[Legacy doc]] inline.\n\nParagraph linking [[other::TASK-9]] inline.\n\nRoadma\n\n~~p\\]\\]~~\n\n#Paragraph linking [unknown](/alice/ws/tasks/TASK-999) inline.\n\n[report.pdf](pad-attachment:a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d)" }, { "name": "seed-324679", diff --git a/web/src/lib/utils/markdown.linkTextEscaping.svelte.test.ts b/web/src/lib/utils/markdown.linkTextEscaping.svelte.test.ts new file mode 100644 index 000000000..df9890fdd --- /dev/null +++ b/web/src/lib/utils/markdown.linkTextEscaping.svelte.test.ts @@ -0,0 +1,110 @@ +import { describe, it, expect } from 'vitest'; +import { createHeadlessEditor, flushPipeline } from '$lib/collab/materializer/entry'; +import { wikiLinksToMarkdown } from './markdown'; +import type { Item } from '$lib/types'; + +// BUG-3315 U1: a wiki link whose text contains a character the markdown pipeline +// escapes or parses must survive a tab's load-and-save UNCHANGED. +// +// This runs through the REAL shared pipeline, the functions both a tab's flush +// and the server materializer (TASK-2198) use: wikiLinksToMarkdown at load, the +// headless editor with the live extension set, then flushPipeline +// (markdownToWikiLinks + cleanBrokenLinks) at save. +// +// It was red before the fix, in both directions: +// - An AUTO link, stored as [[TASK-1]] and following the title, came back as a +// mangled OVERRIDE: `a > b` as [[TASK-1|a > b]], `*` as `\\*`. `` +// was DROPPED at load, because the html-enabled parser read it as a tag. The +// first save of any item linking such a title pinned the mangled text, with +// no rename involved. A census found 76 stored overrides of this shape. +// - A DELIBERATE override containing the same characters was rewritten on save. +// The `&` and quote rows passed before the fix too, and stay as controls: the +// load escapes `&` now, and they pin that the escape does not break them. +// Reverting only the load half fails 7 of these legs; reverting only the flush +// half fails 15. + +const OTHER_TITLE = 'Some unrelated current title'; + +function item(title: string): Item { + return { + id: 'id-1', + title, + slug: 'fixture', + collection_slug: 'tasks', + collection_prefix: 'TASK', + item_number: 1 + } as unknown as Item; +} + +// One tab session: load the stored body with `title` current, save it back. +function loadAndSave(stored: string, title: string): { text: string; flushed: string } { + const ed = createHeadlessEditor(); + try { + ed.commands.setContent(wikiLinksToMarkdown(stored, [item(title)], 'ws', 'u')); + const md = (ed.storage as { markdown: { getMarkdown(): string } }).markdown.getMarkdown(); + return { text: ed.getText(), flushed: flushPipeline(md, [item(title)] as never) }; + } finally { + ed.destroy(); + } +} + +const titles: Array<[string, string]> = [ + ['greater-than', 'a > b'], + ['less-than', 'a < b'], + ['asterisk', 'on /auth/* endpoints'], + ['emphasis-shaped asterisks', 'the *real* one'], + ['html-tag-shaped text', 'pad help topics'], + ['ampersand', 'x & y'], + ['double quotes', 'say "hi" twice'], + ['single quote', "it's done"], + ['everything at once', 'a & "c" *d* > e'], +]; + +describe('BUG-3315 U1: link text with markdown/HTML-significant characters survives a save', () => { + for (const [name, title] of titles) { + it(`auto link follows the title (${name})`, () => { + const { text, flushed } = loadAndSave('see [[TASK-1]] here', title); + // The load must render the whole title; a dropped `` fails here. + expect(text).toBe(`see ${title} here`); + expect(flushed).toBe('see [[TASK-1]] here'); + }); + + it(`deliberate override is kept byte-identical (${name})`, () => { + const stored = `see [[TASK-1|${title}]] here`; + const { text, flushed } = loadAndSave(stored, OTHER_TITLE); + expect(text).toBe(`see ${title} here`); + expect(flushed).toBe(stored); + }); + } + + // The two legs above could both pass with an override that is merely STABLE + // rather than correct, so a second save of the first save must also change + // nothing. + it('a second save is a fixed point', () => { + for (const [, title] of titles) { + const once = loadAndSave(`see [[TASK-1|${title}]] and [[TASK-1]]`, OTHER_TITLE).flushed; + expect(loadAndSave(once, OTHER_TITLE).flushed).toBe(once); + } + }); + + // KNOWN LIMITATION, pinned rather than hidden. The serializer writes a literal + // `>` and a literal `>` as the same bytes, so after a save no reading of the + // markdown can tell them apart, and the flush takes the decoded one. A title + // containing entity-SHAPED text therefore still becomes an override. The load is + // correct, and the second save is a fixed point. Receipt: 0 of 9,620 live titles + // contained `<` / `>` / `&` / `"` / `'` when this was written, + // against 1,304 containing one of < > & * _ ~ `. + it('entity-shaped title text: load is exact, save still overrides (limitation)', () => { + const title = 'literally > here'; + const auto = loadAndSave('see [[TASK-1]] here', title); + expect(auto.text).toBe(`see ${title} here`); + expect(auto.flushed).toBe('see [[TASK-1|literally > here]] here'); + expect(loadAndSave(auto.flushed, title).flushed).toBe(auto.flushed); + }); + + // Control: text that genuinely differs from the title is still an override. + it('an edited display text still becomes an override', () => { + const { flushed } = loadAndSave('see [[TASK-1|a > c]] here', 'a > b'); + expect(flushed).toBe('see [[TASK-1|a > c]] here'); + }); +}); diff --git a/web/src/lib/utils/markdown.ts b/web/src/lib/utils/markdown.ts index d2ece961f..c7fc901ee 100644 --- a/web/src/lib/utils/markdown.ts +++ b/web/src/lib/utils/markdown.ts @@ -878,11 +878,15 @@ export function markdownToWikiLinks(markdown: string, items: Item[]): string { }); } -// Escape the characters that would terminate or unbalance a markdown link's -// text span. `\` must be doubled first so it doesn't interfere with the -// subsequent bracket escapes. +// Escape a link's display text so the editor's markdown parser reads it as +// LITERAL text. Brackets would terminate or unbalance the span, and the rest are +// the characters its inline rules act on (BUG-3315): `*` `_` and `~` become +// emphasis or strikethrough, `` ` `` code, `<` an HTML tag (html is enabled, so +// `` was dropped), and `&` an entity (`>` was decoded). Every one of +// them is ASCII punctuation, which CommonMark lets a backslash escape anywhere. +// A single pass, so an escape's own backslash is never escaped again. function escapeMarkdownLinkText(s: string): string { - return s.replace(/\\/g, '\\\\').replace(/([\[\]])/g, '\\$1'); + return s.replace(/[\\[\]*_~`<>&]/g, '\\$&'); } // Escape the characters that would terminate a [[...]] wiki-link body, or @@ -915,11 +919,20 @@ function splitWikiBody(body: string): { key: string; displayOverride: string | n return { key: body, displayOverride: null }; } -// Inverse of escapeMarkdownLinkText. Also undoes the \[\[ / \]\] escapes that -// tiptap-markdown inserts to prevent its own output from looking like our -// wiki-link sentinels. +// Recover a link's literal display text from the SERIALIZER's output. That is not +// the inverse of escapeMarkdownLinkText, because the serializer escapes its own +// way. It backslash-escapes ASCII punctuation (including the \[\[ / \]\] it +// inserts so its output never looks like our wiki-link sentinels), and writes +// `<`, `>` and `&` as entities. Undoing only the bracket escapes, as this used to, +// left `>` and `\*` in the text, so a title containing them never compared +// equal to itself, and the first save of any item linking one pinned the mangled +// text as an override (BUG-3315). A single pass, so `\>` (a literal `>`) +// is not decoded twice. +const LINK_TEXT_ENTITIES: Record = { lt: '<', gt: '>', amp: '&', quot: '"', '#39': "'" }; function unescapeMarkdownLinkText(s: string): string { - return s.replace(/\\(\[|\]|\\)/g, '$1'); + return s.replace(/\\([!-/:-@[-`{-~])|&(lt|gt|amp|quot|#39);/g, (_m, punct: string | undefined, ent: string | undefined) => + punct ?? LINK_TEXT_ENTITIES[ent as string] + ); } /**