mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(web): link text with markdown/HTML-significant characters survives a save (BUG-3315 U1) (#1686)
A wiki link's display text was mangled on its way through the editor, and the first save of any item linking such a title pinned the mangled text as a display override, with no rename involved (a census found 76 stored overrides of this shape among 107 that differ from their target's title). Load: escapeMarkdownLinkText escaped only brackets and backslashes, so the html-enabled parser read the title as markdown. `*real*` became emphasis, `<topic>` was dropped as a tag, `>` was decoded. It now backslash-escapes \ [ ] * _ ~ ` < > &, all ASCII punctuation CommonMark lets a backslash escape. Save: unescapeMarkdownLinkText undid only bracket and backslash escapes, while the serializer also backslash-escapes other punctuation and writes < > & as entities. It now undoes both, in one pass. One residue is intrinsic and pinned by a test: the serializer writes a literal `>` and a literal `>` as the same bytes, so a title containing entity-shaped text still becomes an override (0 of 9,620 live titles do). The materializer corpus: two expectations change (a backtick and `<b>` in link text, both this fix), taken from the goja bundle's output. A full regeneration was not used because the generator is nondeterministic: with unchanged code it rewrites all 50 cases' rows and 1 expected value. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
This commit is contained in:
+2
-2
@@ -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\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\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<table class=\"table-wrapper\" style=\"min-width: 25px;\">\n<colgroup><col style=\"min-width: 25px;\"></colgroup><tbody><tr><td colspan=\"1\" rowspan=\"1\"><ul class=\"tight\" data-tight=\"true\"><li><p>wi`t<a class=\"editor-link\" data-href=\"/-/r/other/TASK-9\">h ima</a>ge</p></li></ul><p>TAS<b>K-3]] and [[</p></td></tr></tbody>\n</table>\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<table class=\"table-wrapper\" style=\"min-width: 25px;\">\n<colgroup><col style=\"min-width: 25px;\"></colgroup><tbody><tr><td colspan=\"1\" rowspan=\"1\"><ul class=\"tight\" data-tight=\"true\"><li><p>wi`t<a class=\"editor-link\" data-href=\"/-/r/other/TASK-9\">h ima</a>ge</p></li></ul><p>TAS<b>K-3]] and [[</p></td></tr></tbody>\n</table>\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)"
|
||||
},
|
||||
{
|
||||
"name": "seed-324679",
|
||||
|
||||
@@ -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 `\\*`. `<topic>`
|
||||
// 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 <topic> topics'],
|
||||
['ampersand', 'x & y'],
|
||||
['double quotes', 'say "hi" twice'],
|
||||
['single quote', "it's done"],
|
||||
['everything at once', 'a<b> & "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 `<topic>` 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');
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
// `<topic>` 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<string, string> = { 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]
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user