Skip to content

Commit

Permalink
Implement TryFrom various integer types
Browse files Browse the repository at this point in the history
Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid
value encountered while converting to a `u8`. This is buggy because we
cast to the `u8` to stash the value so we loose valuable bits.

Implement `TryFrom` by doing:
- Add an `Error::Overflow` variant.
- Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls
- Add a new `TryFromError` and nest it in `Error::TryFrom`
- Implement `TryFrom<T> for u5` impls for all standard integer types.

Inspired by issue rust-bitcoin#60 and the stdlib macros of the same name.
  • Loading branch information
tcharding committed Mar 3, 2023
1 parent 7ab12fc commit 25462c9
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 18 deletions.
31 changes: 31 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Written by the Rust Bitcoin developers.
// SPDX-License-Identifier: CC0-1.0

// TODO: We can get this from `bitcoin-internals` crate once that is released.

//! # Error
//!
//! Error handling macros and helpers.
//!

/// Formats error.
///
/// If `std` feature is OFF appends error source (delimited by `: `). We do this because
/// `e.source()` is only available in std builds, without this macro the error source is lost for
/// no-std builds.
#[macro_export]
macro_rules! write_err {
($writer:expr, $string:literal $(, $args:expr)*; $source:expr) => {
{
#[cfg(feature = "std")]
{
let _ = &$source; // Prevents clippy warnings.
write!($writer, $string $(, $args)*)
}
#[cfg(not(feature = "std"))]
{
write!($writer, concat!($string, ": {}") $(, $args)*, $source)
}
}
}
}
136 changes: 118 additions & 18 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ use core::{fmt, mem};
#[cfg(feature = "std")]
use std::borrow::Cow;

mod error;

/// Integer in the range `0..32`
#[derive(PartialEq, Eq, Debug, Copy, Clone, Default, PartialOrd, Ord, Hash)]
#[allow(non_camel_case_types)]
Expand All @@ -91,18 +93,56 @@ impl From<u5> for u8 {
fn from(v: u5) -> u8 { v.0 }
}

impl TryFrom<u8> for u5 {
type Error = Error;

/// Errors if `value` is out of range.
fn try_from(value: u8) -> Result<Self, Self::Error> {
if value > 31 {
Err(Error::InvalidData(value))
} else {
Ok(u5(value))
}
macro_rules! impl_try_from_upper_bounded {
($($ty:ident)+) => {
$(
impl TryFrom<$ty> for u5 {
type Error = TryFromIntError;

/// Tries to create the target number type from a source number type.
///
/// # Errors
///
/// Returns an error if `value` overflows a `u5`.
fn try_from(value: $ty) -> Result<Self, Self::Error> {
if value > 31 {
Err(TryFromIntError::PosOverflow)
} else {
let x = u8::try_from(value).expect("within range");
Ok(u5(x))
}
}
}
)+
}
}
macro_rules! impl_try_from_both_bounded {
($($ty:ident)+) => {
$(
impl TryFrom<$ty> for u5 {
type Error = TryFromIntError;

/// Tries to create the target number type from a source number type.
///
/// # Errors
///
/// Returns an error if `value` is outside of the range of a `u5`.
fn try_from(value: $ty) -> Result<Self, Self::Error> {
if value < 0 {
Err(TryFromIntError::NegOverflow)
} else if value > 31 {
Err(TryFromIntError::PosOverflow)
} else {
let x = u8::try_from(value).expect("within range");
Ok(u5(x))
}
}
}
)+
}
}
impl_try_from_upper_bounded!(u8 u16 u32 u64 u128);
impl_try_from_both_bounded!(i8 i16 i32 i64 i128);

impl AsRef<u8> for u5 {
fn as_ref(&self) -> &u8 { &self.0 }
Expand Down Expand Up @@ -340,7 +380,10 @@ impl<T: AsRef<[u8]>> CheckBase32<Vec<u5>> for T {
type Err = Error;

fn check_base32(self) -> Result<Vec<u5>, Self::Err> {
self.as_ref().iter().map(|x| u5::try_from(*x)).collect::<Result<Vec<u5>, Error>>()
self.as_ref()
.iter()
.map(|x| u5::try_from(*x).map_err(Error::TryFrom))
.collect::<Result<Vec<u5>, Error>>()
}
}

Expand Down Expand Up @@ -669,12 +712,14 @@ pub enum Error {
InvalidLength,
/// Some part of the string contains an invalid character.
InvalidChar(char),
/// Some part of the data has an invalid value.
InvalidData(u8),
/// The bit conversion failed due to a padding issue.
InvalidPadding,
/// The whole string must be of one case.
MixedCase,
/// Attempted to convert a value which overflows a `u5`.
Overflow,
/// Conversion to u5 failed.
TryFrom(TryFromIntError),
}

impl fmt::Display for Error {
Expand All @@ -686,9 +731,10 @@ impl fmt::Display for Error {
InvalidChecksum => write!(f, "invalid checksum"),
InvalidLength => write!(f, "invalid length"),
InvalidChar(n) => write!(f, "invalid character (code={})", n),
InvalidData(n) => write!(f, "invalid data point ({})", n),
InvalidPadding => write!(f, "invalid padding"),
MixedCase => write!(f, "mixed-case strings not allowed"),
TryFrom(ref e) => write_err!(f, "conversion to u5 failed"; e),
Overflow => write!(f, "attempted to convert a value which overflows a u5"),
}
}
}
Expand All @@ -699,12 +745,54 @@ impl std::error::Error for Error {
use Error::*;

match *self {
TryFrom(ref e) => Some(e),
MissingSeparator | InvalidChecksum | InvalidLength | InvalidChar(_)
| InvalidData(_) | InvalidPadding | MixedCase => None,
| InvalidPadding | MixedCase | Overflow => None,
}
}
}

impl From<TryFromIntError> for Error {
fn from(e: TryFromIntError) -> Self { Error::TryFrom(e) }
}

/// Error return when TryFrom<T> fails for T -> u5 conversion.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub enum TryFromIntError {
/// Attempted to convert a negative value to a `u5`.
NegOverflow,
/// Attempted to convert a value which overflows a `u5`.
PosOverflow,
}

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

