-
Notifications
You must be signed in to change notification settings - Fork 215
digest: add FixedOutputDirty
trait + finalize_into*
#180
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
Conversation
@newpavlov this is slightly different from what you originally proposed, but I think achieves the same effect. Namely if you wanted something exactly like what you had before, it could be achieved with: impl MyDigest {
/// Private method not intended to be used directly
fn finalize_fixed_core(&mut self, out: &mut GenericArray<u8, Self::OutputSize>, reset: bool) {
[...]
}
}
impl FixedOutput for MyDigest{
type OutputSize = [...];
#[inline]
fn finalize_fixed(self) -> GenericArray<u8, Self::OutputSize> {
let mut buf = Default::default();
self.finalize_fixed_core(&mut buf, false);
buf
}
#[inline]
fn finalize_fixed_reset(&mut self) -> GenericArray<u8, Self::OutputSize> { .. }
fn finalize_fixed_into(self, out: &mut GenericArray<u8, Self::OutputSize>) { .. }
fn finalize_fixed_into_reset(&mut self, out: &mut GenericArray<u8, Self::OutputSize>) { .. }
} ...it's just by not making it part of the trait, it could actually be a private method. |
47d5f10
to
a2f119e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One problem with this approach is that it makes harder for compiler to eliminate reset code, this is why I was thinking about using the private method, which would keep reset code behind a simple branch which can eliminated by trivial const propagation. Before going with this approach, it would be nice to see that compiler is indeed will be able to properly remove reset code for finalize_fixed
.
That's only if you rely on the default implementation though. If you want to avoid the resets entirely, just define your own implementation (per above) which doesn't invoke the reset code at all. |
This will lead to unnecessary code duplication, which I would like to avoid. How about something like this? pub trait FixedOutput {
// ...
/// Retrieve result into provided buffer and leave hasher in a dirty state.
/// Usage of this method in user code is discouraged, prefer `finalize_fixed`
/// or `finalize_fixed_reset`.
fn finalize_into_dirty(&mut self, out: &mut GenericArray<u8, Self::OutputSize>);
fn finalize_into_reset(&mut self, out: &mut GenericArray<u8, Self::OutputSize>)
where
Self: Reset,
{
self.finalize_into_dirty(out);
self.reset();
}
}
pub trait Reset {
fn reset(&mut self);
} Yes, users may misuse |
I think you could achieve something like that with a separate trait and a blanket impl: pub trait FixedOutputDirty {
/// Retrieve result into provided buffer and leave hasher in a dirty state.
/// Usage of this method in user code is discouraged, prefer `finalize_fixed`
/// or `finalize_fixed_reset`.
fn finalize_into_dirty(&mut self, out: &mut GenericArray<u8, Self::OutputSize>);
}
impl<D: FixedOutputDirty + Reset> FixedOutput for D {
#[inline]
fn finalize_into(&mut self, out: &mut GenericArray<u8, Self::OutputSize>)
where
Self: Reset,
{
self.finalize_into_dirty(out);
}
#[inline]
fn finalize_into_reset(&mut self, out: &mut GenericArray<u8, Self::OutputSize>)
where
Self: Reset,
{
self.finalize_into_dirty(out);
self.reset();
}
} |
Also should I shorten |
The separate trait looks good, making room for
Yes, sounds good. |
a2f119e
to
7ef7bee
Compare
Reset
trait with methods; add *into* methodsFixedOutputDirty
trait + finalize_into*
7ef7bee
to
daf6cfb
Compare
Updated to use a |
daf6cfb
to
4f53f96
Compare
Adds a `FixedOutputDirty` trait which writes the digest output to a provided byte array, but does not reset the internal state. This is intended for implementations to use in order to ensure that they are not reset in the event the instance is consumed. Also adds a set of `finalize_into` and `finalize_into_reset` methods to `FixedOutput` whhich also write their input into a provided byte array, and changes the existing `finalize_fixed` (and newly added `finalize_fixed_reset`) methods to have a default implementation which returns a byte array allocated on the stack. Finally, adds a blanket impl of `FixedOutput` for `FixedOutputDirty` + `Reset` types which handles safely invoking the underlying implementation by either consuming the instance (avoiding a reset) or borrowing the hasher, obtaining the output, and resetting.
4f53f96
to
2526a6a
Compare
BTW should we do something similar for XOF and |
Sure |
Adds traits which provide an implementation-focused low-level API similar to the `FixedOutputDirty` trait introduced in #180, but for the `ExtendableOutput` and `VariableOutput` traits. Like `FixedOutputDirty`, these traits have blanket impls when the same type also impls the `Reset` trait. Also adds corresponding `*_reset` methods to the respective `finalize*` methods of these traits.
…183) Adds traits which provide an implementation-focused low-level API similar to the `FixedOutputDirty` trait introduced in #180, but for the `ExtendableOutput` and `VariableOutput` traits. Like `FixedOutputDirty`, these traits have blanket impls when the same type also impls the `Reset` trait. Also adds corresponding `*_reset` methods to the respective `finalize*` methods of these traits.
Adds a proper conversion between argon2::Error and password_hash::Error and uses it when hashing the password.
Closes RustCrypto/hashes#86
Replaces theReset
trait with a set of_reset
methods on all of the various traits, and uses these as the default impl for the methods which consume hashers.Also adds a set of methods toFixedOutput
(finalize_fixed_into*
) which eliminate copies by accepting an existing buffer as an argument.Adds a
FixedOutputDirty
trait which writes the digest output to a provided byte array, but does not reset the internal state. This is intended for implementations to use in order to ensure that they are not reset in the event the instance is consumed.Also adds a set of
finalize_into
andfinalize_into_reset
methods toFixedOutput
whhich also write their input into a provided byte array, and changes the existingfinalize_fixed
(and newly addedfinalize_fixed_reset
) methods to have a default implementation which returns a byte array allocated on the stack.Finally, adds a blanket impl of
FixedOutput
forFixedOutputDirty
+Reset
types which handles safely invoking the underlying implementation by either consuming the instance (avoiding a reset) or borrowing the hasher, obtaining the output, and resetting.