-
-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix markup injection issues #236
base: main
Are you sure you want to change the base?
Conversation
merge conflict, otherwise looks fine (but let's tests check it too) |
81f3f70
to
be5c363
Compare
a224c12
to
fd9ff2f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
- Coverage 93.17% 93.04% -0.13%
==========================================
Files 58 58
Lines 11048 11074 +26
==========================================
+ Hits 10294 10304 +10
- Misses 754 770 +16 ☔ View full report in Codecov by Sentry. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025010617-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111705-4.3&flavor=update
Failed tests15 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/119126#dependencies 3 fixed
Unstable tests
|
FWIW openQA says:
|
fd9ff2f
to
689d549
Compare
Both pylint and tests detected some issues, I think the same one. |
689d549
to
e0794e2
Compare
This will need rebase on top of #240 (queued for merging already). |
e0794e2
to
4f3d7f1
Compare
openQA says:
(there may be more issues like this) |
8b8a126
to
275d9f6
Compare
275d9f6
to
99c6f83
Compare
Mixed up PRs ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And that's all what openQA plus some manual testing found.
99c6f83
to
b2733b2
Compare
My test box uses KDE+Wayland, so this change was to make the code easier to test. I’ll revert it. |
This fixes some (theoretical) markup injection problems. I believe none of the strings that I escape here will ever contain "<" or "&", but it is always safer to escape.
b2733b2
to
fa4bcc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some unit tests, codecov complains. I'm not sure if this is needed (see: theoretical), and I think your time could be better spent on things like fixing tray icons from non-dom0 VMs under wayland, but sure, we can have this.
This fixes some (theoretical) markup injection problems. I believe none of the strings that I escape here will ever contain "<" or "&", but it is always safer to escape.