password-hash: proposed API changes for Salt/SaltString#1216
password-hash: proposed API changes for Salt/SaltString#1216dns2utf8 wants to merge 1 commit intoRustCrypto:masterfrom
Salt/SaltString#1216Conversation
|
It looks like you're trying to do quite a few different things in a single PR. Please scope your PRs so they are either focused on a single crate or focused on making cross-cutting changes to all crates. For example, in this PR it would be good if you could just focus on the |
dns2utf8
force-pushed
the
rustify_the_api
branch
from
d4778e2 to
ead7d5e
Compare
|
Thanks for the feedback. Let's focus on the |
| /* | ||
| /// Borrow this value as bytes. | ||
| pub fn as_bytes(&self) -> &'a [u8] { | ||
| self.as_str().as_bytes() | ||
| } | ||
| */ |
There was a problem hiding this comment.
I removed this method because it is misleading in the rust context. Since this would leak a not ideal memory representation
| /// | ||
| /// [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md#argon2-encoding | ||
| pub fn b64_decode<'b>(&self, buf: &'b mut [u8]) -> Result<&'b [u8]> { | ||
| pub fn to_raw<'b>(&self, buf: &'b mut [u8]) -> Result<&'b [u8]> { |
There was a problem hiding this comment.
Not sure this name is an improvement
| /// Byte array containing an ASCiI-encoded string. | ||
| bytes: [u8; Salt::MAX_LENGTH], | ||
| /// Byte array containing an ASCiI-encoded string, length always matches the byte length. | ||
| characters: [u8; Salt::MAX_LENGTH], |
There was a problem hiding this comment.
I'd probably suggest the shorter chars
| /// Length of the string in ASCII characters (i.e. bytes). | ||
| length: u8, | ||
| /// Length of the string in ASCII characters in Bytes. | ||
| ascii_length: usize, |
There was a problem hiding this comment.
Not sure this is an improvement. ascii_length is the same as the length in bytes.
| /// Encode the given byte slice as B64 into a new [`SaltString`]. | ||
| /// | ||
| /// Returns `None` if the slice is too long. | ||
| pub fn from_raw(input: &[u8]) -> Result<Self> { |
There was a problem hiding this comment.
Again, not sure this is an improvement. Perhaps from_bytes?
| } | ||
|
|
||
| /// Decode this [`SaltString`] from B64 into the provided output buffer. | ||
| pub fn to_raw<'a>(&self, buf: &'a mut [u8]) -> Result<&'a [u8]> { |
| /// Borrow this value as bytes. | ||
| pub fn as_bytes(&self) -> &[u8] { | ||
| self.as_str().as_bytes() | ||
| pub fn as_b64_bytes(&self) -> &[u8] { |
There was a problem hiding this comment.
Not wild about this name either
tarcieri
changed the title
Salt/SaltString
|
I opened #1266 as an alternative |
2 participants
Making the API for Salt and SaltString better to understand
Initially opened the issue in the wrong repo, sorry for RustCrypto/password-hashes#371