fix: sanitize clipboard and notification HTML to prevent Qt6 StyledText crash - #7381
Open
markbus-ai wants to merge 1 commit into
Open
fix: sanitize clipboard and notification HTML to prevent Qt6 StyledText crash#7381markbus-ai wants to merge 1 commit into
markbus-ai wants to merge 1 commit into
Conversation
…xt crash Clipboard entries from browsers/messaging apps (WhatsApp, etc.) often contain raw HTML that triggers a Qt6 bug in QQuickTextPrivate::updateLayout() when rendered with Text.StyledText. The crash occurs because the nbImages counter gets out of sync when invalid img tags are deleted. This fix adds sanitization at two layers: 1. Clipboard: Strip HTML tags and decode entities before storing entries. Existing entries are sanitized on load via parseHistory(). 2. Notifications: Improve sanitizeBody() to decode HTML entities before stripping img tags, catching encoded variants like <img>. The root cause is an upstream Qt6 bug (nbImages++ missing in qquickstyledtext.cpp). A separate PR will be submitted to Qt.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens text sanitization for notifications and clipboard history by decoding common HTML entities and stripping HTML tags (especially <img>), aiming to prevent rendering crashes and unwanted rich content.
Changes:
- Decode common HTML entities before sanitization to catch encoded tags.
- Strip
<img>tags in notification bodies more aggressively. - Sanitize clipboard text by decoding entities, stripping tags, and normalizing whitespace.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| shell/plugins/notifications/NotificationLogic.js | Decodes HTML entities then strips <img> tags from notification bodies. |
| shell/plugins/clipboard/ClipboardHistory.js | Introduces sanitizeText() and applies it to normalized clipboard “text” entries. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+13
to
+16
| text = text | ||
| .replace(/</gi, "<") | ||
| .replace(/>/gi, ">") | ||
| .replace(/&/gi, "&") |
Comment on lines
+5
to
+15
| function sanitizeText(text) { | ||
| var s = String(text || "") | ||
|
|
||
| // Decode HTML entities first | ||
| s = s | ||
| .replace(/</gi, "<") | ||
| .replace(/>/gi, ">") | ||
| .replace(/&/gi, "&") | ||
| .replace(/"/gi, '"') | ||
| .replace(/'/gi, "'") | ||
| .replace(/ /gi, " ") |
Comment on lines
+17
to
+18
| // Strip HTML tags (including self-closing and malformed) | ||
| s = s.replace(/<[^>]+>/g, "") |
Comment on lines
8
to
+18
| function sanitizeBody(body, app, appIcon) { | ||
| var text = String(body || "").replace(/<img[^>]*>/gi, "") | ||
| var text = String(body || "") | ||
|
|
||
| // Decode HTML entities first so we can catch encoded img tags | ||
| // e.g. <img src="..."> -> <img src="..."> | ||
| text = text | ||
| .replace(/</gi, "<") | ||
| .replace(/>/gi, ">") | ||
| .replace(/&/gi, "&") | ||
| .replace(/"/gi, '"') | ||
| .replace(/'/gi, "'") |
| // Strip all img tags aggressively (handles multiline, malformed, etc.) | ||
| // This catches: <img ...>, <IMG ...>, <img/>, <img ... />, and any variation | ||
| // The \b word boundary ensures we match "img" but not "image" or other tags | ||
| text = text.replace(/<img\b[^>]*>/gi, "") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Clipboard entries from browsers/messaging apps (WhatsApp, etc.) often contain raw HTML that triggers a Qt6 bug in
QQuickTextPrivate::updateLayout()when rendered withText.StyledText. The crash occurs because thenbImagescounter gets out of sync when invalid<img>tags are deleted.Root Cause
Upstream Qt6 bug in
qquickstyledtext.cpp: when an<img>tag has an invalid URL, the code deletes the image object but does not incrementnbImages. This causes the counter to desynchronize, leading to invalid memory access when subsequent<img>tags are processed.Fix
This PR adds sanitization at two layers:
Clipboard: Strip HTML tags and decode entities before storing entries. Existing entries are sanitized on load via
parseHistory().Notifications: Improve
sanitizeBody()to decode HTML entities before stripping<img>tags, catching encoded variants like<img>.Testing
Upstream
A separate PR will be submitted to Qt6 to fix the root cause (
nbImages++missing inqquickstyledtext.cpp).