mcf: expand Error into a proper enum

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<T> = core::result::Result<T, Error>;
 /// 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<B64Error> for Error {
+    fn from(base64_err: B64Error) -> Self {
+        Error::Base64(base64_err)
     }
 }
 
 impl From<fmt::Error> for Error {
     fn from(_: fmt::Error) -> Self {
-        Error {}
+        Error::EncodingInvalid
     }
 }

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<Vec<u8>> {
-        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),
             }
         }
 

mcf/src/lib.rs

@@ -184,7 +184,7 @@ mod allocating {
         pub fn push_displayable<D: fmt::Display>(&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),
         }
     }
 

phc/src/error.rs

@@ -12,7 +12,7 @@ pub type Result<T> = core::result::Result<T, Error>;
 #[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,20 +92,20 @@ 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,
         }
     }
 }
 
 impl From<B64Error> for Error {
     fn from(err: B64Error) -> Error {
-        Error::B64Encoding(err)
+        Error::Base64(err)
     }
 }
 
 impl From<base64ct::InvalidLengthError> for Error {
     fn from(_: base64ct::InvalidLengthError) -> Error {
-        Error::B64Encoding(B64Error::InvalidLength)
+        Error::Base64(B64Error::InvalidLength)
     }
 }

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));
     }
 }