Skip to content

Commit c6e63d9

Browse files
authored
gh-134696: align OpenSSL and HACL*-based hash functions constructors AC signatures (#134713)
OpenSSL and HACL*-based hash functions constructors now support both `data` and `string` parameters. Previously these constructor functions inconsistently supported sometimes `data` and sometimes `string`, while the documentation expected `data` to be given in all cases.
1 parent 4d31d19 commit c6e63d9

16 files changed

+830
-421
lines changed

Lib/hashlib.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,29 +141,29 @@ def __get_openssl_constructor(name):
141141
return __get_builtin_constructor(name)
142142

143143

144-
def __py_new(name, data=b'', **kwargs):
144+
def __py_new(name, *args, **kwargs):
145145
"""new(name, data=b'', **kwargs) - Return a new hashing object using the
146146
named algorithm; optionally initialized with data (which must be
147147
a bytes-like object).
148148
"""
149-
return __get_builtin_constructor(name)(data, **kwargs)
149+
return __get_builtin_constructor(name)(*args, **kwargs)
150150

151151

152-
def __hash_new(name, data=b'', **kwargs):
152+
def __hash_new(name, *args, **kwargs):
153153
"""new(name, data=b'') - Return a new hashing object using the named algorithm;
154154
optionally initialized with data (which must be a bytes-like object).
155155
"""
156156
if name in __block_openssl_constructor:
157157
# Prefer our builtin blake2 implementation.
158-
return __get_builtin_constructor(name)(data, **kwargs)
158+
return __get_builtin_constructor(name)(*args, **kwargs)
159159
try:
160-
return _hashlib.new(name, data, **kwargs)
160+
return _hashlib.new(name, *args, **kwargs)
161161
except ValueError:
162162
# If the _hashlib module (OpenSSL) doesn't support the named
163163
# hash, try using our builtin implementations.
164164
# This allows for SHA224/256 and SHA384/512 support even though
165165
# the OpenSSL library prior to 0.9.8 doesn't provide them.
166-
return __get_builtin_constructor(name)(data)
166+
return __get_builtin_constructor(name)(*args, **kwargs)
167167

168168

169169
try:

Lib/test/test_hashlib.py

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import itertools
1313
import logging
1414
import os
15+
import re
1516
import sys
1617
import sysconfig
1718
import tempfile
@@ -140,11 +141,10 @@ def __init__(self, *args, **kwargs):
140141
# of hashlib.new given the algorithm name.
141142
for algorithm, constructors in self.constructors_to_test.items():
142143
constructors.add(getattr(hashlib, algorithm))
143-
def _test_algorithm_via_hashlib_new(data=None, _alg=algorithm, **kwargs):
144-
if data is None:
145-
return hashlib.new(_alg, **kwargs)
146-
return hashlib.new(_alg, data, **kwargs)
147-
constructors.add(_test_algorithm_via_hashlib_new)
144+
def c(*args, __algorithm_name=algorithm, **kwargs):
145+
return hashlib.new(__algorithm_name, *args, **kwargs)
146+
c.__name__ = f'do_test_algorithm_via_hashlib_new_{algorithm}'
147+
constructors.add(c)
148148

149149
_hashlib = self._conditional_import_module('_hashlib')
150150
self._hashlib = _hashlib
@@ -249,6 +249,56 @@ def test_usedforsecurity_false(self):
249249
self._hashlib.new("md5", usedforsecurity=False)
250250
self._hashlib.openssl_md5(usedforsecurity=False)
251251

252+
def test_clinic_signature(self):
253+
for constructor in self.hash_constructors:
254+
with self.subTest(constructor.__name__):
255+
constructor(b'')
256+
constructor(data=b'')
257+
constructor(string=b'') # should be deprecated in the future
258+
259+
def test_clinic_signature_errors(self):
260+
nomsg = b''
261+
mymsg = b'msg'
262+
conflicting_call = re.escape(
263+
"'data' and 'string' are mutually exclusive "
264+
"and support for 'string' keyword parameter "
265+
"is slated for removal in a future version."
266+
)
267+
duplicated_param = re.escape("given by name ('data') and position")
268+
unexpected_param = re.escape("got an unexpected keyword argument '_'")
269+
for args, kwds, errmsg in [
270+
# Reject duplicated arguments before unknown keyword arguments.
271+
((nomsg,), dict(data=nomsg, _=nomsg), duplicated_param),
272+
((mymsg,), dict(data=nomsg, _=nomsg), duplicated_param),
273+
# Reject duplicated arguments before conflicting ones.
274+
*itertools.product(
275+
[[nomsg], [mymsg]],
276+
[dict(data=nomsg), dict(data=nomsg, string=nomsg)],
277+
[duplicated_param]
278+
),
279+
# Reject unknown keyword arguments before conflicting ones.
280+
*itertools.product(
281+
[()],
282+
[
283+
dict(_=None),
284+
dict(data=nomsg, _=None),
285+
dict(string=nomsg, _=None),
286+
dict(string=nomsg, data=nomsg, _=None),
287+
],
288+
[unexpected_param]
289+
),
290+
((nomsg,), dict(_=None), unexpected_param),
291+
((mymsg,), dict(_=None), unexpected_param),
292+
# Reject conflicting arguments.
293+
[(nomsg,), dict(string=nomsg), conflicting_call],
294+
[(mymsg,), dict(string=nomsg), conflicting_call],
295+
[(), dict(data=nomsg, string=nomsg), conflicting_call],
296+
]:
297+
for constructor in self.hash_constructors:
298+
with self.subTest(constructor.__name__, args=args, kwds=kwds):
299+
with self.assertRaisesRegex(TypeError, errmsg):
300+
constructor(*args, **kwds)
301+
252302
def test_unknown_hash(self):
253303
self.assertRaises(ValueError, hashlib.new, 'spam spam spam spam spam')
254304
self.assertRaises(TypeError, hashlib.new, 1)
@@ -718,8 +768,6 @@ def check_blake2(self, constructor, salt_size, person_size, key_size,
718768
self.assertRaises(ValueError, constructor, node_offset=-1)
719769
self.assertRaises(OverflowError, constructor, node_offset=max_offset+1)
720770

721-
self.assertRaises(TypeError, constructor, data=b'')
722-
self.assertRaises(TypeError, constructor, string=b'')
723771
self.assertRaises(TypeError, constructor, '')
724772

725773
constructor(
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Built-in HACL* and OpenSSL implementations of hash function constructors
2+
now correctly accept the same *documented* named arguments. For instance,
3+
:func:`~hashlib.md5` could be previously invoked as ``md5(data=data)``
4+
or ``md5(string=string)`` depending on the underlying implementation
5+
but these calls were not compatible. Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)