Issue 29835: py_blake2*_new_impl produces inconsistent error messages, and raises OverflowError where ValueError might be better (original) (raw)

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 "", line 1, in OverflowError: leaf_size is too large hashlib.blake2b(leaf_size=1 << 1000) Traceback (most recent call last): File "", line 1, in OverflowError: Python int too large to convert to C unsigned long

   hashlib.blake2b(depth=256) Traceback (most recent call last): File "", line 1, in ValueError: depth must be between 1 and 255 hashlib.blake2b(depth=256 << 1000) Traceback (most recent call last): File "", line 1, in 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 "", line 1, in OverflowError: can't convert negative value to unsigned int

   hashlib.blake2b(depth=-1 << 1000) Traceback (most recent call last): File "", line 1, in 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 "", line 1, in 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.)