Skip to content

Commit

Permalink
Stricter validation of RFC 6901 array indices (#80)
Browse files Browse the repository at this point in the history
* introduces a strict check on index values as rust allows for leading '+' in parsing of uints.
* [breaking] introduces a new variant of `ParseIndexError` to account for invalid characters.
  • Loading branch information
asmello authored Oct 21, 2024
1 parent 9a15d91 commit adfc513
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Adds method `into_buf` for `Box<Pointer>` and `impl From<PathBuf> for Box<Pointer>`.
- Adds unsafe associated methods `Pointer::new_unchecked` and `PointerBuf::new_unchecked` for
external zero-cost construction.
- Adds new `ParseIndexError` variant to express the presence non-digit characters in the token.
- Adds `Token::is_next` for checking if a token represents the `-` character.

### Changed

- Changed signature of `PathBuf::parse` to avoid requiring allocation.
- Bumps minimum Rust version to 1.79.

### Fixed

- Make validation of array indices conform to RFC 6901 in the presence of non-digit characters.

## [0.6.2] 2024-09-30

### Added
Expand Down
10 changes: 10 additions & 0 deletions src/assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,16 @@ mod tests {
}),
expected_data: json!([]),
},
Test {
ptr: "/+23",
data: json!([]),
assign: json!("foo"),
expected: Err(AssignError::FailedToParseIndex {
offset: 0,
source: ParseIndexError::InvalidCharacters("+".into()),
}),
expected_data: json!([]),
},
]);
}

Expand Down
32 changes: 29 additions & 3 deletions src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
//! ```

use crate::Token;
use alloc::string::String;
use alloc::{string::String, vec::Vec};
use core::{fmt, num::ParseIntError, str::FromStr};

/// Represents an abstract index into an array.
Expand Down Expand Up @@ -166,7 +166,21 @@ impl FromStr for Index {
} else if s.starts_with('0') && s != "0" {
Err(ParseIndexError::LeadingZeros)
} else {
Ok(s.parse::<usize>().map(Index::Num)?)
let idx = s.parse::<usize>().map(Index::Num)?;
if s.chars().all(|c| c.is_ascii_digit()) {
Ok(idx)
} else {
// this comes up with the `+` sign which is valid for
// representing a `usize` but not allowed in RFC 6901 array
// indices
let mut invalid: Vec<_> = s.chars().filter(|c| !c.is_ascii_digit()).collect();
// remove duplicate characters
invalid.sort_unstable();
invalid.dedup();
Err(ParseIndexError::InvalidCharacters(
invalid.into_iter().collect(),
))
}
}
}
}
Expand Down Expand Up @@ -267,6 +281,8 @@ pub enum ParseIndexError {
InvalidInteger(ParseIntError),
/// The Token contains leading zeros.
LeadingZeros,
/// The Token contains non-digit characters.
InvalidCharacters(String),
}

impl From<ParseIntError> for ParseIndexError {
Expand All @@ -285,6 +301,11 @@ impl fmt::Display for ParseIndexError {
f,
"token contained leading zeros, which are disallowed by RFC 6901"
),
ParseIndexError::InvalidCharacters(chars) => write!(
f,
"token contains non-digit character(s) '{chars}', \
which are disallowed by RFC 6901",
),
}
}
}
Expand All @@ -294,7 +315,7 @@ impl std::error::Error for ParseIndexError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
ParseIndexError::InvalidInteger(source) => Some(source),
ParseIndexError::LeadingZeros => None,
ParseIndexError::LeadingZeros | ParseIndexError::InvalidCharacters(_) => None,
}
}
}
Expand Down Expand Up @@ -413,6 +434,11 @@ mod tests {
ParseIndexError::LeadingZeros.to_string(),
"token contained leading zeros, which are disallowed by RFC 6901"
);
assert_eq!(
ParseIndexError::InvalidCharacters("+@".into()).to_string(),
"token contains non-digit character(s) '+@', \
which are disallowed by RFC 6901"
);
}

#[test]
Expand Down

0 comments on commit adfc513

Please sign in to comment.