Skip to content

password-hash: add Error::OutputSize - #1026

Merged
tarcieri merged 2 commits into
RustCrypto:masterfrom
khonsulabs:password-hash-output-size
Jan 9, 2023
Merged

password-hash: add Error::OutputSize#1026
tarcieri merged 2 commits into
RustCrypto:masterfrom
khonsulabs:password-hash-output-size

Conversation

@daxpedda

Copy link
Copy Markdown
Contributor

See RustCrypto/password-hashes#313.

Sometimes a very specific output size is expected. Any bike-shedding is very welcome.

Comment threadpassword-hash/src/errors.rs Outdated
@daxpedda
daxpedda requested a review from tarcieriJune 17, 2022 11:53
@tarcieritarcieri changed the title Add password_hash::Error::OutputSizepassword-hash: add Error::OutputSizeJun 28, 2022
@daxpedda
daxpeddaforce-pushed the password-hash-output-size branch from cf55a17 to c1f7018CompareJuly 24, 2022 09:29
@tarcieri

Copy link
Copy Markdown
Member

This is back to having three different variants that effectively mean different flavors of the same thing.

What about this instead?

OutputSize{expected:Ordering,size: usize }

@daxpedda
daxpeddaforce-pushed the password-hash-output-size branch from 3387d43 to c80afb1CompareJuly 24, 2022 14:12
@daxpedda

daxpedda commented Jul 24, 2022

Copy link
Copy Markdown
ContributorAuthor
OutputSize{expected:Ordering,size: usize }

I wasn't sure how you mean that, I kinda turned it around, the usize holds the expected size and the Ordering holds if the actual size is too long, too short, or unequal (doesn't match properly I guess).

Also squashed the commits.

EDIT: I hope the documentation is sufficient and alright.

@daxpedda
daxpeddaforce-pushed the password-hash-output-size branch 2 times, most recently from 7808718 to ea87046CompareJuly 24, 2022 14:22
Comment threadpassword-hash/src/errors.rs Outdated
@daxpedda
daxpeddaforce-pushed the password-hash-output-size branch from ea87046 to a396237CompareJuly 24, 2022 15:53

@tarcieritarcieri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good to me now.

It's a breaking change though, so I might keep this open until we're ready to start working on the next version of password-hash.

@daxpedda

Copy link
Copy Markdown
ContributorAuthor

@tarcieri just saw the new update to 0.5.0-pre.0, are we ready to add this breaking change now?

Comment threadpassword-hash/src/errors.rs Outdated
Co-authored-by: Tony Arcieri <bascule@gmail.com>
@daxpedda
daxpeddaforce-pushed the password-hash-output-size branch from 70af450 to 91e2049CompareJanuary 9, 2023 16:13
@tarcieri
tarcieri merged commit e1c4d9d into RustCrypto:masterJan 9, 2023
@tarcieri

Copy link
Copy Markdown
Member

Thanks!

@tarcieritarcieri mentioned this pull request Mar 4, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@daxpedda@tarcieri