FIX: Preserve pipes in Markdown table cells (#42791)

Previously, pipes inside complete Markdown links and images were treated as table column separators before inline parsing, corrupting rows and rich-editor round trips. This change protects those pipes during block parsing and escapes serialized table cells, preserving hand-authored and generated Markdown.
2026-09-05 04:40:41 -05:00 · 2026-08-25 19:47:54 +02:00
parent e5aca7217a
commit 5a117324bb
7 changed files with 155 additions and 15 deletions
@@ -1,5 +1,103 @@
+// Table rows are split before inline Markdown is parsed, so pipes inside links
+// need a same-length placeholder until block parsing has finished. Normalize
+// has already replaced source NULs before this rule runs.
+const PIPE_PLACEHOLDER = "\0";
+
+function linkEndWithPipe(inlineState, start, isImage) {
+  const { md, src } = inlineState;
+  const inlineEnd = inlineState.pos;
+  const labelMarker = start + (isImage ? 1 : 0);
+  const labelEnd = md.helpers.parseLinkLabel(
+    inlineState,
+    labelMarker,
+    !isImage
+  );
+  if (labelEnd < 0) {
+    return;
+  }
+
+  const destinationMarker = labelEnd + 1;
+  let end;
+
+  if (src[destinationMarker] === "(" && inlineEnd > destinationMarker + 1) {
+    end = inlineEnd;
+  } else if (src[destinationMarker] === "[") {
+    // Reference definitions are not collected until block parsing.
+    const referenceEnd = md.helpers.parseLinkLabel(
+      inlineState,
+      destinationMarker
+    );
+    if (referenceEnd >= 0) {
+      end = referenceEnd + 1;
+    }
+  }
+
+  return end && src.slice(start, end).includes("|") ? end : undefined;
+}
+
+function protectLinePipes(md, env, line) {
+  if (!line.includes("|") || !line.includes("[")) {
+    return line;
+  }
+
+  const inlineState = new md.inline.State(line, md, env, []);
+  let output = "";
+  let copiedUntil = 0;
+
+  while (inlineState.pos < line.length) {
+    const start = inlineState.pos;
+    const isImage = line.startsWith("![", start);
+    md.inline.skipToken(inlineState);
+
+    if (!isImage && line[start] !== "[") {
+      continue;
+    }
+
+    const end = linkEndWithPipe(inlineState, start, isImage);
+    if (end !== undefined) {
+      output += line.slice(copiedUntil, start);
+      output += line.slice(start, end).replaceAll("|", PIPE_PLACEHOLDER);
+      copiedUntil = end;
+      inlineState.pos = end;
+    }
+  }
+
+  return copiedUntil ? output + line.slice(copiedUntil) : line;
+}
+
+function protectLinkPipes(state) {
+  if (!state.src.includes("|") || !state.src.includes("[")) {
+    return;
+  }
+
+  state.src = state.src
+    .split("\n")
+    .map((line) => protectLinePipes(state.md, state.env, line))
+    .join("\n");
+}
+
+function restoreLinkPipes(state) {
+  if (!state.src.includes(PIPE_PLACEHOLDER)) {
+    return;
+  }
+
+  for (const token of state.tokens) {
+    if (token.content?.includes(PIPE_PLACEHOLDER)) {
+      token.content = token.content.replaceAll(PIPE_PLACEHOLDER, "|");
+    }
+    if (token.info?.includes(PIPE_PLACEHOLDER)) {
+      token.info = token.info.replaceAll(PIPE_PLACEHOLDER, "|");
+    }
+  }
+
+  state.src = state.src.replaceAll(PIPE_PLACEHOLDER, "|");
+}
+
 export function setup(helper) {
  helper.registerPlugin((md) => {
+    md.core.ruler.after("normalize", "protect_link_pipes", protectLinkPipes);
+    md.core.ruler.after("block", "restore_link_pipes", restoreLinkPipes);
+
    md.renderer.rules.table_open = function () {
      return '<div class="md-table">\n<table>\n';
    };
@@ -710,11 +710,15 @@ export function getCaretPosition(element, options) {
 * @return {String} Markdown table
 */
 export function arrayToTable(array, cols, colPrefix = "col", alignments) {
+  const escapeCell = (value) =>
+    String(value ?? "")
+      .replace(/\r?\n|\r/g, " ")
+      .replaceAll("|", "\\|");
+
  let table = "";

-  // Generate table headers
  table += "|";
-  table += cols.join(" | ");
+  table += cols.map(escapeCell).join(" | ");
  table += "|\n|";

  const alignMap = {
@@ -735,11 +739,7 @@ export function arrayToTable(array, cols, colPrefix = "col", alignments) {

    table +=
      cols
-        .map(function (_key, index) {
-          return String(item[`${colPrefix}${index}`] || "")
-            .replace(/\r?\n|\r/g, " ")
-            .replaceAll("|", "\\|");
-        })
+        .map((_key, index) => escapeCell(item[`${colPrefix}${index}`]))
        .join(" | ") + "|\n";
  });

@@ -131,7 +131,6 @@ const extension = {
    },
  },
  serializeNode: {
-    // TODO(renato): state.renderInline should escape `|` if `state.inTable`
    table(state, node) {
      state.flushClose(1);

@@ -145,7 +144,7 @@ const extension = {
      const prevInTable = state.inTable;
      state.inTable = true;

-      // leading newline, it seems to have issues in a line just below a > blockquote otherwise
+      // Keep a table following a blockquote from being parsed as part of it.
      if (state.out) {
        state.out += "\n";
      }
@@ -170,7 +169,11 @@ const extension = {
            }
            state.out += cellIndex === 0 ? "| " : " | ";

+            const cellStart = state.out.length;
            state.renderInline(cell);
+            state.out =
+              state.out.slice(0, cellStart) +
+              state.out.slice(cellStart).replaceAll("|", "\\|");

            if (headerBuffer !== undefined) {
              if (cell.attrs.alignment === "center") {
@@ -28,6 +28,11 @@ module(
        `<div class="md-table"><table><thead><tr><th>Line1<br>Line2</th><th>Cell 2</th></tr></thead><tbody><tr><td>Cell 3</td><td>Cell 4</td></tr></tbody></table></div>`,
        `| Line1<br>Line2 | Cell 2 |\n|----|----|\n| Cell 3 | Cell 4 |\n\n`,
      ],
+      "table with pipes inside a link": [
+        `| Link | Note |\n| --- | --- |\n| [x|y](https://example.com "title|value") | ok |`,
+        `<div class="md-table"><table><thead><tr><th>Link</th><th>Note</th></tr></thead><tbody><tr><td><a href="https://example.com" title="title|value">x|y</a></td><td>ok</td></tr></tbody></table></div>`,
+        `| Link | Note |\n|----|----|\n| [x\\|y](https://example.com "title\\|value") | ok |\n\n`,
+      ],
    }).forEach(([name, [markdown, html, expectedMarkdown]]) => {
      test(name, async function (assert) {
        await testMarkdown(assert, markdown, html, expectedMarkdown);
@@ -198,10 +198,16 @@ module("Unit | Utility | to-markdown", function (hooks) {

    html = `<table>
              <tr><th>Heading 1</th><th>Head 2</th></tr>
-              <tr><td><a href="http://example.com"><img src="http://example.com/image.png" alt="Lorem" width="45" height="45"></a></td><td>ipsum</td></tr>
+              <tr><td><a href="http://example.com"><img src="http://example.com/image|large.png" alt="Lorem" width="45" height="45" title="wide|image"></a></td><td>ipsum</td></tr>
+              <tr><td>x | y</td><td><code>a|b</code></td></tr>
+              <tr><td><a class="attachment" href="http://example.com/file|v.pdf">file.pdf</a></td><td><ruby lang="ja|latin">字</ruby></td></tr>
            </table>`;
-    markdown = `| Heading 1 | Head 2 |\n|----|----|\n| [![Lorem|45x45](http://example.com/image.png)](http://example.com) | ipsum |`;
-    assert.strictEqual(await toMarkdown(html), markdown);
+    markdown = `| Heading 1 | Head 2 |\n|----|----|\n| [![Lorem\\|45x45](http://example.com/image\\|large.png "wide\\|image")](http://example.com) | ipsum |\n| x \\| y | \`a\\|b\` |\n| [file.pdf\\|attachment](http://example.com/file\\|v.pdf) | <ruby lang="ja\\|latin">字</ruby> |`;
+    assert.strictEqual(
+      await toMarkdown(html),
+      markdown,
+      "pipes are escaped across table-cell serializer paths"
+    );
  });

  test("table with br in header is still a valid table", async function (assert) {
@@ -607,7 +607,7 @@ module("Unit | Utilities | table-builder", function (hooks) {
    );
  });

-  test("arrayToTable should escape `|`", function (assert) {
+  test("arrayToTable escapes `|` in headings and cells", function (assert) {
    const tableData = [
      {
        col0: "`a|b`",
@@ -618,8 +618,8 @@ module("Unit | Utilities | table-builder", function (hooks) {
      { col0: "1|1", col1: "2|2", col2: "3|3", col3: "4|4" },
    ];
    assert.strictEqual(
-      arrayToTable(tableData, ["Col 1", "Col 2", "Col 3", "Col 4"]),
-      "|Col 1 | Col 2 | Col 3 | Col 4|\n|--- | --- | --- | ---|\n|`a\\|b` | ![image\\|200x50](/images/discourse-logo-sketch.png) |  | \\||\n|1\\|1 | 2\\|2 | 3\\|3 | 4\\|4|\n",
+      arrayToTable(tableData, ["Col | 1", "Col 2", "Col 3", "Col 4"]),
+      "|Col \\| 1 | Col 2 | Col 3 | Col 4|\n|--- | --- | --- | ---|\n|`a\\|b` | ![image\\|200x50](/images/discourse-logo-sketch.png) |  | \\||\n|1\\|1 | 2\\|2 | 3\\|3 | 4\\|4|\n",
      "it creates a valid table"
    );
  });
@@ -2736,6 +2736,34 @@ HTML
    end
  end

+  describe "links inside tables" do
+    it "keeps pipes inside complete links and images within their cells" do
+      cooked = PrettyText.cook <<~MD
+        | Kind | Content |
+        | --- | --- |
+        | Link | [x\\]y|z](https://example.com/link) |
+        | Destination | [destination](<https://example.com/a|b> 'title|value') |
+        | Image | ![rocket|large](https://example.com/rocket.png) |
+        | Reference | [ref|label][ref] |
+
+        [ref]: https://example.com/reference
+      MD
+
+      doc = Nokogiri::HTML5.fragment(cooked)
+      expect(doc.css("tbody tr").map { |row| row.css("td").map(&:text) }).to eq(
+        [["Link", "x]y|z"], %w[Destination destination], ["Image", ""], %w[Reference ref|label]],
+      )
+      expect(doc.css("tbody a").map { |link| [link.text, link["href"], link["title"]] }).to eq(
+        [
+          ["x]y|z", "https://example.com/link", nil],
+          %w[destination https://example.com/a%7Cb title|value],
+          ["ref|label", "https://example.com/reference", nil],
+        ],
+      )
+      expect(doc.at_css("tbody img")["alt"]).to eq("rocket|large")
+    end
+  end
+
  describe "upload decoding" do
    it "can decode upload:// for default setup" do
      set_cdn_url("https://cdn.com")