From 58ee84b245020ffcfb7f2d89f43622bf5b0aeaec Mon Sep 17 00:00:00 2001
From: Phil <phil@estuary.dev>
Date: Tue, 6 Feb 2024 15:44:24 -0500
Subject: [PATCH] json: fix ordering of integers greater than i64::MAX

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.
---
 crates/json/src/number.rs | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/crates/json/src/number.rs b/crates/json/src/number.rs
index 3f399d91c4..d588afc944 100644
--- a/crates/json/src/number.rs
+++ b/crates/json/src/number.rs
@@ -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),
         }
     }
 }
@@ -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),
 
@@ -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]