Error on leading zeros when decoding indices (#66)

* adds error variant to account for leading zeros error
chanced · Aug 6, 2024 · a2453aa · a2453aa
1 parent e6c64f8
commit a2453aa
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+-   `Token::to_index` now fails if the token contains leading zeros, as mandated by the RFC.
+
+### Changed
+
+-   `ParseIndexError` is now an enum to reflect the new failure mode when parsing indices.
+
+## [0.5.1]
+
+### Changed
+
+-   README tweak.
+
+## [0.5.0] 
+
 This is a breaking release including:
 
 -   [#30](https://github.com/chanced/jsonptr/pull/30) and [#37](https://github.com/chanced/jsonptr/pull/37) by [@asmello](https://github.com/asmello)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -9,7 +9,7 @@ license       = "MIT OR Apache-2.0"
 name          = "jsonptr"
 repository    = "https://github.com/chanced/jsonptr"
 rust-version  = "1.76.0"
-version       = "0.5.1"
+version       = "0.6.0"
 
 [dependencies]
 serde      = { version = "1.0.203", optional = true, features = ["alloc"] }

diff --git a/src/assign.rs b/src/assign.rs
@@ -11,7 +11,7 @@
 //! in the case of arrays, or a scalar value (including `null`) based upon a
 //! best-guess effort on the meaning of each [`Token`](crate::Token):
 //! - If the [`Token`](crate::Token) is equal to `"0"` or `"-"`, the token will
-//!  be considered an index of an array.
+//!   be considered an index of an array.
 //! - All tokens not equal to `"0"` or `"-"` will be considered keys of an
 //!   object.
 //!
@@ -63,7 +63,7 @@ use core::fmt::{self, Debug};
 /// effort on the meaning of each [`Token`](crate::Token):
 ///
 /// - If the [`Token`](crate::Token) is equal to `"0"` or `"-"`, the token will
-///  be considered an index of an array.
+///   be considered an index of an array.
 /// - All tokens not equal to `"0"` or `"-"` will be considered keys of an
 ///   object.
 ///
@@ -753,9 +753,17 @@ mod tests {
                 assign: json!("foo"),
                 expected: Err(AssignError::FailedToParseIndex {
                     offset: 0,
-                    source: ParseIndexError {
-                        source: usize::from_str("foo").unwrap_err(),
-                    },
+                    source: ParseIndexError::InvalidInteger(usize::from_str("foo").unwrap_err()),
+                }),
+                expected_data: json!([]),
+            },
+            Test {
+                ptr: "/002",
+                data: json!([]),
+                assign: json!("foo"),
+                expected: Err(AssignError::FailedToParseIndex {
+                    offset: 0,
+                    source: ParseIndexError::LeadingZeros,
                 }),
                 expected_data: json!([]),
             },
@@ -907,9 +915,7 @@ mod tests {
                 assign: "foo".into(),
                 expected: Err(AssignError::FailedToParseIndex {
                     offset: 0,
-                    source: ParseIndexError {
-                        source: usize::from_str("foo").unwrap_err(),
-                    },
+                    source: ParseIndexError::InvalidInteger(usize::from_str("foo").unwrap_err()),
                 }),
                 expected_data: Value::Array(vec![]),
             },

diff --git a/src/index.rs b/src/index.rs
@@ -118,10 +118,10 @@ impl Index {
 
     /// Resolves the index for a given array length.
     ///
-    /// No bound checking will take place. If you wish to ensure the index can
-    /// be used to access an existing element in the array, use [`Self::for_len`]
-    /// - or use [`Self::for_len_incl`] if you wish to accept [`Self::Next`] as
-    /// valid as well.
+    /// No bound checking will take place. If you wish to ensure the
+    /// index can be used to access an existing element in the array, use
+    /// [`Self::for_len`] - or use [`Self::for_len_incl`] if you wish to accept
+    /// [`Self::Next`] as valid as well.
     ///
     /// # Examples
     ///
@@ -163,6 +163,8 @@ impl FromStr for Index {
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         if s == "-" {
             Ok(Index::Next)
+        } else if s.starts_with('0') && s != "0" {
+            Err(ParseIndexError::LeadingZeros)
         } else {
             Ok(s.parse::<usize>().map(Index::Num)?)
         }
@@ -260,27 +262,40 @@ impl std::error::Error for OutOfBoundsError {}
 
 /// Indicates that the `Token` could not be parsed as valid RFC 6901 index.
 #[derive(Debug, PartialEq, Eq)]
-pub struct ParseIndexError {
-    /// The source `ParseIntError`
-    pub source: ParseIntError,
+pub enum ParseIndexError {
+    /// The Token does not represent a valid integer.
+    InvalidInteger(ParseIntError),
+    /// The Token contains leading zeros.
+    LeadingZeros,
 }
 
 impl From<ParseIntError> for ParseIndexError {
     fn from(source: ParseIntError) -> Self {
-        Self { source }
+        Self::InvalidInteger(source)
     }
 }
 
 impl fmt::Display for ParseIndexError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "failed to parse token as an integer")
+        match self {
+            ParseIndexError::InvalidInteger(source) => {
+                write!(f, "failed to parse token as an integer: {source}")
+            }
+            ParseIndexError::LeadingZeros => write!(
+                f,
+                "token contained leading zeros, which are disallowed by RFC 6901"
+            ),
+        }
     }
 }
 
 #[cfg(feature = "std")]
 impl std::error::Error for ParseIndexError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        Some(&self.source)
+        match self {
+            ParseIndexError::InvalidInteger(source) => Some(source),
+            ParseIndexError::LeadingZeros => None,
+        }
     }
 }
 
