In python 3.6.5: hashlib blake2b/blake2s constructors do not recognize 'data' keyword. Try the following:
fromhashlibimportblake2bprint (blake2b(b"foobar").hexdigest()) # worksprint (blake2b(data=b"foobar").hexdigest()) # TypeError: 'data' is an invalid keyword argument for this function
None of the hashlib functions are taking keyword arguments for data:
>>> hashlib.sha256(data=b'foo')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: openssl_sha256() takes no keyword arguments
>>> hashlib.blake2b(data=b'foo')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: 'data' is an invalid keyword argument for this function
I have no opinion on the change in the master branch, but I agree with Christian that the 3.7 change should be reverted since it breaks the backward compatibility.
In case of int() the name of it's first argument was documented, in both the module documentation, and in interactive help. But the documented name of the first blake2b() argument was "data", and it never worked. Since help() was not worked for blake2b, the user had weak chance to know that the actual name is "string". Thus there is much less chance of breaking the user code by making this parameter a positional-only than in case of int().
What's the status of this issue for 3.7 and for 3.6? Is everyone OK with what is currently in 3.7, i.e. no revert needed or has it already been reverted elsewhere? Also, there is the open PR for 3.6.
Ned, I'm currently travelling until next weekend. The PR is rather large and I don't have any means or time to review it properly. Perhaps Gregory or Dmitry Chestnykh (original author of pyblake2) are able to assist.
The question remains if Christian's and Victor's concern with 3.7 compatibility have been satisfied by Serihy's response in msg322798. While there is plenty of time to resolve concerns about what was merged into master by PR 8346, we shouldn't release the PR 8581 changes in 3.7.1 if there are still valid compatibility concerns. Since PR 9657 for 3.6 hasn't been reviewed or merged, the only pressing issue for 3.6.7 is ensuring the segfault documented in bpo-34922 is blocked, correct? And PR 8581 prevents the segfault for 3.7 so we would only need a similar fix for it if the PR were reverted, correct?
I have reviewed PR 8346 yet one time. No documented working behavior was changed. Some documentation was updated to match the code, some code was updated to match the documentation, and other minor errors were fixed. I don't think this change breaks compatibility.
Serhiy's fixes (thanks!) are now released in 3.7.0rc2 and 3.6.7rc2 so I'm removing the "release blocker" status. If there is nothing more to be done for this issue, can we close it?
Activity
JuusoLehtivarjo commented on Jun 1, 2018
In python 3.6.5: hashlib blake2b/blake2s constructors do not recognize 'data' keyword. Try the following:
tiran commented on Jul 11, 2018
None of the hashlib functions are taking keyword arguments for data:
vstinner commented on Jul 11, 2018
So it's just a documentation issue, no?
https://docs.python.org/dev/library/hashlib.html#creating-hash-objects
Juuso Lehtivarjo: do you want to write a pull request to fix the documentation?
tirkarthi commented on Jul 19, 2018
This was introduced as part of https://hg.python.org/cpython/rev/4969f6d343b1 . In addition to the signature there is also a line at https://docs.python.org/dev/library/hashlib.html#simple-hashing which could be removed
Thanks
serhiy-storchaka commented on Jul 19, 2018
hashlib.blake2b() and some other constructors accept the first chunk of data as the "string" keyword argument.
tiran commented on Jul 19, 2018
Please treat the first argument as positional-only argument. I don't want to standardize on 'string'.
5 remaining items
serhiy-storchaka commented on Jul 31, 2018
Sorry. Do you prefer to revert the whole changes or just some parts?
vstinner commented on Jul 31, 2018
I have no opinion on the change in the master branch, but I agree with Christian that the 3.7 change should be reverted since it breaks the backward compatibility.
Serhiy modified int() in Python 3.7 to convert its first parameter to positional only parameter. IMHO it's ok to do such change. Moreover, the change has been documented in What's New in Python 3.7:
https://docs.python.org/dev/whatsnew/3.7.html#api-and-feature-removals
If you decide to keep the change in the master branch, it should be documented in What's New in Python 3.8, no?
serhiy-storchaka commented on Jul 31, 2018
In case of int() the name of it's first argument was documented, in both the module documentation, and in interactive help. But the documented name of the first blake2b() argument was "data", and it never worked. Since help() was not worked for blake2b, the user had weak chance to know that the actual name is "string". Thus there is much less chance of breaking the user code by making this parameter a positional-only than in case of int().
But I think Christian has other concerns.
ned-deily commented on Oct 6, 2018
What's the status of this issue for 3.7 and for 3.6? Is everyone OK with what is currently in 3.7, i.e. no revert needed or has it already been reverted elsewhere? Also, there is the open PR for 3.6.
tiran commented on Oct 6, 2018
Ned, I'm currently travelling until next weekend. The PR is rather large and I don't have any means or time to review it properly. Perhaps Gregory or Dmitry Chestnykh (original author of pyblake2) are able to assist.
ned-deily commented on Oct 7, 2018
OK. This is now blocking the release of 3.7.1rc2 and 3.6.7rc2. See also bpo-34922.
ned-deily commented on Oct 7, 2018
The question remains if Christian's and Victor's concern with 3.7 compatibility have been satisfied by Serihy's response in msg322798. While there is plenty of time to resolve concerns about what was merged into master by PR 8346, we shouldn't release the PR 8581 changes in 3.7.1 if there are still valid compatibility concerns. Since PR 9657 for 3.6 hasn't been reviewed or merged, the only pressing issue for 3.6.7 is ensuring the segfault documented in bpo-34922 is blocked, correct? And PR 8581 prevents the segfault for 3.7 so we would only need a similar fix for it if the PR were reverted, correct?
serhiy-storchaka commented on Oct 7, 2018
I have reviewed PR 8346 yet one time. No documented working behavior was changed. Some documentation was updated to match the code, some code was updated to match the documentation, and other minor errors were fixed. I don't think this change breaks compatibility.
serhiy-storchaka commented on Oct 7, 2018
Other variant of the crash in bpo-34922 still is reproducible in 3.7.
serhiy-storchaka commented on Oct 11, 2018
New changeset f543e18 by Serhiy Storchaka in branch '3.6':
[3.6] bpo-33729: Fix issues with arguments parsing in hashlib. (GH-8346) (GH-8581) (GH-9657)
f543e18
ned-deily commented on Oct 13, 2018
Serhiy's fixes (thanks!) are now released in 3.7.0rc2 and 3.6.7rc2 so I'm removing the "release blocker" status. If there is nothing more to be done for this issue, can we close it?