Escape angle brackets in attachment JSON so pasted attachments survive DOMPurify

action_text-trix/app/assets/javascripts/trix.js

@@ -1333,6 +1333,19 @@ $\
       offset: leftIndex
     };
   };
+  const angleBracketEscapes = {
+    "<": "\\u003c",
+    ">": "\\u003e"
+  };
+
+  // Escapes "<" and ">" in JSON text as "\u003c" and "\u003e". In JSON they can only occur
+  // inside string literals, where the escapes spell the same characters, so JSON.parse reads
+  // the result back to the same value.
+  //
+  // This keeps sequences such as "</style>", "-->" and "]]>" out of the HTML attributes Trix
+  // stores JSON in: DOMPurify's SAFE_FOR_XML mode drops any attribute containing one, and it
+  // does so before honoring the hook that keeps data-trix-* attributes.
+  const escapeAngleBracketsInJSON = json => json.replace(/[<>]/g, bracket => angleBracketEscapes[bracket]);
 
   class Hash extends TrixObject {
     static fromCommonAttributesOfObjects() {
@@ -4249,29 +4262,20 @@ $\
   var purify = createDOMPurify();
 
   const ALLOWED_ATTRIBUTE_PATTERN = /^data-trix-/;
-
-  // DOMPurify's SAFE_FOR_XML check drops attributes whose values contain markup before it
-  // honors forceKeepAttr, so allowed attributes are stashed here and restored afterwards.
-  let stashedAttributes = [];
   purify.addHook("uponSanitizeAttribute", function (node, data) {
     if (data.attrName === "data-trix-serialized-attributes") {
       data.keepAttr = false;
       return;
     }
+
+    // SAFE_FOR_XML drops an attribute whose value carries a raw-text closing sequence before
+    // forceKeepAttr is honored. sanitizeElement escapes those brackets in the JSON attachment
+    // attributes first, so only a value that isn't JSON is left for SAFE_FOR_XML to remove.
     if (ALLOWED_ATTRIBUTE_PATTERN.test(data.attrName)) {
       data.forceKeepAttr = true;
-      stashedAttributes.push([data.attrName, node.getAttribute(data.attrName)]);
     }
   });
-  purify.addHook("afterSanitizeAttributes", function (node) {
-    stashedAttributes.forEach(_ref => {
-      let [name, value] = _ref;
-      if (value !== null && !node.hasAttribute(name)) {
-        node.setAttribute(name, value);
-      }
-    });
-    stashedAttributes = [];
-  });
+  const JSON_ATTRIBUTES = "data-trix-attachment data-trix-attributes".split(" ");
   const DEFAULT_ALLOWED_ATTRIBUTES = "style href src width height language class".split(" ");
   const DEFAULT_FORBIDDEN_PROTOCOLS = "javascript:".split(" ");
   const DEFAULT_FORBIDDEN_ELEMENTS = "script iframe form noscript".split(" ");
@@ -4344,14 +4348,27 @@ $\
           element.removeAttribute("href");
         }
       }
-      Array.from(element.attributes).forEach(_ref2 => {
+      Array.from(element.attributes).forEach(_ref => {
         let {
           name
-        } = _ref2;
+        } = _ref;
         if (!this.allowedAttributes.includes(name) && name.indexOf("data-trix") !== 0) {
           element.removeAttribute(name);
         }
       });
+
+      // HTML from older Trix versions, server-side renderers and stored content carries the
+      // JSON with literal angle brackets, and SAFE_FOR_XML drops any attribute whose value
+      // contains "</style>" or another raw-text closing sequence. Escaping the brackets before
+      // DOMPurify sees the value leaves it nothing to drop, and JSON.parse reads the same value
+      // back. A value that doesn't parse is left for SAFE_FOR_XML to remove: HTMLParser ignores
+      // it either way, and rewriting it could only turn it into something that parses.
+      JSON_ATTRIBUTES.forEach(name => {
+        const value = element.getAttribute(name);
+        if (value && parsesAsJSON(value)) {
+          element.setAttribute(name, escapeAngleBracketsInJSON(value));
+        }
+      });
       return element;
     }
     normalizeListElementNesting() {
@@ -4376,12 +4393,40 @@ $\
       return element.getAttribute("data-trix-serialize") === "false" && !nodeIsAttachmentElement(element);
     }
   }
