Strip image tags after the newline rewrite, not before it

The card binds the body Text to styledBody, which rewrites newlines to <br/>
*after* sanitizeBody has run. That rewrite inserts tag syntax into text the
stripper deliberately kept: a kept tag may hold a `<` of its own, and `<x`,
newline, `<img src="http://host/x.png">` is one tag named `x` to both the
stripper and Qt, so it survives whole — until the rewrite splits it into
`<x<br/>` and a live image tag the input never contained.

Measured against Qt 6.11.2 with an offscreen StyledText and a local HTTP
server: that body issues the GET after this branch's sanitizer and issues
nothing before it, because the one-pass /<img[^>]*>/gi it replaces deleted the
inner substring outright. The whole-tag bound is still the right trade — it is
what stops the stripper manufacturing tags — but it only holds if nothing edits
the string afterwards.

So move the rewrite into NotificationLogic, next to the reasoning it depends
on, and strip again after it. What Qt parses is then what was checked last. The
tests assert on styledBody for the same reason, since sanitizeBody's output is
no longer the string that reaches the renderer, and a regex assertion pins the
card's binding because no JavaScript assertion can see a QML property.
This commit is contained in:
David Heinemeier Hansson
2026-08-27 16:53:55 +02:00
parent 0260d2accb
commit 7026ede90b
3 changed files with 61 additions and 2 deletions
@@ -77,6 +77,18 @@ function stripImageTags(text) {
return out
}
// What the card renders, and the last thing to touch the string before Qt parses
// it. The newline rewrite belongs here rather than in the card because it inserts
// `<br/>` into text stripImageTags chose to KEEP, and a kept tag may hold a `<` of
// its own: `<x`, newline, `<img src="http://…">` is one tag named `x` to both the
// stripper and Qt, until the rewrite splits it into `<x<br/>` and a live image tag
// the input never contained. Measured against Qt 6.11.2 — the rewritten form
// fetches, the original does not. So strip again after, and what Qt parses is what
// was checked last.
function styledBody(body, app, appIcon) {
return stripImageTags(sanitizeBody(body, app, appIcon).replace(/\r\n|\r|\n/g, "<br/>"))
}
function sanitizeBody(body, app, appIcon) {
var text = stripImageTags(String(body || ""))
if (!isChromiumDerived(app, appIcon)) return text
@@ -438,6 +450,7 @@ if (typeof module !== "undefined") {
module.exports = {
isChromiumDerived: isChromiumDerived,
sanitizeBody: sanitizeBody,
styledBody: styledBody,
summaryStartsWithGlyph: summaryStartsWithGlyph,
shouldBypassDnd: shouldBypassDnd,
isEphemeralApp: isEphemeralApp,
@@ -44,7 +44,7 @@ BorderSurface {
readonly property bool singleLineToast: sanitizedBody.length === 0
readonly property bool collapseRedundantIcon: singleLineToast && !hasGlyph && summaryStartsWithGlyph
readonly property string sanitizedBody: sanitizeBody(body)
readonly property string styledBody: sanitizedBody.replace(/\r\n|\r|\n/g, "<br/>")
readonly property string styledBody: NotificationLogic.styledBody(body, app, appIcon)
readonly property color dimColor: Qt.darker(Color.notifications.text, 1.4)
readonly property color bodyColor: Qt.darker(Color.notifications.text, 1.15)
+47 -1
View File
@@ -44,8 +44,12 @@ function survivingTagNames(text) {
return names
}
// Assert on styledBody, not sanitizeBody: styledBody is the string the card
// binds to the StyledText, so it is the only one Qt ever parses. Checking the
// sanitizer's output instead would pass a body whose surviving tag the newline
// rewrite later splits open.
function assertNoImageSurvives(body, description) {
const out = notifications.sanitizeBody(body, 'Slack', '')
const out = notifications.styledBody(body, 'Slack', '')
const names = survivingTagNames(out)
assert(
!names.includes('img'),
@@ -99,6 +103,48 @@ assertNoImageSurvives(
'notifications leave no image tag when U+0085 follows the angle bracket'
)
// The card rewrites newlines to <br/> for the StyledText, which puts tag syntax
// inside a tag the stripper kept: `<x`, newline, `<img …>` is one tag named `x`
// to both the stripper and Qt, and the rewrite splits it into `<x<br/>` and a
// live image tag. Measured against Qt 6.11.2 — the rewritten form issues the GET
// and the original does not — so the strip has to run after the rewrite, which
// is what styledBody() does.
assertNoImageSurvives(
'<x\n<img src="http://host/split.png">',
'notifications leave no image tag when a newline rewrite splits a kept tag'
)
assertNoImageSurvives(
'<x\r\n<img src="http://host/split-crlf.png">',
'notifications leave no image tag when a CRLF rewrite splits a kept tag'
)
assertEqual(
notifications.styledBody('<x\n<img src="http://host/split.png">', 'Slack', ''),
'<x<br/>',
'notifications drop the image half of a tag the newline rewrite splits'
)
// The rewrite itself still happens, and body markup other than images survives it.
assertEqual(
notifications.styledBody('<b>bold</b>\nsecond line', 'Slack', ''),
'<b>bold</b><br/>second line',
'notifications keep body markup and the line break the card renders'
)
// The order above is only worth anything if the card actually renders it, and no
// JavaScript assertion can see a QML binding. Pin the binding itself: the rewrite
// belongs in the logic module, where the strip runs after it.
const cardQml = fs.readFileSync(path.join(root, 'shell/plugins/notifications/components/NotificationCard.qml'), 'utf8')
assert(
/readonly property string styledBody: NotificationLogic\.styledBody\(body, app, appIcon\)/.test(cardQml),
'the notification card renders the body that was stripped after the newline rewrite'
)
assert(
!/<br\/>/.test(cardQml),
'the notification card does not rewrite newlines itself, which would leave tag syntax unchecked'
)
assertEqual(
notifications.sanitizeBody('trailing <img src="http://host/z.png"', 'Slack', ''),
'trailing ',