Skip to content

Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest(). #19745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JaeHyuckSa
Copy link
Contributor

@JaeHyuckSa JaeHyuckSa commented Aug 18, 2025

Trac ticket number

ticket-36546

Branch description

django.utils.crypto.constant_time_compare() is now just a duplicate wrapper around the standard library’s hmac.compare_digest(), so it is being deprecated in favor of using the standard function directly.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Copy link
Member

@timgraham timgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use hmac.compare_digest() since the secrets version is merely an alias of it. The earlier version of your patch that removed usage of constant_time_compare() was correct to do so.

See also Deprecating a feature checklist.

@JaeHyuckSa JaeHyuckSa marked this pull request as draft August 18, 2025 15:54
@JaeHyuckSa JaeHyuckSa force-pushed the issue-36546 branch 2 times, most recently from 0206d4f to 925b937 Compare August 19, 2025 14:06
@JaeHyuckSa JaeHyuckSa changed the title Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of secrets.compare_digest(). Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest(). Aug 19, 2025
@JaeHyuckSa JaeHyuckSa changed the title Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest(). Fixed #36546 -- Deprecated django.utils.crypto.constant_time_compare() in favor of hmac.compare_digest(). Aug 19, 2025
@JaeHyuckSa JaeHyuckSa force-pushed the issue-36546 branch 2 times, most recently from 6f3340d to 4b453a1 Compare August 19, 2025 14:31
@JaeHyuckSa
Copy link
Contributor Author

Hi @timgraham — thank you for the guidance and for taking the time. I’ve reviewed the “Deprecating a feature” checklist and applied the changes across the codebase as suggested.

@JaeHyuckSa JaeHyuckSa marked this pull request as ready for review August 19, 2025 14:33
Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Have initial comments 👍

@sarahboyce
Copy link
Contributor

Can you either grant me permission to push or make sure this is a single squashed commit (no merge commit)?

@JaeHyuckSa
Copy link
Contributor Author

Can you either grant me permission to push or make sure this is a single squashed commit (no merge commit)?

I’ve granted you permission to push now. Please let me know if you run into any issues!

…) in favor of hmac.compare_digest().

Signed-off-by: SaJH <wogur981208@gmail.com>
Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@@ -570,6 +570,9 @@ Miscellaneous
* The undocumented ``django.core.mail.forbid_multi_line_headers()`` and
``django.core.mail.message.sanitize_address()`` functions are deprecated.

* The ``django.utils.crypto.constant_time_compare()`` function is deprecated.
Use ``hmac.compare_digest()`` instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Use ``hmac.compare_digest()`` instead.
Use :py:func:`hmac.compare_digest` instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always wondered about the "py" prefix since it's used inconsistently in Django's docs. AI says, "Since the Python domain is the default in Sphinx, you can often omit the :py: prefix when referring to Python objects, for example, using :func: instead of :py:func:. However, explicitly including :py: ensures clarity and avoids potential ambiguity if other domains are also in use." I'd be in favor of omitting it since Django's documentation doesn't use other domains.

Btw, I would say, "The django.utils.crypto.constant_time_compare() function is deprecated because it's merely an alias of hmac.compare_digest()." (This gives a bit more context for readers and helps them understand that the functions behavior identically and there is no adaptation required.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants