diff --git a/action_text-trix/app/assets/javascripts/trix.js b/action_text-trix/app/assets/javascripts/trix.js index 2a5b409c5..547c5fca3 100644 --- a/action_text-trix/app/assets/javascripts/trix.js +++ b/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 "", "-->" 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 "" 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 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 "" 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.replace(/<\/html[^>]*>[^]*$/i, ""); 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,7 +4541,7 @@ $\ } getData() { const data = { - trixAttachment: JSON.stringify(this.attachment), + trixAttachment: toJSONAttribute(this.attachment), trixContentType: this.attachment.getContentType(), trixId: this.attachment.id }; @@ -4504,7 +4549,7 @@ $\ 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 || ""); diff --git a/src/test/system/pasting_test.js b/src/test/system/pasting_test.js index 5000e2b69..6c3401deb 100644 --- a/src/test/system/pasting_test.js +++ b/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 = "

quoted mail

" + const attachment = new Attachment({ content, contentType: "text/html" }) + insertText(Text.textForAttachmentWithAttributes(attachment, { caption: "" })) + 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(), "") + 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 = "

quoted mail

" + const html = attachmentHTML({ content, contentType: "text/html" }, { caption: "" }) + + 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(), "") + expectDocument(`copy${OBJECT_REPLACEMENT_CHARACTER}me\n`) + }) + test("prefers plain text when html lacks formatting", async () => { const pasteData = { "text/html": "a\nb", diff --git a/src/test/test_helpers/editor_helpers.js b/src/test/test_helpers/editor_helpers.js index d97808864..5a12dc2c6 100644 --- a/src/test/test_helpers/editor_helpers.js +++ b/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 `` +} diff --git a/src/test/test_helpers/fixtures/fixtures.js b/src/test/test_helpers/fixtures/fixtures.js index 746f3da2c..7a7bffb09 100644 --- a/src/test/test_helpers/fixtures/fixtures.js +++ b/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, "\\u003e"), trixContentType: contentType, trixId: attachment.id, }, diff --git a/src/test/unit.js b/src/test/unit.js index 43520dae1..215cf1d67 100644 --- a/src/test/unit.js +++ b/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" diff --git a/src/test/unit/document_view_test.js b/src/test/unit/document_view_test.js index c6bcbca91..fc570576c 100644 --- a/src/test/unit/document_view_test.js +++ b/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 "". + test("renders attachment JSON without angle brackets", () => { + const content = "

quoted mail

" + const attachment = new Attachment({ content, contentType: "text/html" }) + const text = Text.textForAttachmentWithAttributes(attachment, { caption: "" }) + 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, "") + }) }) diff --git a/src/test/unit/helpers/strings_test.js b/src/test/unit/helpers/strings_test.js new file mode 100644 index 000000000..646d1d868 --- /dev/null +++ b/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: "", 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: "" }, + { 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: "" }) + const escaped = escapeAngleBracketsInJSON(json) + + assert.equal(escapeAngleBracketsInJSON(escaped), escaped) + }) + }) +}) diff --git a/src/test/unit/html_parser_test.js b/src/test/unit/html_parser_test.js index 8335609a9..f891d06c1 100644 --- a/src/test/unit/html_parser_test.js +++ b/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 = "

quoted mail

" + const html = attachmentHTML({ contentType: "text/html", content }, { caption: "" }) + 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(), "") + }) + + test("parses attachment whose content closes an html tag when pasting", () => { + const content = "

quoted mail

" + const html = `
before
${attachmentHTML({ contentType: "text/html", content })}
after
` + 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: "

quoted

" }) + const content = `

reply

${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"] diff --git a/src/test/unit/html_sanitizer_test.js b/src/test/unit/html_sanitizer_test.js index 962a782ad..382358d5c 100644 --- a/src/test/unit/html_sanitizer_test.js +++ b/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 = `
` 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,104 @@ 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. sanitizeElement escapes the angle + // brackets in the JSON attachment attributes first, so the value never trips the rule and + // what comes out carries no raw angle brackets either. + const safeForXMLTriggers = [ + "", "", "", "", "", "", "", "", "", + "-->", "--!>", "]]>", + ] + + safeForXMLTriggers.forEach((trigger) => { + test(`keeps attachment JSON containing ${trigger} under SAFE_FOR_XML`, () => { + const attachment = { contentType: "text/html", content: `

before ${trigger} after

` } + 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("removes a Trix attribute that isn't JSON when its value contains a trigger under SAFE_FOR_XML", () => { + const html = "
" + const figure = HTMLSanitizer.sanitize(html, { purifyOptions: { SAFE_FOR_XML: true } }).body.querySelector("figure") + + assert.ok(figure, "attachment element was dropped") + assert.equal(figure.getAttribute("data-trix-attachment"), "{\"contentType\":\"image/png\"}") + assert.notOk(figure.hasAttribute("data-trix-attributes"), "non-JSON attribute was kept") + assert.notOk(figure.hasAttribute("data-trix-content-type"), "non-JSON attribute was kept") + }) + + test("keeps nested attachment JSON containing under SAFE_FOR_XML", () => { + const inner = { contentType: "text/html", content: "

quoted

" } + const attachment = { contentType: "text/html", content: `

reply

${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 under SAFE_FOR_XML", () => { + const attachment = { contentType: "image/png", filename: "example.png" } + const attributes = { caption: "" } + 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("removes content after the closing html tag", () => { + const html = "
a
\u0000gunk" + assert.equal(HTMLSanitizer.sanitize(html).getHTML(), "
a
") + }) + + test("keeps content after a closing html tag inside an attribute, a comment or raw text", () => { + const html = "
a
\">b
c
gunk" + const sanitized = HTMLSanitizer.sanitize(html).getHTML() + + assert.ok(sanitized.startsWith("
a
b
"), sanitized) + assert.ok(sanitized.endsWith("
c
"), sanitized) + }) + + test("keeps content after two closing html tags inside one attribute value", () => { + const attributes = { caption: "onetwo" } + const html = `${attachmentHTML({ contentType: "image/png" }, attributes)}
a
gunk` + const body = HTMLSanitizer.sanitize(html).getBody() + + assert.deepEqual(JSON.parse(body.querySelector("figure").getAttribute("data-trix-attributes")), attributes) + assert.equal(body.querySelector("div").textContent, "a") + assert.notOk(body.textContent.includes("gunk"), body.innerHTML) + }) + + test("ignores marker elements supplied by the input when finding the closing html tag", () => { + const content = "
\">a
b
" + const markers = `")}>` + assert.equal(HTMLSanitizer.sanitize(content + markers).getHTML(), "
a
b
") + }) + + test("treats a closing html tag followed by non-ASCII whitespace as the browser does", () => { + const html = "
a
b
" + assert.equal(HTMLSanitizer.sanitize(html).getHTML(), "
a
b
") + }) + + test("leaves malformed attachment JSON alone", () => { + const html = "
\">
" + 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) => { diff --git a/src/trix/core/helpers/strings.js b/src/trix/core/helpers/strings.js index 34c8afc71..fd7dbd3f6 100644 --- a/src/trix/core/helpers/strings.js +++ b/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 "", "-->" 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]) diff --git a/src/trix/models/html_sanitizer.js b/src/trix/models/html_sanitizer.js index d04a3d486..a6dd993ba 100644 --- a/src/trix/models/html_sanitizer.js +++ b/src/trix/models/html_sanitizer.js @@ -1,37 +1,26 @@ import BasicObject from "trix/core/basic_object" -import { nodeIsAttachmentElement, removeNode, tagName, walkTree } from "trix/core/helpers" +import { escapeAngleBracketsInJSON, nodeIsAttachmentElement, removeNode, tagName, walkTree } from "trix/core/helpers" 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 } + // 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) ]) } }) -DOMPurify.addHook("afterSanitizeAttributes", function (node) { - stashedAttributes.forEach(([ name, value ]) => { - 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(" ") @@ -116,6 +105,19 @@ export default class HTMLSanitizer extends BasicObject { } }) + // 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 "" 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 } @@ -146,11 +148,42 @@ export default class HTMLSanitizer extends BasicObject { } } +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 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 "" 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-${Math.random().toString(36).slice(2)}` + const doc = document.implementation.createHTMLDocument("") + doc.documentElement.innerHTML = html.replace(CLOSING_HTML_TAG_PATTERN, (tag, offset) => `<${marker} data-offset=${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(html = "") { - // Remove everything after - html = html.replace(/<\/html[^>]*>[^]*$/i, "") 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) diff --git a/src/trix/views/attachment_view.js b/src/trix/views/attachment_view.js index dbf4c1244..e70d16cc6 100644 --- a/src/trix/views/attachment_view.js +++ b/src/trix/views/attachment_view.js @@ -1,6 +1,6 @@ import * as config from "trix/config" import { ZERO_WIDTH_SPACE } from "trix/constants" -import { copyObject, makeElement } from "trix/core/helpers" +import { copyObject, escapeAngleBracketsInJSON, makeElement } from "trix/core/helpers" import ObjectView from "trix/views/object_view" import HTMLSanitizer from "trix/models/html_sanitizer" import DOMPurify from "dompurify" @@ -108,14 +108,14 @@ export default class AttachmentView extends ObjectView { 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()) { @@ -168,6 +168,11 @@ const createCursorTarget = (name) => }, }) +// 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 || "")