Add more details to ParamValueInvalid and introduce SaltInvalid error

password-hash/src/errors.rs

@@ -33,7 +33,7 @@ pub enum Error {
     ParamNameInvalid,
 
     /// Invalid parameter value.
-    ParamValueInvalid,
+    ParamValueInvalid(ParamValueError),
 
     /// Maximum number of parameters exceeded.
     ParamsMaxExceeded,
@@ -56,10 +56,23 @@ pub enum Error {
     /// Salt too long.
     SaltTooLong,
 
+    /// Salt invalid.
+    SaltInvalid,
+
     /// Invalid algorithm version.
     Version,
 }
 
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum ParamValueError {
+    ToLong,
+    ToShort,
+    NotProvided,
+    InvalidChar,
+    InvalidFormat,
+}
+
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> core::result::Result<(), fmt::Error> {
         match self {
@@ -70,14 +83,27 @@ impl fmt::Display for Error {
             Self::OutputTooLong => f.write_str("PHF output too long (max 64-bytes)"),
             Self::ParamNameDuplicated => f.write_str("duplicate parameter"),
             Self::ParamNameInvalid => f.write_str("invalid parameter name"),
-            Self::ParamValueInvalid => f.write_str("invalid parameter value"),
+            Self::ParamValueInvalid(param_err) => match param_err {
+                ParamValueError::ToLong => f.write_str("invalid parameter value: value to long"),
+                ParamValueError::ToShort => f.write_str("invalid parameter value: value to short"),
+                ParamValueError::NotProvided => {
+                    f.write_str("invalid parameter value: required value not provided")
+                }
+                ParamValueError::InvalidChar => {
+                    f.write_str("invalid parameter value: contains invalid character")
+                }
+                ParamValueError::InvalidFormat => {
+                    f.write_str("invalid parameter value: value format is invalid")
+                }
+            },
             Self::ParamsMaxExceeded => f.write_str("maximum number of parameters reached"),
             Self::Password => write!(f, "invalid password"),
             Self::PhcStringInvalid => write!(f, "password hash string invalid"),
             Self::PhcStringTooShort => write!(f, "password hash string too short"),
             Self::PhcStringTooLong => write!(f, "password hash string too long"),
             Self::SaltTooShort => write!(f, "salt too short"),
             Self::SaltTooLong => write!(f, "salt too long"),
+            Self::SaltInvalid => write!(f, "salt invalid"),
             Self::Version => write!(f, "invalid algorithm version"),
         }
     }

password-hash/src/params.rs

@@ -1,5 +1,6 @@
 //! Algorithm parameters.
 
+use crate::errors::ParamValueError;
 use crate::{
     value::{Decimal, Value},
     Error, Ident, Result,
@@ -56,7 +57,9 @@ impl ParamsString {
         value: impl TryInto<Value<'a>>,
     ) -> Result<()> {
         let name = name.try_into().map_err(|_| Error::ParamNameInvalid)?;
-        let value = value.try_into().map_err(|_| Error::ParamValueInvalid)?;
+        let value = value
+            .try_into()
+            .map_err(|_| Error::ParamValueInvalid(ParamValueError::InvalidFormat))?;
         self.add(name, value)
     }
 
@@ -162,11 +165,11 @@ impl FromStr for ParamsString {
             // Validate value
             param
                 .next()
-                .ok_or(Error::ParamValueInvalid)
+                .ok_or(Error::ParamValueInvalid(ParamValueError::NotProvided))
                 .and_then(Value::try_from)?;
 
             if param.next().is_some() {
-                return Err(Error::ParamValueInvalid);
+                return Err(Error::ParamValueInvalid(ParamValueError::NotProvided));
             }
         }
 

password-hash/src/salt.rs

@@ -6,6 +6,7 @@ use core::{
     fmt, str,
 };
 
+use crate::errors::ParamValueError;
 #[cfg(feature = "rand_core")]
 use rand_core::{CryptoRng, RngCore};
 
@@ -106,7 +107,7 @@ impl<'a> Salt<'a> {
             return Err(Error::SaltTooLong);
         }
 
-        input.try_into().map(Self)
+        input.try_into().map(Self).map_err(map_error_to_salt_error)
     }
 
     /// Attempt to decode a B64-encoded [`Salt`], writing the decoded result
@@ -134,6 +135,17 @@ impl<'a> Salt<'a> {
     }
 }
 
+fn map_error_to_salt_error(err: Error) -> Error {
+    match err {
+        Error::ParamValueInvalid(value_err) => match value_err {
+            ParamValueError::ToLong => Error::SaltTooLong,
+            ParamValueError::ToShort | ParamValueError::NotProvided => Error::SaltTooShort,
+            _ => Error::SaltInvalid,
+        },
+        _ => Error::SaltInvalid,
+    }
+}
+
 impl<'a> AsRef<str> for Salt<'a> {
     fn as_ref(&self) -> &str {
         self.as_str()
@@ -257,6 +269,7 @@ impl<'a> From<&'a SaltString> for Salt<'a> {
 #[cfg(test)]
 mod tests {
     use super::{Error, Salt};
+    use crate::errors::ParamValueError;
 
     #[test]
     fn new_with_valid_min_length_input() {
@@ -286,4 +299,11 @@ mod tests {
         let err = Salt::new(s).err().unwrap();
         assert_eq!(err, Error::SaltTooLong);
     }
+
+    #[test]
+    fn reject_new_invalid_char() {
+        let s = "01234_abcd";
+        let err = Salt::new(s).err().unwrap();
+        assert_eq!(err, Error::SaltInvalid);
+    }
 }

password-hash/src/value.rs

@@ -13,6 +13,7 @@
 //!
 //! [1]: https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md
 
+use crate::errors::ParamValueError;
 use crate::{Encoding, Error, Result};
 use core::{convert::TryFrom, fmt, str};
 
@@ -50,7 +51,7 @@ impl<'a> Value<'a> {
     /// the PHC string format's rules.
     pub fn new(input: &'a str) -> Result<Self> {
         if input.as_bytes().len() > Self::MAX_LENGTH {
-            return Err(Error::ParamValueInvalid);
+            return Err(Error::ParamValueInvalid(ParamValueError::ToLong));
         }
 
         // Check that the characters are permitted in a PHC parameter value.
@@ -124,26 +125,26 @@ impl<'a> Value<'a> {
 
         // Empty strings aren't decimals
         if value.is_empty() {
-            return Err(Error::ParamValueInvalid);
+            return Err(Error::ParamValueInvalid(ParamValueError::NotProvided));
         }
 
         // Ensure all characters are digits
         for c in value.chars() {
             if !matches!(c, '0'..='9') {
-                return Err(Error::ParamValueInvalid);
+                return Err(Error::ParamValueInvalid(ParamValueError::InvalidChar));
             }
         }
 
         // Disallow leading zeroes
         if value.starts_with('0') && value.len() > 1 {
-            return Err(Error::ParamValueInvalid);
+            return Err(Error::ParamValueInvalid(ParamValueError::InvalidFormat));
         }
 
         value.parse().map_err(|_| {
             // In theory a value overflow should be the only potential error here.
             // When `ParseIntError::kind` is stable it might be good to double check:
             // <https://github.com/rust-lang/rust/issues/22639>
-            Error::ParamValueInvalid
+            Error::ParamValueInvalid(ParamValueError::InvalidFormat)
         })
     }
 
@@ -193,7 +194,7 @@ impl<'a> fmt::Display for Value<'a> {
 fn assert_valid_value(input: &str) -> Result<()> {
     for c in input.chars() {
         if !is_char_valid(c) {
-            return Err(Error::ParamValueInvalid);
+            return Err(Error::ParamValueInvalid(ParamValueError::InvalidChar));
         }
     }
 
@@ -207,7 +208,7 @@ fn is_char_valid(c: char) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::{Error, Value};
+    use super::{Error, ParamValueError, Value};
     use core::convert::TryFrom;
 
     // Invalid value examples
@@ -236,21 +237,30 @@ mod tests {
     fn reject_decimal_with_leading_zero() {
         let value = Value::new("01").unwrap();
         let err = u32::try_from(value).err().unwrap();
-        assert!(matches!(err, Error::ParamValueInvalid));
+        assert!(matches!(
+            err,
+            Error::ParamValueInvalid(ParamValueError::InvalidFormat)
+        ));
     }
 
     #[test]
     fn reject_overlong_decimal() {
         let value = Value::new("4294967296").unwrap();
         let err = u32::try_from(value).err().unwrap();
-        assert_eq!(err, Error::ParamValueInvalid);
+        assert_eq!(
+            err,
+            Error::ParamValueInvalid(ParamValueError::InvalidFormat)
+        );
     }
 
     #[test]
     fn reject_negative() {
         let value = Value::new("-1").unwrap();
         let err = u32::try_from(value).err().unwrap();
-        assert!(matches!(err, Error::ParamValueInvalid));
+        assert!(matches!(
+            err,
+            Error::ParamValueInvalid(ParamValueError::InvalidChar)
+        ));
     }
 
     //
@@ -278,18 +288,21 @@ mod tests {
     #[test]
     fn reject_invalid_char() {
         let err = Value::new(INVALID_CHAR).err().unwrap();
-        assert!(matches!(err, Error::ParamValueInvalid));
+        assert!(matches!(
+            err,
+            Error::ParamValueInvalid(ParamValueError::InvalidChar)
+        ));
     }
 
     #[test]
     fn reject_too_long() {
         let err = Value::new(INVALID_TOO_LONG).err().unwrap();
-        assert_eq!(err, Error::ParamValueInvalid);
+        assert_eq!(err, Error::ParamValueInvalid(ParamValueError::ToLong));
     }
 
     #[test]
     fn reject_invalid_char_and_too_long() {
         let err = Value::new(INVALID_CHAR_AND_TOO_LONG).err().unwrap();
-        assert_eq!(err, Error::ParamValueInvalid);
+        assert_eq!(err, Error::ParamValueInvalid(ParamValueError::ToLong));
     }
 }