Escape angle brackets in attachment JSON so pasted attachments survive DOMPurify

Copying an attachment out of Trix and pasting it back silently dropped it
whenever the attachment JSON contained "</style>" or another sequence
DOMPurify's SAFE_FOR_XML mode treats as a raw-text or comment terminator.
Paste runs the clipboard's text/html through HTMLParser under
SAFE_FOR_XML, and DOMPurify's attribute rule removes the whole
data-trix-attachment attribute on a match, before the forceKeepAttr set by
Trix's uponSanitizeAttribute hook is honored. Quoted mail with an embedded
<style> block is the common case: in HEY, 67 of 153 real mail bodies lost
their embedded content on paste. Dragging the same content was lossless.

Escape "<" and ">" inside the JSON as "\u003c" and "\u003e" at both ends:
AttachmentView emits data-trix-attachment and data-trix-attributes that way,
so Trix's own HTML never carries a trigger sequence, and HTMLSanitizer
rewrites those attributes before DOMPurify sees them, so stored, server-
rendered and older-Trix HTML with literal brackets survives too. In JSON
text angle brackets only occur inside string literals, where the escapes
spell the same characters, so JSON.parse reads back the same value; the
sanitizer only rewrites values that already parse as JSON.

Author: Jeremy Daer

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

@@ -1332,6 +1332,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() {
@@ -3256,6 +3269,7 @@ $\
     });
     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(" ");
@@ -3336,6 +3350,20 @@ $\
           element.removeAttribute(name);
         }
       });
+
+      // HTML from older Trix versions, server-side renderers and stored content carries the
+      // JSON with literal angle brackets. The hooks above put back a data-trix-* attribute that
+      // SAFE_FOR_XML drops for containing "</style>" or another raw-text closing sequence, but
+      // the restored value still carries it. 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 alone: 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() {
@@ -3360,6 +3388,14 @@ $\
       return element.getAttribute("data-trix-serialize") === "false" && !nodeIsAttachmentElement(element);
     }
   }
+  const parsesAsJSON = string => {
+    try {
+      JSON.parse(string);
+      return true;
+    } catch (error) {
+      return false;
+    }
+  };
   const createBodyElementForHTML = function () {
     let html = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : "";
     // Remove everything after </html>
@@ -3480,15 +3516,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;
@@ -3535,6 +3571,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 a style tag", async () => {
+    const content = "<style>.quoted { color: red }</style><p>quoted mail</p>"
+    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,
@@ -302,6 +303,28 @@ 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 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"]
 

src/test/unit/html_sanitizer_test.js

@@ -1,5 +1,6 @@
 import {
   assert,
+  attachmentHTML,
   test,
   testGroup,
 } from "test/test_helper"
@@ -53,7 +54,9 @@ testGroup("HTMLSanitizer", () => {
       const value = `{"contentType":"text/html","content":"${markup}"}`
       const html = `<figure data-trix-attachment='${value}'></figure>`
       const body = HTMLSanitizer.sanitize(html, { purifyOptions: { SAFE_FOR_XML: true } }).getBody()
-      assert.equal(body.querySelector("figure").getAttribute("data-trix-attachment"), value)
+      const kept = body.querySelector("figure").getAttribute("data-trix-attachment")
+      assert.deepEqual(JSON.parse(kept), JSON.parse(value))
+      assert.notOk(/[<>]/.test(kept), "raw angle brackets left in: " + kept)
     })
   })
 
@@ -62,6 +65,60 @@ testGroup("HTMLSanitizer", () => {
     const body = HTMLSanitizer.sanitize(html, { purifyOptions: { SAFE_FOR_XML: true } }).getBody()
     assert.equal(body.querySelector("a").hasAttribute("class"), false)
   })
