classification
Title: Default hmac.new() digestmod has not been removed from documentation
Type: Stage: patch review
Components: Documentation Versions: Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Alex.Willmer, Leandro Lima, docs@python
Priority: normal Keywords: patch

Created on 2019-06-10 21:02 by Alex.Willmer, last changed 2019-11-27 03:38 by Leandro Lima.

Pull Requests
URL Status Linked Edit
PR 13947 open Alex.Willmer, 2019-06-10 21:46
Messages (3)
msg345144 - (view) Author: Alex Willmer (Alex.Willmer) * Date: 2019-06-10 21:02
Until Python 3.8 hmc.new() defaulted the digestmod argument to 'hmac-md5'. This was deperecated, to be removed in Python 3.8. In Python 3.8.0b1 it is gone, e.g.

Python 3.8.0b1 (default, Jun  6 2019, 03:44:52) 
[GCC 7.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import hmac
>>> hmac.new(b'qwertyuiop').name
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.8/hmac.py", line 146, in new
    return HMAC(key, msg, digestmod)
  File "/usr/lib/python3.8/hmac.py", line 49, in __init__
    raise ValueError('`digestmod` is required.')
ValueError: `digestmod` is required.

but the deprecation note, and the documented signature haven't been updated.

PR incoming
msg345145 - (view) Author: Alex Willmer (Alex.Willmer) * Date: 2019-06-10 21:20
Scratch the part about documented signature, it's still `hmac.new(... digestmod=None)`, the check happens in the body of the function
msg357551 - (view) Author: Leandro Lima (Leandro Lima) Date: 2019-11-27 03:38
In my view, this function signature changed too silently. Even using static type checkers, I could only find about this compatibility breaking change when actually running the code.

If I understand well the reason it was done this way, digestmod needed to become a mandatory argument, but this couldn't be done without changing the order between msg and digestmod in the function's signature.

In my view, the two other ways this could be solved were:
1. hmac.new(key: Union[bytes, bytearray],
            digestmod: str,
            msg: Union[bytes, bytearray, None] = None)
2. hmac.new(key: Union[bytes, bytearray],
            *,
            digestmod: str,
            msg: Union[bytes, bytearray] = None)

If the signature of the function changed to reflect digestmod becoming mandatory, then static code checkers could catch a misuse of the function.

Now, suppose that we're dealing with someone that doesn't use static code analysis and a legacy signature used in some code:

hmac.new(b"key", b"msg")

- In option (1), we'd be passing b"msg" as the digestmod argument when the original intention was to pass it as the msg argument. But since both have disjoint expected types, this mistake would be rejected because passing the wrong type would lead to a TypeError
- In option (2) we'd be making clear that from now on, both msg and digestmod would only be specifiable as keyword arguments and an inadvertent use of the old signature would also lead to a TypeError.

Given that it seems a rather safe signature change (that is: there's no chance someone would be able to use the old signature with the new definition) and that actually changing the signature would allow for static code analysis tools to actually catch the error without needing to run the code, I think that we should consider further changing this function and making sure that the change doesn't go so easily unnoticed like today.
History
Date User Action Args
2019-11-27 03:38:58Leandro Limasetnosy: + Leandro Lima
messages: + msg357551
2019-06-10 21:46:07Alex.Willmersetkeywords: + patch
stage: patch review
pull_requests: + pull_request13814
2019-06-10 21:20:37Alex.Willmersetmessages: + msg345145
2019-06-10 21:02:15Alex.Willmercreate