From 7b3b608cafbed8049ac7a34104c66622c1445ffc Mon Sep 17 00:00:00 2001
From: Timothy Flynn <trflynn89@pm.me>
Date: Thu, 26 Sep 2024 16:13:24 -0400
Subject: [PATCH] AK: Do not coerce i64 and u64 values to i32 and u32

First, this isn't actually helpful, as we no longer store 32-bit values
in JsonValue. They are stored as 64-bit values anyways.

But more imporatantly, there was a bug here when trying to coerce an i64
to an i32. All negative values were cast to an i32, without checking if
the value is below NumericLimits<i32>::min.
---
 AK/JsonParser.cpp     | 17 ++++-------------
 Tests/AK/TestJSON.cpp | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/AK/JsonParser.cpp b/AK/JsonParser.cpp
index 522c178fdc6..6a2482cfacd 100644
--- a/AK/JsonParser.cpp
+++ b/AK/JsonParser.cpp
@@ -271,19 +271,10 @@ ErrorOr<JsonValue> JsonParser::parse_number()
 
     StringView number_string(number_buffer.data(), number_buffer.size());
 
-    auto to_unsigned_result = number_string.to_number<u64>();
-    if (to_unsigned_result.has_value()) {
-        if (*to_unsigned_result <= NumericLimits<u32>::max())
-            return JsonValue((u32)*to_unsigned_result);
-
-        return JsonValue(*to_unsigned_result);
-    } else if (auto signed_number = number_string.to_number<i64>(); signed_number.has_value()) {
-
-        if (*signed_number <= NumericLimits<i32>::max())
-            return JsonValue((i32)*signed_number);
-
-        return JsonValue(*signed_number);
-    }
+    if (auto number = number_string.to_number<u64>(); number.has_value())
+        return JsonValue(*number);
+    if (auto number = number_string.to_number<i64>(); number.has_value())
+        return JsonValue(*number);
 
     // It's possible the unsigned value is bigger than u64 max
     return fallback_to_double_parse();
diff --git a/Tests/AK/TestJSON.cpp b/Tests/AK/TestJSON.cpp
index 8c49755e9b8..296c2782ddb 100644
--- a/Tests/AK/TestJSON.cpp
+++ b/Tests/AK/TestJSON.cpp
@@ -128,6 +128,34 @@ TEST_CASE(json_64_bit_value)
     EXPECT(big_json_value.equals(big_json_value_copy));
 }
 
+TEST_CASE(json_64_bit_value_coerced_to_32_bit)
+{
+    {
+        auto min = NumericLimits<i64>::min();
+        auto max = NumericLimits<i64>::max();
+
+        auto json = TRY_OR_FAIL(JsonValue::from_string(MUST(String::number(min))));
+        EXPECT_EQ(json.get_integer<i64>(), min);
+        EXPECT(!json.is_integer<i32>());
+
+        json = TRY_OR_FAIL(JsonValue::from_string(MUST(String::number(max))));
+        EXPECT_EQ(json.get_integer<i64>(), max);
+        EXPECT(!json.is_integer<i32>());
+    }
+    {
+        auto min = NumericLimits<u64>::min();
+        auto max = NumericLimits<u64>::max();
+
+        auto json = TRY_OR_FAIL(JsonValue::from_string(MUST(String::number(min))));
+        EXPECT_EQ(json.get_integer<u64>(), min);
+        EXPECT_EQ(json.get_integer<u32>(), min);
+
+        json = TRY_OR_FAIL(JsonValue::from_string(MUST(String::number(max))));
+        EXPECT_EQ(json.get_integer<u64>(), max);
+        EXPECT(!json.is_integer<u32>());
+    }
+}
+
 TEST_CASE(json_duplicate_keys)
 {
     JsonObject json;