rust-crypto icon indicating copy to clipboard operation
rust-crypto copied to clipboard

Implement input_str() and result_str() convenience methods for Mac trait

Open DaGenix opened this issue 9 years ago • 3 comments

Closes #134

DaGenix avatar Dec 13 '14 04:12 DaGenix

This commit adds the same convenience methods to Mac that exist for Digest.

My concerns with doing this are:

  1. This is a bit of a specialized case. What if you want the result in base-64 or upper-case hex? These functions won't help you.
  2. When working with a Mac its important to use fixed time comparison functions to check the results. Making it easier to get to the result as a String instead of as a MacResult (which enforces fixed-time checks) might make it easier to make this type of mistake. On the other hand, I'm not sure that that theoretical reason to not have these functions justifies making it harder to work with Mac results as hex strings when that is a perfectly valid thing to do.

This commit makes Mac and Digest more similiar and Mac easier to work with for the case of string inputs and lower case hex formatted outputs, which does seem nice.

Anyway, I'll leave this open for at least a few days for comments.

Thoughts?

DaGenix avatar Dec 13 '14 04:12 DaGenix

Personally I'd just drop result_str completely. It's not too hard to call to_hex() yourself.

Having input_str() that does .as_bytes() and a result_str() that does to_hex() is asymmetric and confusing. Renaming result_str to result_hex or similar might help.

ebfe avatar Dec 13 '14 07:12 ebfe

I feel input_str() is not really subject to either of your concerns, but probably you're only talking about result_str() there. I agree with ebfe that calling to_hex() seems odd. As hex implies str, one might rename it to result_hex().

burdges avatar Sep 07 '16 11:09 burdges