Fix _hashlib.compare_digest to reject non-ASCII strings#7280

ever0de

Add non-ASCII string check to _hashlib.compare_digest, matching the behavior of _operator._compare_digest. When both arguments are strings, non-ASCII characters now correctly raise TypeError.

Also replace the non-constant-time == comparison with constant_time_eq for proper timing-attack resistance, and return PyResult instead of PyResult.

Summary by CodeRabbit

Chores
- Added a dependency to support constant-time cryptographic comparisons.
Bug Fixes
- Digest comparison now yields a direct boolean result and uses constant-time comparison under the hood.
- Stricter input validation with clearer errors for non-ASCII strings and incompatible/bytes-like inputs.

coderabbitai

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 959cb04 and c100b7f.

⛔ Files ignored due to path filters (2)

Cargo.lock is excluded by !**/*.lock
Lib/test/test_hmac.py is excluded by !Lib/**

📒 Files selected for processing (2)

crates/stdlib/Cargo.toml
crates/stdlib/src/hashlib.rs

📝 Walkthrough

Walkthrough

Adds the constant_time_eq dependency and refactors compare_digest in hashlib to return PyResult<bool> and perform constant-time comparisons for ASCII strings and byte buffers, with updated error handling for type and encoding mismatches.

Changes

Cohort / File(s)	Summary
Dependency Management `crates/stdlib/Cargo.toml`	Added `constant_time_eq` dependency (workspace = true).
Hash Comparison Refactor `crates/stdlib/src/hashlib.rs`	Changed `compare_digest` signature to return `PyResult<bool>`; replaced previous comparison logic with `constant_time_eq` for constant-time comparisons of ASCII strings and byte buffers; updated error handling/messages and removed previous PyObject construction paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Update hashlib from v3.14.3 and align _hashlib to CPython #7120 — Modifies crates/stdlib/src/hashlib.rs; likely related refactor or prior changes to _hashlib implementation.

Poem

🐰 I hopped through bytes in moonlit code,
I learned to time my steps just so,
Now secrets hide where they belong,
Constant and steady — safe and slow. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: rejecting non-ASCII strings in _hashlib.compare_digest, which is the primary objective of the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/stdlib/src/hashlib.rs`:
- Around line 738-745: The mixed-type error message currently uses
b.as_object().class().name() which reports the wrong type; update the match arms
handling ArgStrOrBytesLike variants (e.g., the (ArgStrOrBytesLike::Buf(a),
ArgStrOrBytesLike::Buf(b)) branch and the fallback) so that when one operand is
Str and the other is Buf you return vm.new_type_error with the incompatible type
set to "str" (or dynamically detect which of the two is ArgStrOrBytesLike::Str
and use "str") instead of using b.as_object().class().name(); keep the
constant_time_eq branch unchanged and only change the error construction for
mixed Str/Buf cases.

ℹ️ Review info

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2833cd8 and 959cb04.

⛔ Files ignored due to path filters (2)

Cargo.lock is excluded by !**/*.lock
Lib/test/test_hmac.py is excluded by !Lib/**

📒 Files selected for processing (2)

crates/stdlib/Cargo.toml
crates/stdlib/src/hashlib.rs

Add non-ASCII string check to _hashlib.compare_digest, matching the behavior of _operator._compare_digest. When both arguments are strings, non-ASCII characters now correctly raise TypeError. Also replace the non-constant-time == comparison with constant_time_eq for proper timing-attack resistance, and return PyResult<bool> instead of PyResult<PyObjectRef>.

github-actions

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

Error: 'frozenset' object has no attribute 'discard'

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

) Add non-ASCII string check to _hashlib.compare_digest, matching the behavior of _operator._compare_digest. When both arguments are strings, non-ASCII characters now correctly raise TypeError. Also replace the non-constant-time == comparison with constant_time_eq for proper timing-attack resistance, and return PyResult<bool> instead of PyResult<PyObjectRef>.

coderabbitai Bot reviewed Feb 28, 2026

View reviewed changes

ShaharNaveh reviewed Feb 28, 2026

View reviewed changes

ever0de force-pushed the fix/hashlib-compare-digest-ascii-check branch from 959cb04 to c100b7f Compare February 28, 2026 16:57

youknowone approved these changes Mar 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix _hashlib.compare_digest to reject non-ASCII strings#7280