Codecov Report

Patch coverage: 98.72% and project coverage change: +0.14 🎉

Comparison is base (6df64af) 93.92% compared to head (f59f75f) 94.06%.

@@            Coverage Diff             @@
##             main    #2000      +/-   ##
==========================================
+ Coverage   93.92%   94.06%   +0.14%     
==========================================
  Files          33       38       +5     
  Lines        7011     7081      +70     
  Branches     1394     1394              
==========================================
+ Hits         6585     6661      +76     
+ Misses        272      269       -3     
+ Partials      154      151       -3     

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

@exiledkingcc
canyou upgrade the workflow to add test with cryptography?

Could you please also adjust https://github.com/py-pdf/pypdf/blob/main/docs/user/encryption-decryption.md (the comment about pycryptodome)?

Comparison of the two libraries:

• Maintainer(s): pycryptodome is just one mostly anonymous user, whereas pyca is a PyPI organization
• PyPI downloads: cryptography has 156,673,182 downloads, pycryptodome has 19,755,432
• Github Stars: cryptography has 5.7k stars, pycryptodome as 2.4k
• Github Forks: cryptography has 1.6k, pycryptodome as 0.4k

It seems to me that we should primarily use cryptography and only use pycryptodome as a fallback. What do you think @exiledkingcc ?

your opinion sounds very reasonable，i agree with it. but should contain it in the commit? then how about pip install pypdf[crypto] and ralated documents, will they should be changed in the commit also?

It's ok for me if you leave it as it is. That is not a blocker.

The change of the default (from pycryptodome to cryptography) should probably be done AFTER I merge this one. Just to keep this PR fairly stable. I want to merge it on Friday evening / early Saturday.

@exiledkingcc @MartinThoma I can't understand, v3.14.0 release mentions this PR but pip install "pypdf[crypto]" still installs PyCryptodome instead of cryptography. What did I not understand well?

Also, could you guys clarify if cryptography is a pure-python library and if it handles AES encryption?

for now, cryptography is a fallback for PyCryptodome. it will change in the future, since cryptography seems more official.

Also, could you guys clarify if cryptography is a pure-python library and if it handles AES encryption?

cryptography is a wrapper of openssl, not pure-python.

for now, cryptography is a fallback for PyCryptodome. it will change in the future, since cryptography seems more official.

Does it mean for now we need pip install cryptography and pip install pypdf[crypto] will install cryptography in the future?

cryptography is a wrapper of openssl, not pure-python.

Does it handle AES encryption?

for now，you can pip install pypdf cryptography to use cryptography in pypdf. in the future, pip install pypdf[crypto] will install cryptography automatically.

cryptography can handle AES, and many other algorithms.

The latest cryptography does not support Python 3.6 anymore since 41.0.0. I'm not sure if that might be a blocker from changing the default.

There are still older versions, but I don't know if they are secure as they might have unfixed known bugs.

That is a pretty strong reason for me to drop Python 3.6 support.

Does [cryptography] handle AES encryption?

Yes: https://github.com/py-pdf/pypdf/blob/main/pypdf/_crypt_providers/_cryptography.py#L31

Just like PyCryptodome: https://github.com/py-pdf/pypdf/blob/main/pypdf/_crypt_providers/_pycryptodome.py#L30C1-L30C1