Skip to content

Commit ab91460

Browse files
picnixzpicnixz
committed
[3.14] gh-134696: align OpenSSL and HACL*-based hash functions constructors AC signatures (GH-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. (cherry picked from commit c6e63d9) (cherry picked from commit 379d0bc) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent 2912fcc commit ab91460

16 files changed

+873
-438
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: 74 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
@@ -141,11 +142,10 @@ def __init__(self, *args, **kwargs):
141142
# of hashlib.new given the algorithm name.
142143
for algorithm, constructors in self.constructors_to_test.items():
143144
constructors.add(getattr(hashlib, algorithm))
144-
def _test_algorithm_via_hashlib_new(data=None, _alg=algorithm, **kwargs):
145-
if data is None:
146-
return hashlib.new(_alg, **kwargs)
147-
return hashlib.new(_alg, data, **kwargs)
148-
constructors.add(_test_algorithm_via_hashlib_new)
145+
def c(*args, __algorithm_name=algorithm, **kwargs):
146+
return hashlib.new(__algorithm_name, *args, **kwargs)
147+
c.__name__ = f'do_test_algorithm_via_hashlib_new_{algorithm}'
148+
constructors.add(c)
149149

150150
_hashlib = self._conditional_import_module('_hashlib')
151151
self._hashlib = _hashlib
@@ -250,6 +250,75 @@ def test_usedforsecurity_false(self):
250250
self._hashlib.new("md5", usedforsecurity=False)
251251
self._hashlib.openssl_md5(usedforsecurity=False)
252252

253+
@unittest.skipIf(get_fips_mode(), "skip in FIPS mode")
254+
def test_clinic_signature(self):
255+
for constructor in self.hash_constructors:
256+
with self.subTest(constructor.__name__):
257+
constructor(b'')
258+
constructor(data=b'')
259+
constructor(string=b'') # should be deprecated in the future
260+
261+
digest_name = constructor(b'').name
262+
with self.subTest(digest_name):
263+
hashlib.new(digest_name, b'')
264+
hashlib.new(digest_name, data=b'')
265+
hashlib.new(digest_name, string=b'')
266+
if self._hashlib:
267+
self._hashlib.new(digest_name, b'')
268+
self._hashlib.new(digest_name, data=b'')
269+
self._hashlib.new(digest_name, string=b'')
270+
271+
@unittest.skipIf(get_fips_mode(), "skip in FIPS mode")
272+
def test_clinic_signature_errors(self):
273+
nomsg = b''
274+
mymsg = b'msg'
275+
conflicting_call = re.escape(
276+
"'data' and 'string' are mutually exclusive "
277+
"and support for 'string' keyword parameter "
278+
"is slated for removal in a future version."
279+
)
280+
duplicated_param = re.escape("given by name ('data') and position")
281+
unexpected_param = re.escape("got an unexpected keyword argument '_'")
282+
for args, kwds, errmsg in [
283+
# Reject duplicated arguments before unknown keyword arguments.
284+
((nomsg,), dict(data=nomsg, _=nomsg), duplicated_param),
285+
((mymsg,), dict(data=nomsg, _=nomsg), duplicated_param),
286+
# Reject duplicated arguments before conflicting ones.
287+
*itertools.product(
288+
[[nomsg], [mymsg]],
289+
[dict(data=nomsg), dict(data=nomsg, string=nomsg)],
290+
[duplicated_param]
291+
),
292+
# Reject unknown keyword arguments before conflicting ones.
293+
*itertools.product(
294+
[()],
295+
[
296+
dict(_=None),
297+
dict(data=nomsg, _=None),
298+
dict(string=nomsg, _=None),
299+
dict(string=nomsg, data=nomsg, _=None),
300+
],
301+
[unexpected_param]
302+
),
303+
((nomsg,), dict(_=None), unexpected_param),
304+
((mymsg,), dict(_=None), unexpected_param),
305+
# Reject conflicting arguments.
306+
[(nomsg,), dict(string=nomsg), conflicting_call],
307+
[(mymsg,), dict(string=nomsg), conflicting_call],
308+
[(), dict(data=nomsg, string=nomsg), conflicting_call],
309+
]:
310+
for constructor in self.hash_constructors:
311+
digest_name = constructor(b'').name
312+
with self.subTest(constructor.__name__, args=args, kwds=kwds):
313+
with self.assertRaisesRegex(TypeError, errmsg):
314+
constructor(*args, **kwds)
315+
with self.subTest(digest_name, args=args, kwds=kwds):
316+
with self.assertRaisesRegex(TypeError, errmsg):
317+
hashlib.new(digest_name, *args, **kwds)
318+
if self._hashlib:
319+
with self.assertRaisesRegex(TypeError, errmsg):
320+
self._hashlib.new(digest_name, *args, **kwds)
321+
253322
def test_unknown_hash(self):
254323
self.assertRaises(ValueError, hashlib.new, 'spam spam spam spam spam')
255324
self.assertRaises(TypeError, hashlib.new, 1)
@@ -719,8 +788,6 @@ def check_blake2(self, constructor, salt_size, person_size, key_size,
719788
self.assertRaises(ValueError, constructor, node_offset=-1)
720789
self.assertRaises(OverflowError, constructor, node_offset=max_offset+1)
721790

722-
self.assertRaises(TypeError, constructor, data=b'')
723-
self.assertRaises(TypeError, constructor, string=b'')
724791
self.assertRaises(TypeError, constructor, '')
725792

726793
constructor(

‎Misc/NEWS.d/next/Library/2025-05-26-14-04-39.gh-issue-134696.P04xUa.rst

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)