BUG: Fail on wrong state of reserved permission bits during encryption#2421
BUG: Fail on wrong state of reserved permission bits during encryption#2421stefan6419846 wants to merge 3 commits intopy-pdf:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2421 +/- ##
=======================================
Coverage 95.14% 95.15%
=======================================
Files 51 51
Lines 8551 8565 +14
Branches 1706 1711 +5
=======================================
+ Hits 8136 8150 +14
Misses 261 261
Partials 154 154 ☔ View full report in Codecov by Sentry. |
|
I dislike this change. as @stefan6419846 has stated it this introduced some possible errors that did not used to be. overloading |
I do not think that this suits the IntFlag usage, where you tend to just concatenate existing values like The general alternative would be to silently and auto-magically fix the corresponding bits in the background, but personally I prefer clean user input which gets validated and fixed by the user. |
Adding a warning as a first step would be a no-brainer. I'm undecided if R13-R32 should then (a) be auto-fixed (b) just get the warning (c) get an exception. |
First your reading of R13-R32 is correct and should be set to 1 |
While this might be a breaking change, I tend to still disagree about this: With |
|
Given the last feedback, I would probably go with the following approach:
This way everyone can decide whether to propagate the warning as an error through a warning filter or whether to silence it. I have seen no indication that a general deprecation of wrong permission bits is desired for now to avoid requiring too deep knowledge of PDF internals. |
That doesn't fit our definition of what warnings are used for: https://pypdf.readthedocs.io/en/stable/user/suppress-warnings.html (but besides that, it sounds reasonable to me). Maybe we should just check if That would also give the user control + we have a default that works for most people / cases. |
|
This would mostly apply to the writer, which AFAIK does not have a strict mode at the moment. We would have to add this if required. |
Fixes #2401.
This change is a bit tricky, as it can be interpreted as a breaking one. We previously did not ensure that the user would use this correctly and this will throw a hard error now. On the other hand, users which would get an error now have been using this the wrong way the whole time anyway.