1. currently, py_blake2s_new_impl and py_blake2b_new_impl might produce
inconsistent error messages (note that the first one happens on platforms
where sizeof(long) > 4):
>>> hashlib.blake2b(leaf_size=1 << 32)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: leaf_size is too large
>>> hashlib.blake2b(leaf_size=1 << 1000)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: Python int too large to convert to C unsigned long
>>> hashlib.blake2b(depth=256)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: depth must be between 1 and 255
>>> hashlib.blake2b(depth=256 << 1000)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: Python int too large to convert to C long
there are similar inconsistent error messages when the function receives big
and small out of range values for other arguments, too.
2. it might be better to raise a ValueError in the following cases:
>>> hashlib.blake2b(leaf_size=-1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: can't convert negative value to unsigned int
>>> hashlib.blake2b(depth=-1 << 1000)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: Python int too large to convert to C long
and maybe also in this case?
>>> hashlib.blake2b(depth=1 << 1000)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: Python int too large to convert to C long
this might be considered as a sub-issue of #29833.
note that solving the issue for leaf_size might be easier after #29834 is
resolved, as one could add something like the following to the if block of
(leaf_size == (unsigned long) -1 && PyErr_Occurred()):
err = PyErr_Occurred();
if (PyErr_GivenExceptionMatches(err, PyExc_OverflowError) ||
PyErr_GivenExceptionMatches(err, PyExc_ValueError)) {
PyErr_SetString(err,
"leaf_size must be between 0 and 2**32-1");
}
however, depth and other arguments are parsed by py_blake*_new, which is
generated by the argument clinic, so ISTM that solving the issue for them might
be harder.
(this issue was created while working on #15988, as can be seen in the code
review comments of PR 668.)
|