-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix NoneType object is not subscriptable from _insert_filtered_annotations #3444
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
base: main
Are you sure you want to change the base?
Fix NoneType object is not subscriptable from _insert_filtered_annotations #3444
Conversation
…ltered_annotations There are cases where the IndirectObject is not None, but d[0] will fail with "TypeError: 'NoneType' object is not subscriptable" in generic/_base.py, in __getitem__ at `return self._get_object_with_check()[key]`
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3444 +/- ##
==========================================
- Coverage 96.97% 96.95% -0.03%
==========================================
Files 54 54
Lines 9337 9340 +3
Branches 1711 1711
==========================================
+ Hits 9055 9056 +1
- Misses 168 170 +2
Partials 114 114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
try: | ||
p = self._get_cloned_page(d[0], pages, reader) | ||
except TypeError: | ||
# There are cases where the IndirectObject is not None, but d[0] will fail: |
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.
You basically want to check for not is_null_or_none(d[0].get_object())
? Having this explicitly is preferred over more costly exception handling.
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.
You basically want to check for
not is_null_or_none(d[0].get_object())
?
No, i can't do that. As i wrote, d[0]
gives a TypeError.
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.
Sorry, my mistake. But catching exceptions is still bad. Then
if not is_null_or_none(d) and isinstance(d.get_object(), ArrayObject)
should work?
Thanks for the PR. Please have a look at the failing checks. |
@stefan6419846 Which checks do you mean? I only see coverage, which i can't fix, as i stated in the description. If you see anything else that i could improve that would make you consider merging this, please let me know! |
We have three failing checks:
At least the title should be solvable directly.
There surely are ways to modify existing example files used in tests to show this behavior (you should be able to find some examples in the tests). I currently do not have enough resources to look into this myself. There are ways to manually anonymize your file as well, but this requires more knowledge and might require you providing me the original file to look into. |
There are cases where the IndirectObject is not None, but
d[0]
will fail withTypeError: 'NoneType' object is not subscriptable
ingeneric/_base.py
, atreturn self._get_object_with_check()[key]
.I previously reported this issue in #3211, but the fix for that issue didn't fix this one.
Unfortunately i still can't attach the PDF file that triggered this issue, because it contains personal information.
If someone has an idea how to create a PDF to test this with, i'd be happy to try.
I fully understand that a maintainer would not be keen on including fixes for rare bugs without a test.
More details on the error
The script i used to reproduce the error:
In the script directory i have a
broken.pdf
(the offending file i can't share here) and anempty.pdf
.This fails as below:
Setting a breakpoint before the error occurs gives:
(To be fair, in this last example i cheated a bit because the variable name
d
clashes with the pdb command. So i renamed that.)