+  const parsesAsJSON = string => {
+    try {
+      JSON.parse(string);
+      return true;
+    } catch (error) {
+      return false;
+    }
+  };
+  const CLOSING_HTML_TAG_PATTERN = /<\/html(?=[\t\n\f\r />])/gi;
+
+  // Windows browsers can paste clipboard bytes after the closing </html> tag, and the HTML
+  // parser would append them to the body as text.
+  const removeContentAfterClosingHTMLTag = function (html) {
+    const offset = html.search(CLOSING_HTML_TAG_PATTERN) < 0 ? -1 : offsetOfClosingHTMLTag(html);
+    return offset < 0 ? html : html.slice(0, offset);
+  };
+
+  // The browser's own tokenizer decides which "</html>" is the closing tag: each one is
+  // swapped for a marker start tag and the string parsed, and the first marker that comes out
+  // as an element was a real tag rather than text inside an attribute value, a comment or a
+  // style element. The marker's name carries a token chosen per call, so no element in the
+  // input can pass for one, and its offset is an unquoted attribute value, so that wherever
+  // the marker lands it carries nothing that would change the tokenizer's state there.
+  const offsetOfClosingHTMLTag = function (html) {
+    const marker = "trix-closing-html-tag-".concat(Math.random().toString(36).slice(2));
+    const doc = document.implementation.createHTMLDocument("");
+    doc.documentElement.innerHTML = html.replace(CLOSING_HTML_TAG_PATTERN, (tag, offset) => "<".concat(marker, " data-offset=").concat(offset));
+    const offsets = Array.from(doc.querySelectorAll(marker), element => parseInt(element.getAttribute("data-offset"), 10));
+    return offsets.length ? offsets.reduce((lowest, offset) => Math.min(lowest, offset)) : -1;
+  };
   const createBodyElementForHTML = function () {
     let html = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : "";
-    // Remove everything after </html>
-    html = html.replace(/<\/html[^>]*>[^]*$/i, "</html>");
     const doc = document.implementation.createHTMLDocument("");
-    doc.documentElement.innerHTML = html;
+    doc.documentElement.innerHTML = removeContentAfterClosingHTMLTag(html);
     Array.from(doc.head.querySelectorAll("style")).forEach(element => {
       doc.body.appendChild(element);
     });
@@ -4496,15 +4541,15 @@ $\
     }
     getData() {
       const data = {
-        trixAttachment: JSON.stringify(this.attachment),
+        trixAttachment: toJSONAttribute(this.attachment),
         trixContentType: this.attachment.getContentType(),
         trixId: this.attachment.id
       };
       const {
         attributes
       } = this.attachmentPiece;
       if (!attributes.isEmpty()) {
-        data.trixAttributes = JSON.stringify(attributes);
+        data.trixAttributes = toJSONAttribute(attributes);
       }
       if (this.attachment.isPending()) {
         data.trixSerialize = false;
@@ -4551,6 +4596,11 @@ $\
       trixSerialize: false
     }
   });