@@ -389,22 +404,27 @@ mod tests {
 
     #[test]
     fn parse_index_error_display() {
-        let err = ParseIndexError {
-            source: "not a number".parse::<usize>().unwrap_err(),
-        };
-        assert_eq!(err.to_string(), "failed to parse token as an integer");
+        let err = ParseIndexError::InvalidInteger("not a number".parse::<usize>().unwrap_err());
+        assert_eq!(
+            err.to_string(),
+            "failed to parse token as an integer: invalid digit found in string"
+        );
+        assert_eq!(
+            ParseIndexError::LeadingZeros.to_string(),
+            "token contained leading zeros, which are disallowed by RFC 6901"
+        );
     }
 
     #[test]
     #[cfg(feature = "std")]
     fn parse_index_error_source() {
         use std::error::Error;
-        let source = "not a number".parse::<usize>().unwrap_err();
-        let err = ParseIndexError { source };
+        let err = ParseIndexError::InvalidInteger("not a number".parse::<usize>().unwrap_err());
         assert_eq!(
             err.source().unwrap().to_string(),
             "not a number".parse::<usize>().unwrap_err().to_string()
         );
+        assert!(ParseIndexError::LeadingZeros.source().is_none());
     }
 
     #[test]

diff --git a/src/pointer.rs b/src/pointer.rs
@@ -1959,8 +1959,8 @@ mod tests {
             let base = PointerBuf::parse(base).expect(&format!("failed to parse ${base}"));
             let mut a = base.clone();
             let mut b = base.clone();
-            a.append(&PointerBuf::parse(a_suffix).unwrap());
-            b.append(&PointerBuf::parse(b_suffix).unwrap());
+            a.append(PointerBuf::parse(a_suffix).unwrap());
+            b.append(PointerBuf::parse(b_suffix).unwrap());
             let intersection = a.intersection(&b);
             assert_eq!(intersection, base);
         }

diff --git a/src/resolve.rs b/src/resolve.rs
@@ -428,9 +428,7 @@ mod tests {
         use std::error::Error;
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert!(err.source().is_some());
 
@@ -454,9 +452,7 @@ mod tests {
     fn resolve_error_display() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert_eq!(format!("{err}"), "failed to parse index at offset 0");
 
@@ -484,9 +480,7 @@ mod tests {
     fn resolve_error_offset() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert_eq!(err.offset(), 0);
 
@@ -510,9 +504,7 @@ mod tests {
     fn resolve_error_is_unreachable() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert!(!err.is_unreachable());
 
@@ -536,9 +528,7 @@ mod tests {
     fn resolve_error_is_not_found() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert!(!err.is_not_found());
 
@@ -562,9 +552,7 @@ mod tests {
     fn resolve_error_is_out_of_bounds() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert!(!err.is_out_of_bounds());
 
@@ -588,9 +576,7 @@ mod tests {
     fn resolve_error_is_failed_to_parse_index() {
         let err = ResolveError::FailedToParseIndex {
             offset: 0,
-            source: ParseIndexError {
-                source: "invalid".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("invalid".parse::<usize>().unwrap_err()),
         };
         assert!(err.is_failed_to_parse_index());
 

diff --git a/src/token.rs b/src/token.rs
@@ -398,9 +398,7 @@ mod tests {
     fn assign_error_display() {
         let err = AssignError::FailedToParseIndex {
             offset: 3,
-            source: ParseIndexError {
-                source: "a".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("a".parse::<usize>().unwrap_err()),
         };
         assert_eq!(
             err.to_string(),
@@ -427,9 +425,7 @@ mod tests {
         use std::error::Error;
         let err = AssignError::FailedToParseIndex {
             offset: 3,
-            source: ParseIndexError {
-                source: "a".parse::<usize>().unwrap_err(),
-            },
+            source: ParseIndexError::InvalidInteger("a".parse::<usize>().unwrap_err()),
         };
         assert!(err.source().is_some());
         assert!(err.source().unwrap().is::<ParseIndexError>());