From fea44b05d923dfea0e8b89f5cad66f8e00bce51a Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 6 Aug 2026 19:07:10 +0800 Subject: [PATCH 1/2] fix: reject out-of-range integers when parsing JSON literals nlohmann reports unsigned integers as is_number_integer(), and get() converts values above INT64_MAX silently instead of throwing. So an integer beyond the signed range was accepted as a literal: the kInt branch ran its int32 range check on the already-wrapped value, and the kLong branch had no range check at all. 18446744073709551615 parsed as Literal::Int(-1) rather than returning a parse error. Add a GetInt64Checked helper that rejects unsigned values above INT64_MAX before the conversion, and use it in the kInt and kLong branches of the type-aware parser plus the untyped overload. This matches Java, where SingleValueParser and ExpressionParser guard the same paths with canConvertToInt()/canConvertToLong(). --- src/iceberg/expression/json_serde.cc | 26 ++++++++++++++++++++---- src/iceberg/test/expression_json_test.cc | 16 ++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/iceberg/expression/json_serde.cc b/src/iceberg/expression/json_serde.cc index df8aba88f..51f7a40aa 100644 --- a/src/iceberg/expression/json_serde.cc +++ b/src/iceberg/expression/json_serde.cc @@ -305,6 +305,21 @@ Result ToJson(const Literal& literal) { } } +/// \brief Read an integral JSON node as int64_t, rejecting out-of-range values. +/// +/// nlohmann reports unsigned integers as is_number_integer(), and get() +/// converts values above INT64_MAX silently rather than throwing, so the range +/// has to be checked before the conversion. Mirrors Java's canConvertToLong(). +Result GetInt64Checked(const nlohmann::json& json) { + if (json.is_number_unsigned() && + json.get() > static_cast(std::numeric_limits::max())) + [[unlikely]] { + return JsonParseError("Cannot parse {} as a long value: out of range", + SafeDumpJson(json)); + } + return json.get(); +} + Result LiteralFromJson(const nlohmann::json& json, const Type* type) { // If {"type": "literal", "value": } wrapper is present, unwrap it first. if (json.is_object() && json.contains(kType) && @@ -326,7 +341,7 @@ Result LiteralFromJson(const nlohmann::json& json, const Type* type) { if (!json.is_number_integer()) [[unlikely]] { return JsonParseError("Cannot parse {} as an int value", SafeDumpJson(json)); } - auto val = json.get(); + ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json)); if (val < std::numeric_limits::min() || val > std::numeric_limits::max()) [[unlikely]] { return JsonParseError("Cannot parse {} as an int value: out of range", @@ -335,11 +350,13 @@ Result LiteralFromJson(const nlohmann::json& json, const Type* type) { return Literal::Int(static_cast(val)); } - case TypeId::kLong: + case TypeId::kLong: { if (!json.is_number_integer()) [[unlikely]] { return JsonParseError("Cannot parse {} as a long value", SafeDumpJson(json)); } - return Literal::Long(json.get()); + ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json)); + return Literal::Long(val); + } case TypeId::kFloat: if (!json.is_number_float()) [[unlikely]] { @@ -484,7 +501,8 @@ Result LiteralFromJson(const nlohmann::json& json) { return Literal::Boolean(json.get()); } if (json.is_number_integer()) { - return Literal::Long(json.get()); + ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json)); + return Literal::Long(val); } if (json.is_number_float()) { return Literal::Double(json.get()); diff --git a/src/iceberg/test/expression_json_test.cc b/src/iceberg/test/expression_json_test.cc index 7b978ef70..21b680135 100644 --- a/src/iceberg/test/expression_json_test.cc +++ b/src/iceberg/test/expression_json_test.cc @@ -490,11 +490,25 @@ INSTANTIATE_TEST_SUITE_P( InvalidLiteralFromJsonTypedParam{"DecimalScaleMismatch", nlohmann::json("123.45"), decimal(9, 4)}, InvalidLiteralFromJsonTypedParam{"DecimalNotString", nlohmann::json(123.45), - decimal(9, 2)}), + decimal(9, 2)}, + // nlohmann reports unsigned integers as is_number_integer(), and + // get() wraps silently for values above INT64_MAX, so an + // out-of-range integer must be rejected explicitly. + InvalidLiteralFromJsonTypedParam{ + "IntUnsignedOverflow", nlohmann::json(18446744073709551615ULL), int32()}, + InvalidLiteralFromJsonTypedParam{ + "LongUnsignedOverflow", nlohmann::json(9223372036854775808ULL), int64()}), [](const ::testing::TestParamInfo& info) { return info.param.name; }); +// The untyped overload infers long from any integral JSON node, so it needs the +// same out-of-range guard as the type-aware one. +TEST(LiteralFromJsonTest, RejectsUnsignedOverflowUntyped) { + EXPECT_FALSE(LiteralFromJson(nlohmann::json(9223372036854775808ULL)).has_value()); + EXPECT_FALSE(LiteralFromJson(nlohmann::json(18446744073709551615ULL)).has_value()); +} + struct SchemaAwarePredicateParam { std::string name; std::string field_name; From bf3f8944084737b49498f9c071cceaa5d01cf9bc Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Sat, 8 Aug 2026 00:01:30 +0800 Subject: [PATCH 2/2] test: pin both halves of the integer range guard The out-of-range check is a conjunction: is_number_unsigned() plus a comparison against INT64_MAX. Neither accept-side had a test, so dropping either half went unnoticed. Add LongMax (an unsigned node exactly at INT64_MAX, where the ULL suffix is load-bearing), LongMin and IntNegative for the signed path, and an untyped negative case. Also cover the int32 narrowing that follows the shared guard on the kInt path, move the helper into an anonymous namespace so it stops taking an external symbol, and use one wording for both out-of-range messages. --- src/iceberg/expression/json_serde.cc | 13 ++++++-- src/iceberg/test/expression_json_test.cc | 41 +++++++++++++++++++++--- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/iceberg/expression/json_serde.cc b/src/iceberg/expression/json_serde.cc index 51f7a40aa..1afed021f 100644 --- a/src/iceberg/expression/json_serde.cc +++ b/src/iceberg/expression/json_serde.cc @@ -17,6 +17,7 @@ * under the License. */ +#include #include #include #include @@ -305,21 +306,29 @@ Result ToJson(const Literal& literal) { } } +namespace { + /// \brief Read an integral JSON node as int64_t, rejecting out-of-range values. /// /// nlohmann reports unsigned integers as is_number_integer(), and get() /// converts values above INT64_MAX silently rather than throwing, so the range /// has to be checked before the conversion. Mirrors Java's canConvertToLong(). +/// +/// The is_number_unsigned() half of the guard is load-bearing: get() on a +/// negative node yields its two's-complement value, which would compare above +/// INT64_MAX and reject every negative literal. Result GetInt64Checked(const nlohmann::json& json) { if (json.is_number_unsigned() && json.get() > static_cast(std::numeric_limits::max())) [[unlikely]] { - return JsonParseError("Cannot parse {} as a long value: out of range", + return JsonParseError("Cannot parse {} as an integer value: out of range", SafeDumpJson(json)); } return json.get(); } +} // namespace + Result LiteralFromJson(const nlohmann::json& json, const Type* type) { // If {"type": "literal", "value": } wrapper is present, unwrap it first. if (json.is_object() && json.contains(kType) && @@ -344,7 +353,7 @@ Result LiteralFromJson(const nlohmann::json& json, const Type* type) { ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json)); if (val < std::numeric_limits::min() || val > std::numeric_limits::max()) [[unlikely]] { - return JsonParseError("Cannot parse {} as an int value: out of range", + return JsonParseError("Cannot parse {} as an integer value: out of range", SafeDumpJson(json)); } return Literal::Int(static_cast(val)); diff --git a/src/iceberg/test/expression_json_test.cc b/src/iceberg/test/expression_json_test.cc index 21b680135..1c993cc10 100644 --- a/src/iceberg/test/expression_json_test.cc +++ b/src/iceberg/test/expression_json_test.cc @@ -17,6 +17,8 @@ * under the License. */ +#include +#include #include #include #include @@ -438,6 +440,18 @@ INSTANTIATE_TEST_SUITE_P( "123"}, LiteralFromJsonTypedParam{"Long", nlohmann::json(9876543210LL), int64(), TypeId::kLong, "9876543210"}, + // Guard both halves of the out-of-range check in GetInt64Checked: an + // unsigned node at INT64_MAX must be accepted (not off-by-one rejected), + // and negative nodes must not be mistaken for huge unsigned ones. The ULL + // suffix below is load-bearing: a signed INT64_MAX node skips the unsigned + // branch of the guard entirely. + LiteralFromJsonTypedParam{"LongMax", nlohmann::json(9223372036854775807ULL), + int64(), TypeId::kLong, "9223372036854775807"}, + LiteralFromJsonTypedParam{"LongMin", + nlohmann::json(std::numeric_limits::min()), + int64(), TypeId::kLong, "-9223372036854775808"}, + LiteralFromJsonTypedParam{"IntNegative", nlohmann::json(-123), int32(), + TypeId::kInt, "-123"}, LiteralFromJsonTypedParam{"Float", nlohmann::json(1.5), float32(), TypeId::kFloat, std::nullopt}, LiteralFromJsonTypedParam{"Double", nlohmann::json(3.14), float64(), @@ -496,17 +510,34 @@ INSTANTIATE_TEST_SUITE_P( // out-of-range integer must be rejected explicitly. InvalidLiteralFromJsonTypedParam{ "IntUnsignedOverflow", nlohmann::json(18446744073709551615ULL), int32()}, - InvalidLiteralFromJsonTypedParam{ - "LongUnsignedOverflow", nlohmann::json(9223372036854775808ULL), int64()}), + InvalidLiteralFromJsonTypedParam{"LongUnsignedOverflow", + nlohmann::json(9223372036854775808ULL), int64()}, + // Within int64 but outside int32: caught by the narrower range check that + // follows the shared int64 guard. + InvalidLiteralFromJsonTypedParam{"IntAboveInt32Max", nlohmann::json(3000000000LL), + int32()}, + InvalidLiteralFromJsonTypedParam{"IntBelowInt32Min", + nlohmann::json(-3000000000LL), int32()}), [](const ::testing::TestParamInfo& info) { return info.param.name; }); // The untyped overload infers long from any integral JSON node, so it needs the -// same out-of-range guard as the type-aware one. +// same out-of-range guard as the type-aware one. Matching on the message keeps the +// assertion tied to the range check if someone later rejects these nodes elsewhere. TEST(LiteralFromJsonTest, RejectsUnsignedOverflowUntyped) { - EXPECT_FALSE(LiteralFromJson(nlohmann::json(9223372036854775808ULL)).has_value()); - EXPECT_FALSE(LiteralFromJson(nlohmann::json(18446744073709551615ULL)).has_value()); + EXPECT_THAT(LiteralFromJson(nlohmann::json(9223372036854775808ULL)), + HasErrorMessage("out of range")); + EXPECT_THAT(LiteralFromJson(nlohmann::json(18446744073709551615ULL)), + HasErrorMessage("out of range")); +} + +// Negative literals must survive the untyped path too: get() on them +// would compare above INT64_MAX if the is_number_unsigned() guard were dropped. +TEST(LiteralFromJsonTest, AcceptsNegativeIntegerUntyped) { + ICEBERG_UNWRAP_OR_FAIL(auto lit, LiteralFromJson(nlohmann::json(-123))); + EXPECT_EQ(lit.type()->type_id(), TypeId::kLong); + EXPECT_EQ(lit.ToString(), "-123"); } struct SchemaAwarePredicateParam {