problem_report: enforce some keys to always have str values in load()#588
Merged
bdrung merged 1 commit intocanonical:mainfrom Apr 24, 2026
Merged
problem_report: enforce some keys to always have str values in load()#588bdrung merged 1 commit intocanonical:mainfrom
bdrung merged 1 commit intocanonical:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #588 +/- ##
=======================================
Coverage 84.51% 84.52%
=======================================
Files 104 104
Lines 20988 21000 +12
Branches 3218 3219 +1
=======================================
+ Hits 17739 17751 +12
Misses 2808 2808
Partials 441 441 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hyask
approved these changes
Apr 24, 2026
Contributor
Hyask
left a comment
There was a problem hiding this comment.
Looks good in overall, just one question.
Member
Author
|
Just rebased on master. |
Member
Author
|
Noticed this problem with this patch set: So more changes are needed. |
plasmashell crashed on the system, and when Apport tried to upload the
problem report, whoopsie-upload-all crashed:
```
INFO:root:Collecting info for /var/crash/_usr_bin_plasmashell.1000.crash...
ERROR: hook /usr/share/apport/general-hooks/generic.py crashed:
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/apport/report.py", line 314, in _run_hook
symb["add_info"](report, ui)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^
File "/usr/share/apport/general-hooks/generic.py", line 70, in add_info
for lib in re.finditer(r"\s(/[^ ]+\.so[.0-9]*)$", report["ProcMaps"], re.M):
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.14/re/__init__.py", line 285, in finditer
return _compile(pattern, flags).finditer(string)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
TypeError: cannot use a string pattern on a bytes-like object
Traceback (most recent call last):
File "/usr/share/apport/whoopsie-upload-all", line 246, in <module>
main()
~~~~^^
File "/usr/share/apport/whoopsie-upload-all", line 228, in main
stamps = collect_info()
File "/usr/share/apport/whoopsie-upload-all", line 162, in collect_info
res = process_report(r)
File "/usr/share/apport/whoopsie-upload-all", line 112, in process_report
r.add_gdb_info()
~~~~~~~~~~~~~~^^
File "/usr/lib/python3/dist-packages/apport/report.py", line 1057, in add_gdb_info
addr_signature = self.crash_signature_addresses()
File "/usr/lib/python3/dist-packages/apport/report.py", line 1797, in crash_signature_addresses
if self["ProcMaps"].startswith("Error: "):
~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
TypeError: startswith first arg must be bytes or a tuple of bytes, not str
```
The `ProcMaps` field in the crash report has following last lines:
```
7f8e5d61b000-7f8e5d61e000 r-xp 00002000 fc:00 19081540 /usr/lib/x86_64-linux-gnu/qt6/plugins/kf6/packagestructure/plasma_wallpaper.so
7f8e5d61e000-7f8e5d61f000 r--p 00005000 fc:00 19081540 /usr/lib/x86_64-linux-gnu/qt6/plugins/kf6/packagestructure/plasma_wallpaper.so
7f8e5d61f000-7f8e5d620000 r--p 00005000 fc:00 190815U\0\0
```
So string value for `ProcMaps` seems to be truncated.
whoopsie-upload-all opens the crash file and keeps the value as `bytes`
because it contains null characters at the end.
The value for `ProcMaps` should always be a string. There are other
fields that are expected to be string as well.
Enforce decoding those fields when reading the crash report. This should
result in more consistent `ProblemReport` object instances. This can be
accompanied by type hints in a future commit.
Bug: https://launchpad.net/bugs/2146806
Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
Member
Author
|
Let's split this PR into two. I rebased this PR on main and only kept "problem_report: enforce some keys to always have str values in load()". The other two commits will be submitted in a follow-up PR once I addressed the apport-retrace issue mentioned above. |
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.
plasmashell crashed on the system, and when Apport tried to upload the problem report, whoopsie-upload-all crashed:
The
ProcMapsfield in the crash report has following last lines:So string value for
ProcMapsseems to be truncated. whoopsie-upload-all opens the crash file and keeps the value asbytesbecause it contains null characters at the end.The value for
ProcMapsshould always be a string. There are other fields that are expected to be string as well.Enforce decoding those fields when reading the crash report and prevent setting those keys to non-string values. This should result in more consistent
ProblemReportobject instances. This can be accompanied by type hints in a future commit.Bug: https://launchpad.net/bugs/2146806