Add fallible hex decoding for Uint#247
Add fallible hex decoding for Uint#247andrewwhitehead wants to merge 1 commit intoRustCrypto:masterfrom
Conversation
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
| /// Check that the input consists of hexadecimal characters, | ||
| /// short-circuiting on invalid input. | ||
| const fn validate_hex(bytes: &[u8]) -> Option<usize> { | ||
| if bytes.len() % 2 == 1 { | ||
| return None; | ||
| } | ||
| let mut i = 0; | ||
| while i < bytes.len() { | ||
| if !matches!(bytes[i], b'0'..=b'9' | b'a' ..= b'f' | b'A'..=b'F') { | ||
| return None; | ||
| } | ||
| i += 1; | ||
| } | ||
| Some(bytes.len() / 2) | ||
| } |
There was a problem hiding this comment.
This is not constant-time.
Perhaps you could change it to be? Or otherwise you need to make the methods *_vartime to honor the "constant time by default" contract of this library, but I would prefer you just make the implementation constant-time instead.
It should also return CtOption rather than Option.
There was a problem hiding this comment.
I don't really see how this can be constant time overall, since the implementation short circuits on invalid input, and I'm not sure what the alternative would be. It would also be nice for it to handle leading zeros on hex values, as well as trimmed values (f.ex. parsing "FF" into a U256) but in that case is it meant to be constant time in the input length or in the number's representation?
There was a problem hiding this comment.
the implementation short circuits on invalid input, and I'm not sure what the alternative would be.
You can defer checking an error condition until the end of the data, which is what from_be_hex does already:
https://github.com/RustCrypto/crypto-bigint/blob/2122b4a/src/uint/encoding.rs#L55-L56
|
Closing as stale |
2 participants
This is mostly for convenience, although it can be irritating to validate hex inputs outside of the library.
Perhaps it would be nice to support hex strings of different lengths?