From 5bcbe07dec3424c2ce038b853fbe8291ac53ae5a Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 10 Dec 2025 15:49:27 -0700 Subject: [PATCH] mcf: expand `Error` into a proper enum The old error type was a simple unit struct that couldn't express anything. This replaces it with an enum that can express various error conditions, including Base64 encoding errors, which might be helpful when trying to support a myriad of different Base64 encodings. --- mcf/src/error.rs | 47 +++++++++++++++++++++++++++++++++++++++++++---- mcf/src/fields.rs | 8 ++++---- mcf/src/lib.rs | 16 ++++++++-------- phc/src/error.rs | 10 +++++----- phc/src/salt.rs | 2 +- 5 files changed, 61 insertions(+), 22 deletions(-) diff --git a/mcf/src/error.rs b/mcf/src/error.rs index c6807f5a5..9a82d36e7 100644 --- a/mcf/src/error.rs +++ b/mcf/src/error.rs @@ -1,5 +1,6 @@ //! Error types. +use base64ct::Error as B64Error; use core::fmt; /// Result type for `mcf`. @@ -8,18 +9,56 @@ pub type Result = core::result::Result; /// Error type. #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[non_exhaustive] -pub struct Error {} +pub enum Error { + /// Base64 encoding errors. + Base64(B64Error), -impl core::error::Error for Error {} + /// `$` delimiter either missing or in an unexpected place + DelimiterInvalid, + + /// Encoding validation failure or error during encode time + EncodingInvalid, + + /// MCF field (between `$` characters) is not well-formed + FieldInvalid, + + /// MCF identifier missing + IdentifierMissing, + + /// MCF identifier invalid (must be `a-z`, `0-9`, or `-`) + IdentifierInvalid, +} + +impl core::error::Error for Error { + fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { + match self { + Error::Base64(e) => Some(e), + _ => None, + } + } +} impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("modular crypt format error") + match self { + Error::Base64(base64_err) => write!(f, "{base64_err}"), + Error::DelimiterInvalid => write!(f, "invalid use of `$` delimiter"), + Error::EncodingInvalid => write!(f, "invalid MCF encoding"), + Error::FieldInvalid => write!(f, "invalid MCF field (between `$` characters)"), + Error::IdentifierMissing => write!(f, "MCF identifier missing"), + Error::IdentifierInvalid => write!(f, "MCF identifier invalid"), + } + } +} + +impl From for Error { + fn from(base64_err: B64Error) -> Self { + Error::Base64(base64_err) } } impl From for Error { fn from(_: fmt::Error) -> Self { - Error {} + Error::EncodingInvalid } } diff --git a/mcf/src/fields.rs b/mcf/src/fields.rs index 52e47728d..f1447b7a5 100644 --- a/mcf/src/fields.rs +++ b/mcf/src/fields.rs @@ -74,25 +74,25 @@ impl<'a> Field<'a> { /// Decode Base64 into the provided output buffer. #[cfg(feature = "base64")] pub fn decode_base64_into(self, base64_variant: Base64, out: &mut [u8]) -> Result<&[u8]> { - base64_variant.decode(self.0, out).map_err(|_| Error {}) + Ok(base64_variant.decode(self.0, out)?) } /// Decode this field as the provided Base64 variant. #[cfg(all(feature = "alloc", feature = "base64"))] pub fn decode_base64(self, base64_variant: Base64) -> Result> { - base64_variant.decode_vec(self.0).map_err(|_| Error {}) + Ok(base64_variant.decode_vec(self.0)?) } /// Validate a field in the password hash is well-formed. pub(crate) fn validate(self) -> Result<()> { if self.0.is_empty() { - return Err(Error {}); + return Err(Error::FieldInvalid); } for c in self.0.chars() { match c { 'A'..='Z' | 'a'..='z' | '0'..='9' | '.' | '/' | '+' | '=' | ',' | '-' => (), - _ => return Err(Error {}), + _ => return Err(Error::EncodingInvalid), } } diff --git a/mcf/src/lib.rs b/mcf/src/lib.rs index bf02f16c1..c0a5495b6 100644 --- a/mcf/src/lib.rs +++ b/mcf/src/lib.rs @@ -184,7 +184,7 @@ mod allocating { pub fn push_displayable(&mut self, displayable: D) -> Result<()> { // TODO(tarcieri): avoid intermediate allocation? let mut buf = String::new(); - fmt::write(&mut buf, format_args!("{}", displayable))?; + fmt::write(&mut buf, format_args!("{displayable}"))?; self.push_str(&buf) } @@ -310,19 +310,19 @@ mod allocating { fn validate(s: &str) -> Result<()> { // Require leading `$` if !s.starts_with(fields::DELIMITER) { - return Err(Error {}); + return Err(Error::DelimiterInvalid); } // Disallow trailing `$` if s.ends_with(fields::DELIMITER) { - return Err(Error {}); + return Err(Error::DelimiterInvalid); } // Validates the hash begins with a leading `$` let mut fields = Fields::new(s); // Validate characters in the identifier field - let id = fields.next().ok_or(Error {})?; + let id = fields.next().ok_or(Error::IdentifierMissing)?; validate_id(id.as_str())?; // Validate the remaining fields have an appropriate format @@ -338,20 +338,20 @@ fn validate(s: &str) -> Result<()> { /// Allowed characters match the regex: `[a-z0-9\-]`, where the first and last characters do NOT /// contain a `-`. fn validate_id(id: &str) -> Result<()> { - let first = id.chars().next().ok_or(Error {})?; - let last = id.chars().last().ok_or(Error {})?; + let first = id.chars().next().ok_or(Error::IdentifierInvalid)?; + let last = id.chars().last().ok_or(Error::IdentifierInvalid)?; for c in [first, last] { match c { 'a'..='z' | '0'..='9' => (), - _ => return Err(Error {}), + _ => return Err(Error::IdentifierInvalid), } } for c in id.chars() { match c { 'a'..='z' | '0'..='9' | '-' => (), - _ => return Err(Error {}), + _ => return Err(Error::IdentifierInvalid), } } diff --git a/phc/src/error.rs b/phc/src/error.rs index 4d1d528ae..36f2bc37d 100644 --- a/phc/src/error.rs +++ b/phc/src/error.rs @@ -12,7 +12,7 @@ pub type Result = core::result::Result; #[non_exhaustive] pub enum Error { /// "B64" encoding error. - B64Encoding(B64Error), + Base64(B64Error), /// Password hash string invalid. MissingField, @@ -64,7 +64,7 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> core::result::Result<(), fmt::Error> { match self { - Self::B64Encoding(err) => write!(f, "{err}"), + Self::Base64(err) => write!(f, "{err}"), Self::MissingField => write!(f, "password hash string missing field"), Self::OutputSize { provided, expected } => match provided { Ordering::Less => write!( @@ -92,7 +92,7 @@ impl fmt::Display for Error { impl core::error::Error for Error { fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { match self { - Self::B64Encoding(err) => Some(err), + Self::Base64(err) => Some(err), _ => None, } } @@ -100,12 +100,12 @@ impl core::error::Error for Error { impl From for Error { fn from(err: B64Error) -> Error { - Error::B64Encoding(err) + Error::Base64(err) } } impl From for Error { fn from(_: base64ct::InvalidLengthError) -> Error { - Error::B64Encoding(B64Error::InvalidLength) + Error::Base64(B64Error::InvalidLength) } } diff --git a/phc/src/salt.rs b/phc/src/salt.rs index 1d64e45ff..98e5fb978 100644 --- a/phc/src/salt.rs +++ b/phc/src/salt.rs @@ -398,6 +398,6 @@ mod tests { fn reject_new_invalid_char() { let s = "01234_abcde"; let err = Salt::from_b64(s).err().unwrap(); - assert_eq!(err, Error::B64Encoding(base64ct::Error::InvalidEncoding)); + assert_eq!(err, Error::Base64(base64ct::Error::InvalidEncoding)); } }