+
+  // DOMPurify's SAFE_FOR_XML attribute rule drops any attribute whose value contains a
+  // sequence that could close a raw-text element or a comment, before the forceKeepAttr set
+  // by Trix's uponSanitizeAttribute hook is honored. The afterSanitizeAttributes hook in
+  // html_sanitizer.js puts the attribute back; escaping the JSON first means the value never
+  // trips the rule, so what comes out carries no raw angle brackets either.
+  const safeForXMLTriggers = [
+    "</style>", "</script>", "</title>", "</xmp>", "</textarea>", "</noscript>", "</iframe>", "</noembed>", "</noframes>",
+    "-->", "--!>", "]]>",
+  ]
+
+  safeForXMLTriggers.forEach((trigger) => {
+    test(`keeps attachment JSON containing ${trigger} under SAFE_FOR_XML`, () => {
+      const attachment = { contentType: "text/html", content: `<p>before ${trigger} after</p>` }
+      const attributes = { caption: `caption ${trigger}` }
+      const sanitized = HTMLSanitizer.sanitize(attachmentHTML(attachment, attributes), { purifyOptions: { SAFE_FOR_XML: true } })
+      const figure = sanitized.body.querySelector("figure")
+
+      assert.ok(figure, "attachment element was dropped")
+      assert.deepEqual(JSON.parse(figure.getAttribute("data-trix-attachment")), attachment)
+      assert.deepEqual(JSON.parse(figure.getAttribute("data-trix-attributes")), attributes)
+      assert.notOk(/[<>]/.test(figure.getAttribute("data-trix-attachment")), "raw angle brackets left in attachment JSON")
+      assert.notOk(/[<>]/.test(figure.getAttribute("data-trix-attributes")), "raw angle brackets left in attributes JSON")
+    })
+  })
+
+  test("keeps nested attachment JSON containing </style> under SAFE_FOR_XML", () => {
+    const inner = { contentType: "text/html", content: "<style>p { color: red }</style><p>quoted</p>" }
+    const attachment = { contentType: "text/html", content: `<p>reply</p>${attachmentHTML(inner)}` }
+    const sanitized = HTMLSanitizer.sanitize(attachmentHTML(attachment), { purifyOptions: { SAFE_FOR_XML: true } })
+    const figure = sanitized.body.querySelector("figure")
+
+    assert.ok(figure, "attachment element was dropped")
+    assert.deepEqual(JSON.parse(figure.getAttribute("data-trix-attachment")), attachment)
+  })
+
+  test("keeps attachment attributes JSON containing </style> under SAFE_FOR_XML", () => {
+    const attachment = { contentType: "image/png", filename: "example.png" }
+    const attributes = { caption: "</style>" }
+    const sanitized = HTMLSanitizer.sanitize(attachmentHTML(attachment, attributes), { purifyOptions: { SAFE_FOR_XML: true } })
+    const figure = sanitized.body.querySelector("figure")
+
+    assert.ok(figure, "attachment element was dropped")
+    assert.deepEqual(JSON.parse(figure.getAttribute("data-trix-attachment")), attachment)
+    assert.deepEqual(JSON.parse(figure.getAttribute("data-trix-attributes")), attributes)
+  })
+
+  test("leaves malformed attachment JSON alone", () => {
+    const html = "<figure data-trix-attachment=\"{&quot;x:}<\" data-trix-attributes=\"<>\"></figure>"
+    const figure = HTMLSanitizer.sanitize(html).body.querySelector("figure")
+
+    assert.equal(figure.getAttribute("data-trix-attachment"), "{\"x:}<")
+    assert.equal(figure.getAttribute("data-trix-attributes"), "<>")
+  })
 })
 
 const withDOMPurifyConfig = (attrConfig = {}, fn) => {

src/trix/core/helpers/strings.js

@@ -73,3 +73,14 @@ const utf16StringDifference = function(a, b) {
     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.
+export const escapeAngleBracketsInJSON = (json) => json.replace(/[<>]/g, (bracket) => angleBracketEscapes[bracket])