Strip image tags whose separator Qt skips but \s does not

QQuickStyledText skips the characters between `<` and the tag name with
QChar::isSpace(), which counts U+0085 NEL. JavaScript's `\s` does not, so
isImageTag() read no name at all from a tag written as `<`, U+0085, `img`,
kept it, and Qt then read `img` and issued the GET the stripper exists to
prevent. Measured against Qt 6.11.2 with an offscreen StyledText and a
local HTTP server.

Read the name by skipping everything that is not part of it rather than by
matching the separator, so the two definitions cannot drift apart again.
Over-skipping is the safe direction: it can only classify more runs as
images, and dropping a run never manufactures a tag.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

shell/plugins/notifications/NotificationLogic.js

@@ -6,10 +6,21 @@ function isChromiumDerived(app, appIcon) {
 }
 
 // True when a `<...>` run is an image tag, so the name is read the way Qt's
-// parser reads it: after the `<` and an optional `/`, the leading run of
-// letters and digits.
+// parser reads it: after the `<`, the leading run of letters and digits.
+//
+// Skip everything up to that run rather than matching the separator, because
+// there is no JavaScript expression for what Qt skips. QQuickStyledText calls
+// skipSpace(), which is QChar::isSpace(), and that set is not `\s`: Qt counts
+// U+0085 NEL and `\s` does not, while `\s` counts U+FEFF and Qt does not. A
+// name read with `\s` therefore misses a tag written as `<`, U+0085, `img`:
+// Qt skips the NEL, reads `img` and issues the GET, while the regex finds no
+// name at all and the tag is kept. Measured against Qt 6.11.2.
+//
+// Over-skipping is the safe direction. It can only classify more runs as
+// images, and dropping a run never manufactures a tag: a dropped run joins two
+// stretches of text that each contain no `<`.
 function isImageTag(tag) {
-  var name = /^<\/?\s*([A-Za-z0-9]+)/.exec(tag)
+  var name = /^<[^A-Za-z0-9]*([A-Za-z0-9]+)/.exec(tag)
   return !!name && name[1].toLowerCase() === "img"
 }
 
@@ -19,8 +30,15 @@ function isImageTag(tag) {
 // with no user action, so image tags go before the renderer sees them.
 //
 // Work in whole tags, never in substrings of one. A `<` opens a tag that runs
-// to the next `>`, nested `<` and all — that is how Qt's parser bounds it —
-// and only a tag whose own name is `img` is dropped.
+// to the next `>`, nested `<` and all, and only a tag whose own name is `img`
+// is dropped.
+//
+// That is the conservative bound, not Qt's exact one: Qt lets a `>` inside a
+// quoted attribute value pass without closing the tag, so a Qt tag can be
+// longer than the run taken here. Do not "correct" this to match Qt. Taking
+// the shorter run only ever splits one Qt tag into several, and a split can
+// only expose an `<img` to be dropped, never hide one — whereas honouring
+// quotes would let `<b title="a>b"><img src="http://host/x.png">` through.
 //
 // Deleting a substring is what makes a naive `/<img[^>]*>/g` unsafe. Given
 //

test/shell.d/notifications-test.sh

@@ -21,7 +21,10 @@ assertEqual(
 // The body renders as StyledText, which fetches <img src> over the network. The
 // invariant that matters is not a particular output string but that no tag Qt
 // would honour as an image survives, so assert that directly. Tags are bounded
-// the way Qt bounds them: a `<` opens a tag that runs to the next `>`.
+// the conservative way the stripper bounds them: a `<` opens a tag that runs to
+// the next `>`. Qt's own bound can be longer, since a `>` inside a quoted
+// attribute value does not close a tag there — which only ever splits one Qt
+// tag into several here, so a name this helper reads is a name Qt reads too.
 function survivingTagNames(text) {
   const names = []
   let i = 0
@@ -30,7 +33,11 @@ function survivingTagNames(text) {
     if (open === -1) break
     const close = text.indexOf('>', open)
     const tag = close === -1 ? text.slice(open) : text.slice(open, close + 1)
-    const name = /^<\/?\s*([A-Za-z0-9]+)/.exec(tag)
+    // Read the name the way Qt does, skipping anything that is not part of it.
+    // Matching the separator with \s instead would give this helper the same
+    // blind spot as the code it is checking — Qt skips U+0085 and \s does not —
+    // and an assertion that shares the implementation's bug proves nothing.
+    const name = /^<[^A-Za-z0-9]*([A-Za-z0-9]+)/.exec(tag)
     if (name) names.push(name[1].toLowerCase())
     i = close === -1 ? text.length : close + 1
   }
@@ -75,6 +82,23 @@ assertNoImageSurvives(
   'notifications leave no image tag when whitespace follows the angle bracket'
 )
 
+// Qt skips the separator between `<` and the tag name with QChar::isSpace(),
+// which counts U+0085 NEL. JavaScript's \s does not. Reading the name with \s
+// finds none here, keeps the tag, and Qt then reads `img` and fetches it —
+// measured against Qt 6.11.2, where this exact body makes a StyledText Text
+// issue an outbound GET. Asserted on the whole output rather than through
+// assertNoImageSurvives so it holds even if that helper is ever loosened.
+assertEqual(
+  notifications.sanitizeBody('<\u0085img src="http://host/nel.png">after', 'Slack', ''),
+  'after',
+  'notifications strip an image tag whose separator is U+0085, which Qt skips but \\s does not'
+)
+
+assertNoImageSurvives(
+  '<\u0085img src="http://host/nel2.png">',
+  'notifications leave no image tag when U+0085 follows the angle bracket'
+)
+
 assertEqual(
   notifications.sanitizeBody('trailing <img src="http://host/z.png"', 'Slack', ''),
   'trailing ',