MatthiasValvekens / pyHanko

pyHanko: sign and stamp PDF files
MIT License
483 stars 71 forks source link

Fix various result propagation issues in AdES engine #251

Closed MatthiasValvekens closed 1 year ago

MatthiasValvekens commented 1 year ago

Description of the changes

The discussion in #228 centres on some files with an LTV configuration that has some interesting properties:

PyHanko's old validation engine can't cope with some of these characteristics. The new AdES validation deals with them mostly correctly, but the status report is confusing because not all of the information that one would expect actually propagates to the status report. This PR fixes some of those issues.

Caveats

Upgrades pyhanko-certvalidator to 0.21.0, which includes a minor but breaking change in the exception hierarchy (see the changelog).

Checklist

Please go over this checklist to increase the chances of your PR being worked on in a timely manner. Deviations are allowed with proper justification (see previous section).

For new features (delete if not applicable)

For bug fixes (delete if not applicable)

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 98.43% and project coverage change: +0.02 :tada:

Comparison is base (a408c16) 98.24% compared to head (a391c8b) 98.27%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #251 +/- ## ========================================== + Coverage 98.24% 98.27% +0.02% ========================================== Files 86 86 Lines 13048 13081 +33 ========================================== + Hits 12819 12855 +36 + Misses 229 226 -3 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `98.27% <98.43%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/MatthiasValvekens/pyHanko/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens) | Coverage Δ | | |---|---|---| | [pyhanko/sign/validation/ades.py](https://codecov.io/gh/MatthiasValvekens/pyHanko/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens#diff-cHloYW5rby9zaWduL3ZhbGlkYXRpb24vYWRlcy5weQ==) | `91.81% <96.42%> (+0.45%)` | :arrow_up: | | [pyhanko/sign/validation/generic\_cms.py](https://codecov.io/gh/MatthiasValvekens/pyHanko/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens#diff-cHloYW5rby9zaWduL3ZhbGlkYXRpb24vZ2VuZXJpY19jbXMucHk=) | `96.98% <100.00%> (+0.72%)` | :arrow_up: | | [pyhanko/sign/validation/status.py](https://codecov.io/gh/MatthiasValvekens/pyHanko/pull/251?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens#diff-cHloYW5rby9zaWduL3ZhbGlkYXRpb24vc3RhdHVzLnB5) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Matthias+Valvekens)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

MatthiasValvekens commented 1 year ago

While trying to get coverage on the full diff I found a more thorny issue with how the AdES validator handles algorithm policy failures---I'll fix that on this branch as well.