Skip to content

Commit 50ddc89

Browse files
charlagpaw-hub
authored andcommitted
Fix external image replacement serialization
Chromium changed how HTML is serialized: https://developer.chrome.com/release-notes/138? https://chromestatus.com/feature/6264983847174144 whatwg/html#6235 Which caused some of our HTML sanitizing tests to fail. To prevent any possible issues with serialization we instead create a Blob URL for our svg. This way we sidestep DOM serialization peculiarities. As a side effect DOM should be smaller. Close #9465 Co-authored-by: paw <[email protected]>
1 parent 95740d6 commit 50ddc89

31 files changed

+143
-95
lines changed

packages/otest/lib/Assertion.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ export class Assertion<T> {
8080
satisfies(check: (value: T) => { pass: false; message: string } | { pass: true }): AssertionDescriber {
8181
const result = check(this.actual)
8282
if (!result.pass) {
83-
return this.addError(`expected to satisfy condition: "${result.message}"`)
83+
return this.addError(`expected
84+
${this.actual}
85+
to satisfy condition:
86+
${result.message}
87+
`)
8488
}
8589
return noop
8690
}
@@ -96,7 +100,11 @@ export class Assertion<T> {
96100
): Promise<AssertionDescriber> {
97101
const result = await check(this.actual)
98102
if (!result.pass) {
99-
return this.addError(`expected to satisfy condition: "${result.message}"`)
103+
return this.addError(`expected
104+
${this.actual}
105+
to satisfy condition:
106+
${result.message}
107+
`)
100108
}
101109
return noop
102110
}
@@ -105,7 +113,10 @@ export class Assertion<T> {
105113
notSatisfies(check: (value: T) => { pass: boolean; message: string }): AssertionDescriber {
106114
const result = check(this.actual)
107115
if (result.pass) {
108-
return this.addError(`expected "${asString(this.actual)}" to NOT satisfy condition: "${result.message}"`)
116+
return this.addError(`expected
117+
${asString(this.actual)}
118+
to NOT satisfy condition:
119+
${result.message}`)
109120
}
110121
return noop
111122
}

src/calendar-app/calendar/gui/eventeditor-model/CalendarEventModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ export async function makeCalendarEventModel(
182182
showProgress: ShowProgressCallback = identity,
183183
uiUpdateCallback: () => void = m.redraw,
184184
): Promise<CalendarEventModel | null> {
185-
const { htmlSanitizer } = await import("../../../../common/misc/HtmlSanitizer.js")
185+
const { getHtmlSanitizer } = await import("../../../../common/misc/HtmlSanitizer.js")
186186
const ownMailAddresses = getOwnMailAddressesWithDefaultSenderInFront(logins, mailboxDetail, mailboxProperties)
187187
if (operation === CalendarOperation.DeleteAll || operation === CalendarOperation.EditAll) {
188188
assertNonNull(initialValues.uid, "tried to edit/delete all with nonexistent uid")
@@ -228,7 +228,7 @@ export async function makeCalendarEventModel(
228228
alarmModel: new CalendarEventAlarmModel(eventType, alarms, new DefaultDateProvider(), uiUpdateCallback),
229229
location: new SimpleTextViewModel(initializationEvent.location, uiUpdateCallback),
230230
summary: new SimpleTextViewModel(initializationEvent.summary, uiUpdateCallback),
231-
description: new SanitizedTextViewModel(initializationEvent.description, htmlSanitizer, uiUpdateCallback),
231+
description: new SanitizedTextViewModel(initializationEvent.description, getHtmlSanitizer(), uiUpdateCallback),
232232
comment: new SimpleTextViewModel("", uiUpdateCallback),
233233
})
234234

src/calendar-app/calendar/gui/eventpopup/CalendarEventPreviewViewModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,11 @@ export class CalendarEventPreviewViewModel {
349349
}
350350

351351
async sanitizeDescription(): Promise<void> {
352-
const { htmlSanitizer } = await import("../../../../common/misc/HtmlSanitizer.js")
352+
const { getHtmlSanitizer } = await import("../../../../common/misc/HtmlSanitizer.js")
353353
this.sanitizedDescription = prepareCalendarDescription(
354354
this.calendarEvent.description,
355355
(s) =>
356-
htmlSanitizer.sanitizeHTML(convertTextToHtml(s), {
356+
getHtmlSanitizer().sanitizeHTML(convertTextToHtml(s), {
357357
blockExternalContent: false,
358358
highlightedStrings: this.highlightedStrings,
359359
}).html,

src/calendar-app/calendar/view/CalendarInvites.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ async function getParsedEvent(fileData: DataFile): Promise<ParsedIcalFileContent
5252
}
5353

5454
export async function showEventDetails(event: CalendarEvent, eventBubbleRect: ClientRect, mail: Mail | null): Promise<void> {
55-
const [latestEvent, { CalendarEventPopup }, { CalendarEventPreviewViewModel }, { htmlSanitizer }] = await Promise.all([
55+
const [latestEvent, { CalendarEventPopup }, { CalendarEventPreviewViewModel }, { getHtmlSanitizer }] = await Promise.all([
5656
getLatestEvent(event),
5757
import("../gui/eventpopup/CalendarEventPopup.js"),
5858
import("../gui/eventpopup/CalendarEventPreviewViewModel.js"),
@@ -93,7 +93,7 @@ export async function showEventDetails(event: CalendarEvent, eventBubbleRect: Cl
9393
lazyIndexEntry,
9494
editModelsFactory,
9595
)
96-
new CalendarEventPopup(viewModel, eventBubbleRect, htmlSanitizer).show()
96+
new CalendarEventPopup(viewModel, eventBubbleRect, getHtmlSanitizer()).show()
9797
}
9898

9999
export async function getEventsFromFile(file: TutanotaFile, invitedConfidentially: boolean): Promise<ParsedIcalFileContent> {

src/calendar-app/calendar/view/CalendarView.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export class CalendarView extends BaseTopLevelView implements TopLevelView<Calen
158158

159159
this.viewModel = attrs.calendarViewModel
160160
this.currentViewType = deviceConfig.getDefaultCalendarView(userId) || CalendarViewType.MONTH
161-
this.htmlSanitizer = import("../../../common/misc/HtmlSanitizer").then((m) => m.htmlSanitizer)
161+
this.htmlSanitizer = import("../../../common/misc/HtmlSanitizer").then((m) => m.getHtmlSanitizer())
162162
this.sidebarColumn = new ViewColumn(
163163
{
164164
view: () =>

src/calendar-app/calendarLocator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ class CalendarLocator implements CommonLocator {
799799
: new WebThemeFacade(deviceConfig)
800800
const lazySanitizer = isTest()
801801
? () => Promise.resolve(sanitizerStub as HtmlSanitizer)
802-
: () => import("../common/misc/HtmlSanitizer").then(({ htmlSanitizer }) => htmlSanitizer)
802+
: () => import("../common/misc/HtmlSanitizer").then(({ getHtmlSanitizer }) => getHtmlSanitizer())
803803

804804
this.themeController = new ThemeController(theme, selectedThemeFacade, lazySanitizer, AppType.Calendar)
805805

src/common/gui/editor/HtmlEditor.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Editor } from "./Editor.js"
44
import type { MaybeTranslation, TranslationKey } from "../../misc/LanguageViewModel"
55
import { lang } from "../../misc/LanguageViewModel"
66
import { px } from "../size"
7-
import { htmlSanitizer } from "../../misc/HtmlSanitizer"
7+
import { getHtmlSanitizer, HtmlSanitizer } from "../../misc/HtmlSanitizer"
88
import { assertNotNull } from "@tutao/tutanota-utils"
99
import { DropDownSelector } from "../base/DropDownSelector.js"
1010
import { RichTextToolbar, RichTextToolbarAttrs } from "../base/RichTextToolbar.js"
@@ -33,14 +33,16 @@ export class HtmlEditor implements Component {
3333
private toolbarAttrs: Omit<RichTextToolbarAttrs, "editor"> = {}
3434
private staticLineAmount: number | null = null // Static amount of lines the editor shall allow at all times
3535

36+
private readonly htmlSanitizer: HtmlSanitizer = getHtmlSanitizer()
37+
3638
constructor(
3739
private label?: MaybeTranslation,
3840
private readonly injections?: () => Children,
3941
) {
4042
this.editor = new Editor(
4143
null,
4244
(html) =>
43-
htmlSanitizer.sanitizeFragment(html, {
45+
this.htmlSanitizer.sanitizeFragment(html, {
4446
blockExternalContent: false,
4547
allowRelativeLinks: this.areRelativeLinksAllowed,
4648
}).fragment,
@@ -216,7 +218,7 @@ export class HtmlEditor implements Component {
216218
}
217219
} else {
218220
if (this.domTextArea) {
219-
return htmlSanitizer.sanitizeHTML(this.domTextArea.value, {
221+
return this.htmlSanitizer.sanitizeHTML(this.domTextArea.value, {
220222
blockExternalContent: false,
221223
allowRelativeLinks: this.areRelativeLinksAllowed,
222224
}).html

src/common/mailFunctionality/SendMailModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -906,9 +906,9 @@ export class SendMailModel {
906906
const attachments = saveAttachments ? this.attachments : null
907907

908908
// We also want to create new drafts for drafts edited from trash or spam folder
909-
const { htmlSanitizer } = await import("../misc/HtmlSanitizer.js")
909+
const { getHtmlSanitizer } = await import("../misc/HtmlSanitizer.js")
910910
const unsanitized_body = this.getBody()
911-
const body = htmlSanitizer.sanitizeHTML(unsanitized_body, {
911+
const body = getHtmlSanitizer().sanitizeHTML(unsanitized_body, {
912912
// store the draft always with external links preserved. this reverts
913913
// the draft-src and draft-srcset attribute stow.
914914
blockExternalContent: false,

src/common/misc/HtmlSanitizer.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
import { ReplacementImage } from "../gui/base/icons/Icons"
2-
import { downcast, isEmpty, stringToUtf8Uint8Array, utf8Uint8ArrayToString } from "@tutao/tutanota-utils"
2+
import { downcast, isEmpty, memoized, stringToUtf8Uint8Array, utf8Uint8ArrayToString } from "@tutao/tutanota-utils"
33
import { DataFile } from "../api/common/DataFile"
4-
import { encodeSVG } from "../gui/base/GuiUtils.js"
54
import DOMPurify, { Config } from "dompurify"
6-
import { splitTextForHighlighting, SearchToken } from "../api/common/utils/QueryTokenUtils"
7-
8-
/** Data url for an SVG image that will be shown in place of external content. */
9-
export const PREVENT_EXTERNAL_IMAGE_LOADING_ICON: string = encodeSVG(ReplacementImage)
5+
import { SearchToken, splitTextForHighlighting } from "../api/common/utils/QueryTokenUtils"
106

117
// background attribute is deprecated but still used in common browsers
128
const EXTERNAL_CONTENT_ATTRS = Object.freeze([
@@ -123,9 +119,9 @@ export class HtmlSanitizer {
123119
private links!: Array<Link>
124120
private purifier!: typeof DOMPurify
125121

126-
constructor() {
122+
constructor(private readonly replacementImageUrl: string) {
127123
if (DOMPurify.isSupported) {
128-
this.purifier = DOMPurify
124+
this.purifier = DOMPurify()
129125
// Do changes in afterSanitizeAttributes and not afterSanitizeElements so that images are not removed again because of the SVGs.
130126
this.purifier.addHook("afterSanitizeAttributes", this.afterSanitizeAttributes.bind(this))
131127
this.purifier.addHook("beforeSanitizeElements", this.beforeSanitizeElements.bind(this))
@@ -335,15 +331,15 @@ export class HtmlSanitizer {
335331

336332
this.inlineImageCids.push(cid)
337333

338-
attribute.value = PREVENT_EXTERNAL_IMAGE_LOADING_ICON
334+
attribute.value = this.replacementImageUrl
339335
htmlNode.setAttribute("cid", cid)
340336
htmlNode.classList.add("tutanota-placeholder")
341337
} else if (config.blockExternalContent && attribute.name === "srcset") {
342338
this.externalContent++
343339

344340
htmlNode.setAttribute("draft-srcset", attribute.value)
345341
htmlNode.removeAttribute("srcset")
346-
htmlNode.setAttribute("src", PREVENT_EXTERNAL_IMAGE_LOADING_ICON)
342+
htmlNode.setAttribute("src", this.replacementImageUrl)
347343
htmlNode.style.maxWidth = "100px"
348344
} else if (
349345
config.blockExternalContent &&
@@ -360,7 +356,7 @@ export class HtmlSanitizer {
360356
this.externalContent++
361357

362358
htmlNode.setAttribute("draft-" + attribute.name, attribute.value)
363-
attribute.value = PREVENT_EXTERNAL_IMAGE_LOADING_ICON
359+
attribute.value = this.replacementImageUrl
364360
htmlNode.attributes.setNamedItem(attribute)
365361
htmlNode.style.maxWidth = "100px"
366362
} else if (!config.blockExternalContent && DRAFT_ATTRIBUTES.includes(attribute.name)) {
@@ -413,7 +409,7 @@ export class HtmlSanitizer {
413409
// image-set('test.jpg' 1x, 'test-2x.jpg' 2x)
414410
if (value.includes("url(") && value.match(/url\(/g)?.length !== value.match(/url\(["']?data:/g)?.length) {
415411
this.externalContent++
416-
;(htmlNode.style as any)[styleAttributeName] = `url("${PREVENT_EXTERNAL_IMAGE_LOADING_ICON}")`
412+
;(htmlNode.style as any)[styleAttributeName] = `url("${this.replacementImageUrl}")`
417413

418414
if (limitWidth) {
419415
htmlNode.style.maxWidth = "100px"
@@ -509,4 +505,14 @@ function isTextElement(node: Node): node is Text {
509505
return node.nodeName === "#text"
510506
}
511507

512-
export const htmlSanitizer: HtmlSanitizer = new HtmlSanitizer()
508+
export const getHtmlSanitizer = memoized(() => {
509+
// Create a blob URL for the replacement image instead of inlining SVG as data URL.
510+
// This way we sidestep serialization/escaping problems plus DOM is smaller.
511+
// It is never revoked and should only be run in browser context so we lazily instantiate
512+
// it once.
513+
const blob = new Blob([ReplacementImage], {
514+
type: "image/svg+xml",
515+
})
516+
517+
return new HtmlSanitizer(URL.createObjectURL(blob))
518+
})

src/common/settings/EditNotificationEmailDialog.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { DropDownSelector } from "../gui/base/DropDownSelector.js"
1111
import { TextField } from "../gui/base/TextField.js"
1212
import { showProgressDialog } from "../gui/dialogs/ProgressDialog.js"
1313
import { assertNotNull, LazyLoaded, memoized, neverNull, ofClass } from "@tutao/tutanota-utils"
14-
import { htmlSanitizer } from "../misc/HtmlSanitizer.js"
14+
import { getHtmlSanitizer } from "../misc/HtmlSanitizer.js"
1515
import { PayloadTooLargeError } from "../api/common/error/RestError.js"
1616
import { SegmentControl } from "../gui/base/SegmentControl.js"
1717
import { UserError } from "../api/main/UserError.js"
@@ -162,6 +162,7 @@ export function show(existingTemplate: NotificationMailTemplate | null, customer
162162
senderDomain = "https://" + ((whitelabelDomainInfo && whitelabelDomainInfo.domain) || "app.tuta.com")
163163
m.redraw()
164164
})
165+
const htmlSanitizer = getHtmlSanitizer()
165166
// Even though savedHtml is always sanitized changing it might lead to mXSS
166167
const sanitizePreview = memoized<(html: string) => string>((html) => {
167168
return htmlSanitizer.sanitizeHTML(html).html

0 commit comments

Comments
 (0)