Skip to content

Commit

Permalink
feat: implement integer type to comply with spec
Browse files Browse the repository at this point in the history
The JSONPath spec states that integers, e.g., those used as indices or in
slice selectors, must be within a specific range [-2^53 +1, 2^53 - 1].

This adds a new Integer type to be used to represent such integers, and
check that they stay within that range. It also caught a couple of places
where overflows would have led to panics.
  • Loading branch information
hiltontj committed Aug 4, 2024
1 parent 12d782d commit eacf1e4
Show file tree
Hide file tree
Showing 9 changed files with 242 additions and 57 deletions.
24 changes: 18 additions & 6 deletions serde_json_path/src/parser/primitive/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use nom::{
combinator::{map_res, opt, recognize},
sequence::tuple,
};
use serde_json_path_core::spec::integer::Integer;

use crate::parser::PResult;

Expand Down Expand Up @@ -37,19 +38,30 @@ pub(crate) fn parse_int_string(input: &str) -> PResult<&str> {
}

#[cfg_attr(feature = "trace", tracing::instrument(level = "trace", parent = None, ret, err))]
pub(crate) fn parse_int(input: &str) -> PResult<isize> {
map_res(parse_int_string, |i_str| i_str.parse::<isize>())(input)
pub(crate) fn parse_int(input: &str) -> PResult<Integer> {
map_res(parse_int_string, |i_str| i_str.parse())(input)
}

#[cfg(test)]
mod tests {
use serde_json_path_core::spec::integer::Integer;

use crate::parser::primitive::int::parse_int;

#[test]
fn parse_integers() {
assert_eq!(parse_int("0"), Ok(("", 0)));
assert_eq!(parse_int("10"), Ok(("", 10)));
assert_eq!(parse_int("-10"), Ok(("", -10)));
assert_eq!(parse_int("010"), Ok(("10", 0)));
assert_eq!(parse_int("0"), Ok(("", Integer::from_i64_opt(0).unwrap())));
assert_eq!(
parse_int("10"),
Ok(("", Integer::from_i64_opt(10).unwrap()))
);
assert_eq!(
parse_int("-10"),
Ok(("", Integer::from_i64_opt(-10).unwrap()))
);
assert_eq!(
parse_int("010"),
Ok(("10", Integer::from_i64_opt(0).unwrap()))
);
}
}
10 changes: 8 additions & 2 deletions serde_json_path/src/parser/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ mod tests {
use test_log::test;

use nom::combinator::all_consuming;
use serde_json_path_core::spec::selector::{index::Index, name::Name, slice::Slice, Selector};
use serde_json_path_core::spec::{
integer::Integer,
selector::{index::Index, name::Name, slice::Slice, Selector},
};

use super::{
parse_child_long_hand, parse_child_segment, parse_descendant_segment,
Expand Down Expand Up @@ -194,7 +197,10 @@ mod tests {
let (_, sk) = parse_child_long_hand(r#"['name',10,0:3]"#).unwrap();
let s = sk.as_long_hand().unwrap();
assert_eq!(s[0], Selector::Name(Name::from("name")));
assert_eq!(s[1], Selector::Index(Index(10)));
assert_eq!(
s[1],
Selector::Index(Index(Integer::from_i64_opt(10).unwrap()))
);
assert_eq!(
s[2],
Selector::ArraySlice(Slice::new().with_start(0).with_end(3))
Expand Down
12 changes: 9 additions & 3 deletions serde_json_path/src/parser/selector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ pub(crate) fn parse_selector(input: &str) -> PResult<Selector> {

#[cfg(test)]
mod tests {
use serde_json_path_core::spec::selector::{name::Name, slice::Slice};
use serde_json_path_core::spec::{
integer::Integer,
selector::{name::Name, slice::Slice},
};

use super::{parse_selector, parse_wildcard_selector, Index, Selector};

Expand All @@ -84,11 +87,14 @@ mod tests {
fn all_selectors() {
{
let (_, s) = parse_selector("0").unwrap();
assert_eq!(s, Selector::Index(Index(0)));
assert_eq!(s, Selector::Index(Index(Integer::from_i64_opt(0).unwrap())));
}
{
let (_, s) = parse_selector("10").unwrap();
assert_eq!(s, Selector::Index(Index(10)));
assert_eq!(
s,
Selector::Index(Index(Integer::from_i64_opt(10).unwrap()))
);
}
{
let (_, s) = parse_selector("'name'").unwrap();
Expand Down
6 changes: 3 additions & 3 deletions serde_json_path/src/parser/selector/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ use nom::{
combinator::{map, opt},
sequence::{preceded, separated_pair, terminated},
};
use serde_json_path_core::spec::selector::slice::Slice;
use serde_json_path_core::spec::{integer::Integer, selector::slice::Slice};

use crate::parser::{primitive::int::parse_int, PResult};

#[cfg_attr(feature = "trace", tracing::instrument(level = "trace", parent = None, ret, err))]
fn parse_int_space_after(input: &str) -> PResult<isize> {
fn parse_int_space_after(input: &str) -> PResult<Integer> {
terminated(parse_int, multispace0)(input)
}

#[cfg_attr(feature = "trace", tracing::instrument(level = "trace", parent = None, ret, err))]
fn parse_int_space_before(input: &str) -> PResult<isize> {
fn parse_int_space_before(input: &str) -> PResult<Integer> {
preceded(multispace0, parse_int)(input)
}

Expand Down
120 changes: 120 additions & 0 deletions serde_json_path_core/src/spec/integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
//! Representation of itegers in the JSONPath specification
//!
//! The JSONPath specification defines some rules for integers used in query strings (see [here][spec]).
//!
//! [spec]: https://www.rfc-editor.org/rfc/rfc9535.html#name-overview
use std::{
num::{ParseIntError, TryFromIntError},
str::FromStr,
};

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Integer(i64);

Check warning on line 13 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a struct

Check failure on line 13 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a struct

Check failure on line 13 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

missing documentation for a struct

Check failure on line 13 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a struct

const MAX: i64 = 9_007_199_254_740_992 - 1;
const MIN: i64 = -9_007_199_254_740_992 + 1;

impl Integer {
fn try_new(value: i64) -> Result<Self, IntegerError> {
if value < MIN || value > MAX {

Check failure on line 20 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

manual `!RangeInclusive::contains` implementation
return Err(IntegerError::OutOfBounds);

Check failure on line 21 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

unneeded `return` statement
} else {
return Ok(Self(value));

Check failure on line 23 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

unneeded `return` statement
}
}

fn check(&self) -> bool {
self.0 >= MIN && self.0 <= MAX
}

pub fn from_i64_opt(value: i64) -> Option<Self> {

Check warning on line 31 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for an associated function

Check failure on line 31 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for an associated function

Check failure on line 31 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

missing documentation for an associated function

Check failure on line 31 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for an associated function
Self::try_new(value).ok()
}

pub fn checked_abs(mut self) -> Option<Self> {

Check warning on line 35 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 35 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 35 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

missing documentation for a method

Check failure on line 35 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a method
self.0 = self.0.checked_abs()?;
self.check().then_some(self)
}

pub fn checked_add(mut self, rhs: Self) -> Option<Self> {

Check warning on line 40 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 40 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 40 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

missing documentation for a method

Check failure on line 40 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a method
self.0 = self.0.checked_add(rhs.0)?;
self.check().then_some(self)
}

pub fn checked_sub(mut self, rhs: Self) -> Option<Self> {

Check warning on line 45 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 45 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 45 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

missing documentation for a method

Check failure on line 45 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a method
self.0 = self.0.checked_sub(rhs.0)?;
self.check().then_some(self)
}

pub fn checked_mul(mut self, rhs: Self) -> Option<Self> {

Check warning on line 50 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 50 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a method

Check failure on line 50 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a method
self.0 = self.0.checked_mul(rhs.0)?;
self.check().then_some(self)
}
}

impl Default for Integer {

Check failure on line 56 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Clippy

this `impl` can be derived
fn default() -> Self {
Self(0)
}
}

impl TryFrom<i64> for Integer {
type Error = IntegerError;

fn try_from(value: i64) -> Result<Self, Self::Error> {
Self::try_new(value)
}
}

impl TryFrom<usize> for Integer {
type Error = IntegerError;

fn try_from(value: usize) -> Result<Self, Self::Error> {
i64::try_from(value)
.map_err(|_| IntegerError::OutOfBounds)
.and_then(Self::try_new)
}
}

impl FromStr for Integer {
type Err = IntegerError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
s.parse::<i64>().map_err(Into::into).and_then(Self::try_new)
}
}

impl std::fmt::Display for Integer {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
}
}

impl TryFrom<Integer> for usize {
type Error = TryFromIntError;

fn try_from(value: Integer) -> Result<Self, Self::Error> {
Self::try_from(value.0)
}
}

impl PartialEq<i64> for Integer {
fn eq(&self, other: &i64) -> bool {
self.0.eq(other)
}
}

impl PartialOrd<i64> for Integer {
fn partial_cmp(&self, other: &i64) -> Option<std::cmp::Ordering> {
self.0.partial_cmp(other)
}
}

#[derive(Debug, thiserror::Error)]
pub enum IntegerError {

Check warning on line 115 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for an enum

Check failure on line 115 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for an enum

Check failure on line 115 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for an enum
#[error("the provided integer was outside the valid range, see https://www.rfc-editor.org/rfc/rfc9535.html#section-2.1-4.1")]
OutOfBounds,

Check warning on line 117 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a variant

Check failure on line 117 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a variant

Check failure on line 117 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a variant
#[error(transparent)]
Parse(#[from] ParseIntError),

Check warning on line 119 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a variant

Check failure on line 119 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Docs

missing documentation for a variant

Check failure on line 119 in serde_json_path_core/src/spec/integer.rs

View workflow job for this annotation

GitHub Actions / Test

missing documentation for a variant
}
1 change: 1 addition & 0 deletions serde_json_path_core/src/spec/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Types representing the IETF JSONPath Standard
pub mod functions;
pub mod integer;
pub mod query;
pub mod segment;
pub mod selector;
18 changes: 11 additions & 7 deletions serde_json_path_core/src/spec/selector/index.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
//! Index selectors in JSONPath
use serde_json::Value;

use crate::{node::LocatedNode, path::NormalizedPath, spec::query::Queryable};
use crate::{
node::LocatedNode,
path::NormalizedPath,
spec::{integer::Integer, query::Queryable},
};

/// For selecting array elements by their index
///
/// Can use negative indices to index from the end of an array
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub struct Index(pub isize);
pub struct Index(pub Integer);

impl std::fmt::Display for Index {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down Expand Up @@ -66,8 +70,8 @@ impl Queryable for Index {
}
}

impl From<isize> for Index {
fn from(i: isize) -> Self {
Self(i)
}
}
// impl From<isize> for Index {
// fn from(i: isize) -> Self {
// Self(i)
// }
// }
Loading

0 comments on commit eacf1e4

Please sign in to comment.