match *self {
NegOverflow => write!(f, "attempted to convert a negative value to a u5"),
PosOverflow => write!(f, "attempted to convert a value which overflows a u5"),
}
}
}

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

match *self {
NegOverflow | PosOverflow => None,
}
}
}

// impl From<convert::Error> for Error {
// fn from(e: convert::Error) -> Self {
// Error::InvalidData(e)
// }
// }

/// Convert between bit sizes
///
/// # Errors
Expand Down Expand Up @@ -738,7 +826,7 @@ where
let v: u32 = u32::from(Into::<u8>::into(*value));
if (v >> from) != 0 {
// Input value exceeds `from` bit size
return Err(Error::InvalidData(v as u8));
return Err(Error::Overflow);
}
acc = (acc << from) | v;
bits += from;
Expand Down Expand Up @@ -885,7 +973,7 @@ mod tests {
// Set of [data, from_bits, to_bits, pad, expected error]
let tests: Vec<(Vec<u8>, u32, u32, bool, Error)> = vec![
(vec![0xff], 8, 5, false, Error::InvalidPadding),
(vec![0x02], 1, 1, true, Error::InvalidData(0x02)),
(vec![0x02], 1, 1, true, Error::Overflow),
];
for t in tests {
let (data, from_bits, to_bits, pad, expected_error) = t;
Expand Down Expand Up @@ -918,7 +1006,10 @@ mod tests {
assert!([0u8, 1, 2, 30, 31, 255].check_base32().is_err());

assert!([1u8, 2, 3, 4].check_base32().is_ok());
assert_eq!([30u8, 31, 35, 20].check_base32(), Err(Error::InvalidData(35)));
assert_eq!(
[30u8, 31, 35, 20].check_base32(),
Err(Error::TryFrom(TryFromIntError::PosOverflow))
)
}

#[test]
Expand Down Expand Up @@ -1028,4 +1119,13 @@ mod tests {

assert_eq!(encoded_str, "hrp1qqqq40atq3");
}

#[test]
fn try_from_err() {
assert!(u5::try_from(32_u8).is_err());
assert!(u5::try_from(32_u16).is_err());
assert!(u5::try_from(32_u32).is_err());
assert!(u5::try_from(32_u64).is_err());
assert!(u5::try_from(32_u128).is_err());
}
}

0 comments on commit 25462c9

Please sign in to comment.