Skip to content

Commit

Permalink
Merge #44: Hide all error internals
Browse files Browse the repository at this point in the history
af04208 Hide all errors (Tobin C. Harding)
881c098 Move error code to error module (Tobin C. Harding)

Pull request description:

  Before we do v1.0 we want to hide all the error internals so that we can modify/improve them without breaking the public API.

  Please pay particular attention to the `Display` strings because we they are considered part of the public API so we don't want to change them willy-nilly. Of note, because the enums have inner structs it was difficult to not duplicate the error message.

  Resolve: #42

ACKs for top commit:
  Kixunil:
    ACK af04208
  apoelstra:
    ACK af04208

Tree-SHA512: 3aa197c7d12d5081055e99bafd15ecb0d9ad3f374c7d32e9a98d3363cd0e455027f32d9e756668cbbba499a0f4028fcfe1ded1a6957105858ec7a1edc0325d22
  • Loading branch information
apoelstra committed Nov 7, 2023
2 parents 9c6df3e + af04208 commit 78df275
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 84 deletions.
2 changes: 1 addition & 1 deletion examples/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'a> fmt::UpperHex for DisplayALittleBitHexy<'a> {
}

/// Example Error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Error {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
Expand Down
146 changes: 146 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// SPDX-License-Identifier: CC0-1.0

use core::fmt;

use crate::write_err;

/// Formats error.
///
/// If `std` feature is OFF appends error source (delimited by `: `). We do this because
Expand All @@ -21,3 +25,145 @@ macro_rules! write_err {
}
}
}

/// Hex decoding error.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum HexToBytesError {
/// Non-hexadecimal character.
InvalidChar(InvalidCharError),
/// Purported hex string had odd length.
OddLengthString(OddLengthStringError),
}

impl fmt::Display for HexToBytesError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToBytesError::*;

match *self {
InvalidChar(ref e) => write_err!(f, "invalid char, failed to create bytes from hex"; e),
OddLengthString(ref e) =>
write_err!(f, "odd length, failed to create bytes from hex"; e),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToBytesError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToBytesError::*;

match *self {
InvalidChar(ref e) => Some(e),
OddLengthString(ref e) => Some(e),
}
}
}

impl From<InvalidCharError> for HexToBytesError {
#[inline]
fn from(e: InvalidCharError) -> Self { Self::InvalidChar(e) }
}

impl From<OddLengthStringError> for HexToBytesError {
#[inline]
fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) }
}

/// Invalid hex character.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub struct InvalidCharError {
pub(crate) invalid: u8,
}

impl fmt::Display for InvalidCharError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "invalid hex char {}", self.invalid)
}
}

#[cfg(feature = "std")]
impl std::error::Error for InvalidCharError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

/// Purported hex string had odd length.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub struct OddLengthStringError {
pub(crate) len: usize,
}

impl fmt::Display for OddLengthStringError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "odd hex string length {}", self.len)
}
}

#[cfg(feature = "std")]
impl std::error::Error for OddLengthStringError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

/// Hex decoding error.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum HexToArrayError {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
/// Tried to parse fixed-length hash from a string with the wrong length (got, want).
InvalidLength(InvalidLengthError),
}

impl fmt::Display for HexToArrayError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToArrayError::*;

match *self {
Conversion(ref e) =>
crate::write_err!(f, "conversion error, failed to create array from hex"; e),
InvalidLength(ref e) =>
write_err!(f, "invalid length, failed to create array from hex"; e),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToArrayError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToArrayError::*;

match *self {
Conversion(ref e) => Some(e),
InvalidLength(ref e) => Some(e),
}
}
}

impl From<HexToBytesError> for HexToArrayError {
#[inline]
fn from(e: HexToBytesError) -> Self { Self::Conversion(e) }
}

impl From<InvalidLengthError> for HexToArrayError {
#[inline]
fn from(e: InvalidLengthError) -> Self { Self::InvalidLength(e) }
}

/// Tried to parse fixed-length hash from a string with the wrong length.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub struct InvalidLengthError {
pub(crate) expected: usize,
pub(crate) got: usize,
}

impl fmt::Display for InvalidLengthError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "bad hex string length {} (expected {})", self.got, self.expected)
}
}

#[cfg(feature = "std")]
impl std::error::Error for InvalidLengthError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}
11 changes: 7 additions & 4 deletions src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use std::io;
#[cfg(all(feature = "core2", not(feature = "std")))]
use core2::io;

use crate::parse::HexToBytesError;
use crate::error::{InvalidCharError, OddLengthStringError};

#[rustfmt::skip] // Keep public re-exports separate.
pub use crate::error::HexToBytesError;