+
+  // Attachment JSON is emitted with angle brackets escaped so that the HTML Trix produces
+  // survives being pasted back into Trix, whose insertHTML parses under DOMPurify's
+  // SAFE_FOR_XML mode.
+  const toJSONAttribute = object => escapeAngleBracketsInJSON(JSON.stringify(object));
   const htmlContainsTagName = function (html, tagName) {
     const div = makeElement("div");
     HTMLSanitizer.setHTML(div, html || "");

src/test/system/pasting_test.js

@@ -4,10 +4,12 @@ import { OBJECT_REPLACEMENT_CHARACTER } from "trix/constants"
 import {
   TEST_IMAGE_URL,
   assert,
+  attachmentHTML,
   clickToolbarButton,
   createFile,
   expandSelection,
   expectDocument,
+  insertText,
   moveCursor,
   pasteContent,
   pressKey,
@@ -18,6 +20,8 @@ import {
   typeCharacters,
 } from "test/test_helper"
 import { delay, nextFrame } from "../test_helpers/timing_helpers"
+import Attachment from "trix/models/attachment"
+import Text from "trix/models/text"
 
 testGroup("Pasting", { template: "editor_empty" }, () => {
   test("paste plain text", async () => {
@@ -191,6 +195,34 @@ testGroup("Pasting", { template: "editor_empty" }, () => {
     assert.notOk(img.hasAttribute("onerror"), "img should not have an onerror attribute")
   })
 
+  test("paste Trix's own HTML for an attachment whose content and caption close a style tag", async () => {
+    const content = "<style>.quoted { color: red }</style><p>quoted mail</p>"
+    const attachment = new Attachment({ content, contentType: "text/html" })
+    insertText(Text.textForAttachmentWithAttributes(attachment, { caption: "</style>" }))
+    await nextFrame()
+
+    await pasteContent("text/html", getEditorElement().value)
+    const pieces = getDocument().getAttachmentPieces()
+
+    assert.equal(pieces.length, 2, "pasted attachment was dropped")
+    assert.equal(pieces[1].attachment.getContent(), content)
+    assert.equal(pieces[1].getCaption(), "</style>")
+    expectDocument(`${OBJECT_REPLACEMENT_CHARACTER}${OBJECT_REPLACEMENT_CHARACTER}\n`)
+  })
+
+  test("paste stored HTML for an attachment whose content and caption close style and html tags", async () => {
+    const content = "<html><body><style>.quoted { color: red }</style><p>quoted mail</p></body></html>"
+    const html = attachmentHTML({ content, contentType: "text/html" }, { caption: "</style>" })
+
+    await pasteContent("text/html", `copy${html}me`)
+    const [ piece ] = getDocument().getAttachmentPieces()
+
+    assert.ok(piece, "pasted attachment was dropped")
+    assert.equal(piece.attachment.getContent(), content)
+    assert.equal(piece.getCaption(), "</style>")
+    expectDocument(`copy${OBJECT_REPLACEMENT_CHARACTER}me\n`)
+  })
+
   test("prefers plain text when html lacks formatting", async () => {
     const pasteData = {
       "text/html": "<meta charset='utf-8'>a\nb",

src/test/test_helpers/editor_helpers.js

@@ -58,3 +58,16 @@ export const replaceDocument = function (document) {
 
 
 const render = () => getEditorController().render()
+
+// Attachment markup as stored content and server-side renderers emit it: the JSON is
+// escaped only as far as an attribute value needs, so any angle brackets in it are
+// literal. (Browsers that escape angle brackets when serializing attributes would hide
+// that shape, so this doesn't go through outerHTML.)
+export const attachmentHTML = function (attachment, attributes) {
+  const attributeHTML = (name, value) =>
+    ` ${name}="${JSON.stringify(value).replace(/&/g, "&").replace(/"/g, """)}"`
+
+  const attributesHTML = attributes ? attributeHTML("data-trix-attributes", attributes) : ""
+
+  return `<figure${attributeHTML("data-trix-attachment", attachment)}${attributesHTML}></figure>`
+}

src/test/test_helpers/fixtures/fixtures.js

@@ -477,12 +477,13 @@ export const fixtures = {
     const attachment = new Attachment({ content, contentType, href })
     const text = Text.textForAttachmentWithAttributes(attachment)
 
+    // Mirrors what AttachmentView emits: angle brackets in the JSON are escaped.
     const figure = makeElement({
       tagName: "figure",
       className: "attachment attachment--content",
       editable: false,
       data: {
-        trixAttachment: JSON.stringify(attachment),
+        trixAttachment: JSON.stringify(attachment).replace(/</g, "\\u003c").replace(/>/g, "\\u003e"),
         trixContentType: contentType,
         trixId: attachment.id,
       },

src/test/unit.js

@@ -6,6 +6,7 @@ import "test/unit/document_test"
 import "test/unit/document_json_deserialization_test"
 import "test/unit/document_view_test"
 import "test/unit/helpers/custom_elements_test"
+import "test/unit/helpers/strings_test"
 import "test/unit/html_parser_test"
 import "test/unit/html_sanitizer_test"
 import "test/unit/location_mapper_test"

src/test/unit/document_view_test.js

@@ -1,9 +1,32 @@
 import { assert, eachFixture, test, testGroup } from "test/test_helper"
 
+import Attachment from "trix/models/attachment"
+import Block from "trix/models/block"
+import Document from "trix/models/document"
+import DocumentView from "trix/views/document_view"
+import Text from "trix/models/text"
+
 testGroup("DocumentView", () => {
   eachFixture((name, details) => {
     test(name, () => {
       assert.documentHTMLEqual(details.document, details.html)
     })
   })
+
+  // Pasting the rendered HTML back into Trix parses it under DOMPurify's SAFE_FOR_XML
+  // mode, which drops any attribute whose value contains "</style>".
+  test("renders attachment JSON without angle brackets", () => {
+    const content = "<style>p { color: red }</style><p>quoted mail</p>"
+    const attachment = new Attachment({ content, contentType: "text/html" })
+    const text = Text.textForAttachmentWithAttributes(attachment, { caption: "</style>" })
+    const figure = DocumentView.render(new Document([ new Block(text) ])).querySelector("figure")
+
+    const attachmentJSON = figure.getAttribute("data-trix-attachment")
+    const attributesJSON = figure.getAttribute("data-trix-attributes")
+
+    assert.notOk(/[<>]/.test(attachmentJSON), "raw angle brackets in attachment JSON")
+    assert.notOk(/[<>]/.test(attributesJSON), "raw angle brackets in attributes JSON")
+    assert.equal(JSON.parse(attachmentJSON).content, content)
+    assert.equal(JSON.parse(attributesJSON).caption, "</style>")
+  })
 })

src/test/unit/helpers/strings_test.js

@@ -0,0 +1,36 @@
+import { assert, test, testGroup } from "test/test_helper"
+import { escapeAngleBracketsInJSON } from "trix/core/helpers"
+
+testGroup("Helpers: Strings", () => {
+  testGroup("escapeAngleBracketsInJSON", () => {
+    test("escapes every angle bracket", () => {
+      const json = JSON.stringify({ content: "<style>a</style><!-- b --><![CDATA[c]]>", caption: "<" })
+      const escaped = escapeAngleBracketsInJSON(json)
+
+      assert.notOk(/[<>]/.test(escaped), "raw angle brackets left in: " + escaped)
+      assert.equal(escaped, "{\"content\":\"\\u003cstyle\\u003ea\\u003c/style\\u003e\\u003c!-- b --\\u003e\\u003c![CDATA[c]]\\u003e\",\"caption\":\"\\u003c\"}")
+    })
+
+    test("parses back to the same value", () => {
+      const values = [
+        { content: "<style>a</style>" },
+        { content: "\\<", caption: "\\\\>" },
+        { content: "\u003c\u003e", caption: "\\u003c" },
+        { content: "<\ud83d\ude00>", caption: "\"<\"" },
+        { nested: [ "<", { deeper: [ ">" ] } ], number: 1, flag: true, nothing: null },
+      ]
+
+      values.forEach((value) => {
+        const json = JSON.stringify(value)
+        assert.deepEqual(JSON.parse(escapeAngleBracketsInJSON(json)), value, json)
+      })
+    })
+
+    test("is idempotent", () => {
+      const json = JSON.stringify({ content: "<style>a</style>" })
+      const escaped = escapeAngleBracketsInJSON(json)
+
+      assert.equal(escapeAngleBracketsInJSON(escaped), escaped)
+    })
+  })
+})

src/test/unit/html_parser_test.js

@@ -1,6 +1,7 @@
 import {
   TEST_IMAGE_URL,
   assert,
+  attachmentHTML,
   createCursorTarget,
   eachFixture,
   fixtures,
@@ -10,6 +11,7 @@ import {
 } from "test/test_helper"
 
 import * as config from "trix/config"
+import { OBJECT_REPLACEMENT_CHARACTER } from "trix/constants"
 import HTMLParser from "trix/models/html_parser"
 import { delay } from "../test_helpers/timing_helpers"
 
@@ -302,6 +304,39 @@ testGroup("HTMLParser", () => {
     assert.equal(document.getAttachmentPieces().length, 1)
   })
 
+  test("parses attachment whose content closes a style tag when pasting", () => {
+    const content = "<style>p { color: red }</style><p>quoted mail</p>"
+    const html = attachmentHTML({ contentType: "text/html", content }, { caption: "</style>" })
+    const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
+    const [ piece ] = document.getAttachmentPieces()
+
+    assert.ok(piece, "attachment was dropped")
+    assert.equal(piece.attachment.getContent(), content)
+    assert.equal(piece.getCaption(), "</style>")
+  })
+
+  test("parses attachment whose content closes an html tag when pasting", () => {
+    const content = "<html><body><p>quoted mail</p></body></html>"
+    const html = `<div>before</div>${attachmentHTML({ contentType: "text/html", content })}<div>after</div>`
+    const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
+    const [ attachment ] = document.getAttachments()
+
+    assert.ok(attachment, "attachment was dropped")
+    assert.equal(attachment.getContent(), content)
+    assert.equal(document.toString(), `before\n${OBJECT_REPLACEMENT_CHARACTER}\nafter\n`)
+  })
+
+  test("parses attachment whose content nests an attachment closing a style tag when pasting", () => {
+    const inner = attachmentHTML({ contentType: "text/html", content: "<style>p { color: red }</style><p>quoted</p>" })
+    const content = `<p>reply</p>${inner}`
+    const html = attachmentHTML({ contentType: "text/html", content })
+    const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
+    const [ attachment ] = document.getAttachments()
+
+    assert.ok(attachment, "attachment was dropped")
+    assert.equal(attachment.getContent(), content)
+  })
+
   test("parses attachment caption from large html string", () => {
     let { html } = fixtures["image attachment with edited caption"]