Skip to content

Commit

Permalink
feat: implement integer type to comply with spec (#98)
Browse files Browse the repository at this point in the history
* feat: implement integer type to comply with spec

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 authored Aug 4, 2024
1 parent 12d782d commit 6c551f0
Show file tree
Hide file tree
Showing 11 changed files with 269 additions and 57 deletions.
3 changes: 3 additions & 0 deletions serde_json_path/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **fixed**: edge case where `.` in regexes for `match` and `search` functions was matching `\r\n` properly ([#92])
- **breaking**: added `regex` feature flag that gates regex functions `match` and `search`
- Feature is enabled by default, but if you have `default-features = false` you'll need to explicitly add it to retain access to these functions
- **breaking**(`serde_json_path_core`): ensure integers used as indices are within the [valid range for I-JSON][i-json-range] ([#98])

[#92]: https://github.com/hiltontj/serde_json_path/pull/92
[#98]: https://github.com/hiltontj/serde_json_path/pull/98
[i-json-range]: https://www.rfc-editor.org/rfc/rfc9535.html#section-2.1-4.1

# 0.6.7 (3 March 2024)

Expand Down
15 changes: 9 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,21 @@ 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_unchecked(0))));
assert_eq!(parse_int("10"), Ok(("", Integer::from_i64_unchecked(10))));
assert_eq!(parse_int("-10"), Ok(("", Integer::from_i64_unchecked(-10))));
assert_eq!(parse_int("010"), Ok(("10", Integer::from_i64_unchecked(0))));
}
}
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_unchecked(10)))
);
assert_eq!(
s[2],
Selector::ArraySlice(Slice::new().with_start(0).with_end(3))
Expand Down
9 changes: 6 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,11 @@ 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_unchecked(0))));
}
{
let (_, s) = parse_selector("10").unwrap();
assert_eq!(s, Selector::Index(Index(10)));
assert_eq!(s, Selector::Index(Index(Integer::from_i64_unchecked(10))));
}
{
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
5 changes: 5 additions & 0 deletions serde_json_path_core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

# Unreleased

- **breaking**: ensure integers used as indices are within the [valid range for I-JSON][i-json-range] ([#98])

[#98]: https://github.com/hiltontj/serde_json_path/pull/98
[i-json-range]: https://www.rfc-editor.org/rfc/rfc9535.html#section-2.1-4.1

# 0.1.6 (3 March 2024)

- **testing**: support tests for non-determinism in compliance test suite ([#85])
Expand Down
145 changes: 145 additions & 0 deletions serde_json_path_core/src/spec/integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
//! 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,
};

/// An integer for internet JSON ([RFC7493][ijson])
///
/// The value must be within the range [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
///
/// [ijson]: https://www.rfc-editor.org/rfc/rfc7493#section-2.2
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
pub struct Integer(i64);

/// The maximum allowed value, 2^53 - 1
const MAX: i64 = 9_007_199_254_740_992 - 1;
/// The minimum allowed value (-2^53) + 1
const MIN: i64 = -9_007_199_254_740_992 + 1;

impl Integer {
fn try_new(value: i64) -> Result<Self, IntegerError> {
if !(MIN..=MAX).contains(&value) {
Err(IntegerError::OutOfBounds)
} else {
Ok(Self(value))
}
}

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

/// Produce an [`Integer`] with the value 0
pub fn zero() -> Self {
Self(0)
}

/// Get an [`Integer`] from an `i64`
///
/// This is intended for initializing an integer with small, non-zero numbers.
///
/// # Panics
///
/// This will panic if the inputted value is out of the valid range
/// [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn from_i64_unchecked(value: i64) -> Self {
Self::try_new(value).expect("value is out of the valid range")
}

/// Take the absolute value, producing `None` if the resulting value is outside
/// the valid range [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn checked_abs(mut self) -> Option<Self> {
self.0 = self.0.checked_abs()?;
self.check().then_some(self)
}

/// Add the two values, producing `None` if the resulting value is outside the
/// valid range [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn checked_add(mut self, rhs: Self) -> Option<Self> {
self.0 = self.0.checked_add(rhs.0)?;
self.check().then_some(self)
}

/// Subtract the `rhs` from `self`, producing `None` if the resulting value is
/// outside the valid range [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn checked_sub(mut self, rhs: Self) -> Option<Self> {
self.0 = self.0.checked_sub(rhs.0)?;
self.check().then_some(self)
}

/// Multiply the two values, producing `None` if the resulting value is outside
/// the valid range [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn checked_mul(mut self, rhs: Self) -> Option<Self> {
self.0 = self.0.checked_mul(rhs.0)?;
self.check().then_some(self)
}
}

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

/// An error for the [`Integer`] type
#[derive(Debug, thiserror::Error)]
pub enum IntegerError {
/// The provided value was outside the valid range [-(2**53)+1, (2**53)-1])
#[error("the provided integer was outside the valid range, see https://www.rfc-editor.org/rfc/rfc9535.html#section-2.1-4.1")]
OutOfBounds,
/// Integer parsing error
#[error(transparent)]
Parse(#[from] ParseIntError),
}
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;
14 changes: 6 additions & 8 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 @@ -65,9 +69,3 @@ impl Queryable for Index {
}
}
}

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

0 comments on commit 6c551f0

Please sign in to comment.