Skip to content
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

Use only non-panicking digest API internally. #2190

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

briansmith
Copy link
Owner

Expose digest::try_finish to the rest of the crate. Use it everywhere digests are computed internally, avoiding digest::finish completely, including transitive uses. This requires adding new non-panicking variants of several APIs. For now, make those variants non-public, until tests and documentation are updated.

This is a bit too extreme because we know some of these cases will never fail as the input is known to be short. We will need to choose to either just accept this, or introduce another new (internal?) infallible API.

@briansmith briansmith self-assigned this Dec 18, 2024
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 93.61702% with 12 lines in your changes missing coverage. Please review.

Project coverage is 96.88%. Comparing base (a8619bc) to head (d434fad).

Files with missing lines Patch % Lines
src/hkdf.rs 69.44% 11 Missing ⚠️
src/hmac.rs 97.36% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2190      +/-   ##
==========================================
- Coverage   96.92%   96.88%   -0.04%     
==========================================
  Files         154      154              
  Lines       20235    20311      +76     
  Branches      461      461              
==========================================
+ Hits        19612    19678      +66     
- Misses        518      528      +10     
  Partials      105      105              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Make the nature of panics in the HMAC module clearer--`ring::hmac`
APIs only panic when the input is too long.
Expose `digest::try_finish` to the rest of the crate. Use it everywhere
digests are computed internally, avoiding `digest::finish` completely,
including transitive uses. This requires adding new non-panicking
variants of several APIs. For now, make those variants non-public,
until tests and documentation are updated.

This is a bit too extreme because we know some of these cases will
never fail as the input is known to be short. We will need to choose to
either just accept this, or introduce another new (internal?)
infallible API.
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.

1 participant