/// Iterator over a hex-encoded string slice which decodes hex and yields bytes.
pub struct HexToBytesIter<'a> {
Expand All @@ -33,7 +36,7 @@ impl<'a> HexToBytesIter<'a> {
#[inline]
pub fn new(s: &'a str) -> Result<HexToBytesIter<'a>, HexToBytesError> {
if s.len() % 2 != 0 {
Err(HexToBytesError::OddLengthString(s.len()))
Err(OddLengthStringError { len: s.len() }.into())
} else {
Ok(HexToBytesIter { iter: s.bytes() })
}
Expand Down Expand Up @@ -93,8 +96,8 @@ impl<'a> io::Read for HexToBytesIter<'a> {

/// `hi` and `lo` are bytes representing hex characters.
fn hex_chars_to_byte(hi: u8, lo: u8) -> Result<u8, HexToBytesError> {
let hih = (hi as char).to_digit(16).ok_or(HexToBytesError::InvalidChar(hi))?;
let loh = (lo as char).to_digit(16).ok_or(HexToBytesError::InvalidChar(lo))?;
let hih = (hi as char).to_digit(16).ok_or(InvalidCharError { invalid: hi })?;
let loh = (lo as char).to_digit(16).ok_or(InvalidCharError { invalid: lo })?;

let ret = (hih << 4) + loh;
Ok(ret as u8)
Expand Down
94 changes: 15 additions & 79 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ use core::{fmt, str};

#[cfg(all(feature = "alloc", not(feature = "std")))]
use crate::alloc::vec::Vec;
use crate::error::InvalidLengthError;
use crate::iter::HexToBytesIter;

#[rustfmt::skip] // Keep public re-exports separate.
pub use crate::error::{HexToBytesError, HexToArrayError};

/// Trait for objects that can be deserialized from hex strings.
pub trait FromHex: Sized {
/// Error type returned while parsing hex string.
Expand Down Expand Up @@ -37,37 +41,6 @@ impl FromHex for Vec<u8> {
}
}

/// Hex decoding error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum HexToBytesError {
/// Non-hexadecimal character.
InvalidChar(u8),
/// Purported hex string had odd length.
OddLengthString(usize),
}

impl fmt::Display for HexToBytesError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToBytesError::*;

match *self {
InvalidChar(ch) => write!(f, "invalid hex character {}", ch),
OddLengthString(ell) => write!(f, "odd hex string length {}", ell),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToBytesError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToBytesError::*;

match self {
InvalidChar(_) | OddLengthString(_) => None,
}
}
}

macro_rules! impl_fromhex_array {
($len:expr) => {
impl FromHex for [u8; $len] {
Expand All @@ -86,7 +59,7 @@ macro_rules! impl_fromhex_array {
}
Ok(ret)
} else {
Err(HexToArrayError::InvalidLength(2 * $len, 2 * iter.len()))
Err(InvalidLengthError { expected: 2 * $len, got: 2 * iter.len() }.into())
}
}
}
Expand All @@ -113,67 +86,28 @@ impl_fromhex_array!(256);
impl_fromhex_array!(384);
impl_fromhex_array!(512);

/// Hex decoding error.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum HexToArrayError {
/// Conversion error while parsing hex string.
Conversion(HexToBytesError),
/// Tried to parse fixed-length hash from a string with the wrong length (got, want).
InvalidLength(usize, usize),
}

impl fmt::Display for HexToArrayError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use HexToArrayError::*;

match *self {
Conversion(ref e) => crate::write_err!(f, "conversion error"; e),
InvalidLength(got, want) =>
write!(f, "bad hex string length {} (expected {})", got, want),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for HexToArrayError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
use HexToArrayError::*;

match *self {
Conversion(ref e) => Some(e),
InvalidLength(_, _) => None,
}
}
}

impl From<HexToBytesError> for HexToArrayError {
#[inline]
fn from(e: HexToBytesError) -> Self { Self::Conversion(e) }
}

#[cfg(test)]
mod tests {
use super::*;
use crate::display::DisplayHex;
use crate::error::{InvalidCharError, InvalidLengthError, OddLengthStringError};

#[test]
#[cfg(feature = "alloc")]
fn hex_error() {
use HexToBytesError::*;

let oddlen = "0123456789abcdef0";
let badchar1 = "Z123456789abcdef";
let badchar2 = "012Y456789abcdeb";
let badchar3 = "«23456789abcdef";

assert_eq!(Vec::<u8>::from_hex(oddlen), Err(OddLengthString(17)));
assert_eq!(Vec::<u8>::from_hex(oddlen), Err(OddLengthStringError { len: 17 }.into()));
assert_eq!(
<[u8; 4]>::from_hex(oddlen),
Err(HexToArrayError::Conversion(OddLengthString(17)))
Err(HexToBytesError::OddLengthString(OddLengthStringError { len: 17 }).into())
);
assert_eq!(Vec::<u8>::from_hex(badchar1), Err(InvalidChar(b'Z')));
assert_eq!(Vec::<u8>::from_hex(badchar2), Err(InvalidChar(b'Y')));
assert_eq!(Vec::<u8>::from_hex(badchar3), Err(InvalidChar(194)));
assert_eq!(Vec::<u8>::from_hex(badchar1), Err(InvalidCharError { invalid: b'Z' }.into()));
assert_eq!(Vec::<u8>::from_hex(badchar2), Err(InvalidCharError { invalid: b'Y' }.into()));
assert_eq!(Vec::<u8>::from_hex(badchar3), Err(InvalidCharError { invalid: 194 }.into()));
}

#[test]
Expand All @@ -183,9 +117,11 @@ mod tests {
}
#[test]
fn hex_to_array_error() {
use HexToArrayError::*;
let len_sixteen = "0123456789abcdef";
assert_eq!(<[u8; 4]>::from_hex(len_sixteen), Err(InvalidLength(8, 16)));
assert_eq!(
<[u8; 4]>::from_hex(len_sixteen),
Err(InvalidLengthError { expected: 8, got: 16 }.into())
)
}

#[test]
Expand Down

0 comments on commit 78df275

Please sign in to comment.