Skip to content

Commit

Permalink
json: fix ordering of integers greater than i64::MAX
Browse files Browse the repository at this point in the history
Fixes a bug in the ordering of JSON numbers that are greater than the maximum
signed 64 bit integer.  The previous ordering logic would convert the unsigned
integer to signed, which would overflow if the value was larger than
`i64::MAX`, leading to an incorrect ordering result.  This fixes that by adding
in special cases for comparisons between negative signed integers and unsigned
integers. The casts from `u64 -> i64` were replaced with casts from `i64 ->
u64`, relying on the previous checks to ensure that the `i64` value is not
negative.
  • Loading branch information
psFried committed Feb 6, 2024
1 parent fd1447c commit 58ee84b
Showing 1 changed file with 18 additions and 3 deletions.
21 changes: 18 additions & 3 deletions crates/json/src/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl Display for Number {
match self {
Unsigned(n) => write!(f, "{}", n),
Signed(n) => write!(f, "{}", n),
Float(n) => write!(f, "{}", n)
Float(n) => write!(f, "{}", n),
}
}
}
Expand Down Expand Up @@ -71,10 +71,12 @@ impl Ord for Number {
fn cmp(&self, other: &Self) -> Ordering {
match (self, other) {
(Unsigned(lhs), Unsigned(rhs)) => lhs.cmp(rhs),
(Unsigned(lhs), Signed(rhs)) => (*lhs as i64).cmp(rhs),
(Unsigned(_), Signed(rhs)) if *rhs < 0 => Ordering::Greater,
(Unsigned(lhs), Signed(rhs)) => lhs.cmp(&(*rhs as u64)),
(Unsigned(lhs), Float(rhs)) => f64_cmp(&(*lhs as f64), rhs),

(Signed(lhs), Unsigned(rhs)) => lhs.cmp(&(*rhs as i64)),
(Signed(lhs), Unsigned(_)) if *lhs < 0 => Ordering::Less,
(Signed(lhs), Unsigned(rhs)) => (*lhs as u64).cmp(&rhs),
(Signed(lhs), Signed(rhs)) => lhs.cmp(rhs),
(Signed(lhs), Float(rhs)) => f64_cmp(&(*lhs as f64), rhs),

Expand Down Expand Up @@ -268,6 +270,19 @@ mod test {
// and equal to itself, in order to provide a total ordering.
is_lt(Float(NAN), Signed(10));
is_lt(Float(NAN), Float(NEG_INFINITY));

// Test cases where an unsigned integer is greater than i64::MAX
is_lt(Signed(-20), Unsigned(10000000000000000000u64));
is_lt(Signed(0), Unsigned(10000000000000000000u64));
is_lt(Unsigned(12), Signed(i64::MAX));
is_lt(Signed(i64::MIN), Unsigned(u64::MAX));
is_lt(Signed(i64::MAX), Unsigned(u64::MAX));
is_lt(Float(-20.0), Unsigned(10000000000000000000u64));
is_lt(Float(NEG_INFINITY), Unsigned(10000000000000000000u64));
is_lt(
Unsigned(10000000000000000000u64),
Float(11000000000000000000.0),
);
}

#[test]
Expand Down

0 comments on commit 58ee84b

Please sign in to comment.