Pasting an embedded-content attachment may destroy it

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

@@ -4248,16 +4248,30 @@ $\
   }
   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;
     }
-    const allowedAttributePattern = /^data-trix-/;
-    if (allowedAttributePattern.test(data.attrName)) {
+    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 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(" ");
@@ -4330,10 +4344,10 @@ $\
           element.removeAttribute("href");
         }
       }
-      Array.from(element.attributes).forEach(_ref => {
+      Array.from(element.attributes).forEach(_ref2 => {
         let {
           name
-        } = _ref;
+        } = _ref2;
         if (!this.allowedAttributes.includes(name) && name.indexOf("data-trix") !== 0) {
           element.removeAttribute(name);
         }

action_text-trix/test/application_system_test_case.rb

@@ -5,7 +5,9 @@
 class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
   driven_by :cuprite, using: :chrome, options: {
     js_errors: true,
-    headless: ENV["HEADLESS"] != "0"
+    headless: ENV["HEADLESS"] != "0",
+    # Ferrum's 10s default is too tight for Chrome to boot on loaded CI runners
+    process_timeout: 60
   }
 end
 

src/test/system/pasting_test.js

@@ -104,6 +104,16 @@ testGroup("Pasting", { template: "editor_empty" }, () => {
     delete window.unsanitized
   })
 
+  test("paste data-trix-attachment with markup in content", async () => {
+    const content = "<style>p { color: red; }</style><p>a</p>"
+    const attachment = JSON.stringify({ contentType: "text/html", content })
+    await pasteContent("text/html", `<div data-trix-attachment='${attachment}'></div>`)
+
+    const attachments = getDocument().getAttachments()
+    assert.equal(attachments.length, 1)
+    assert.equal(attachments[0].getContent(), content)
+  })
+
   test("paste data-trix-attachment unsafe html", async () => {
     window.unsanitized = []
     const pasteData = {

src/test/unit/html_parser_test.js

@@ -294,6 +294,14 @@ testGroup("HTMLParser", () => {
     assert.documentHTMLEqual(HTMLParser.parse(html).getDocument(), expectedHTML)
   })
 
+  test("parses attachments whose content contains markup when sanitizing for XML", () => {
+    const attachment = JSON.stringify({ contentType: "text/html", content: "<style>p { color: red; }</style><p>a</p>" })
+    const html = `<div data-trix-attachment='${attachment}'></div>`
+    const document = HTMLParser.parse(html, { purifyOptions: { SAFE_FOR_XML: true } }).getDocument()
+
+    assert.equal(document.getAttachmentPieces().length, 1)
+  })
+
   test("parses attachment caption from large html string", () => {
     let { html } = fixtures["image attachment with edited caption"]
 

src/test/unit/html_sanitizer_test.js

@@ -40,6 +40,28 @@ testGroup("HTMLSanitizer", () => {
     })
   })
 
+  test("strips data-trix-serialized-attributes containing markup when sanitizing for XML", () => {
+    const html = "<div data-trix-serialized-attributes='{\"a\":\"</style>\"}'>content</div>"
+    const body = HTMLSanitizer.sanitize(html, { purifyOptions: { SAFE_FOR_XML: true } }).getBody()
+    assert.notOk(body.innerHTML.includes("data-trix-serialized-attributes"))
+  })
+
+  test("keeps Trix attributes containing markup when sanitizing for XML", () => {
+    const markupValues = [ "</style>", "</title>", "</textarea>", "<![endif]-->", "]>" ]
+
+    markupValues.forEach((markup) => {
+      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)
+    })
+  })
+
+  test("removes other attributes containing markup when sanitizing for XML", () => {
+    const html = "<a href=\"#\" class=\"</style>\">a</a>"
+    const body = HTMLSanitizer.sanitize(html, { purifyOptions: { SAFE_FOR_XML: true } }).getBody()
+    assert.equal(body.querySelector("a").hasAttribute("class"), false)
+  })
 })
 
 const withDOMPurifyConfig = (attrConfig = {}, fn) => {

src/trix/models/html_sanitizer.js

@@ -4,18 +4,34 @@ import { nodeIsAttachmentElement, removeNode, tagName, walkTree } from "trix/cor
 import DOMPurify from "dompurify"
 import * as config from "trix/config"
 
+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 = []
+
 DOMPurify.addHook("uponSanitizeAttribute", function (node, data) {
   if (data.attrName === "data-trix-serialized-attributes") {
     data.keepAttr = false
     return
   }
 
-  const allowedAttributePattern = /^data-trix-/
-  if (allowedAttributePattern.test(data.attrName)) {
+  if (ALLOWED_ATTRIBUTE_PATTERN.test(data.attrName)) {
     data.forceKeepAttr = true
+    stashedAttributes.push([ data.attrName, node.getAttribute(data.attrName) ])
   }
 })
 
+DOMPurify.addHook("afterSanitizeAttributes", function (node) {
+  stashedAttributes.forEach(([ name, value ]) => {
+    if (value !== null && !node.hasAttribute(name)) {
+      node.setAttribute(name, value)
+    }
+  })
+
+  stashedAttributes = []
+})
+
 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(" ")