diff --git a/src/iceberg/expression/json_serde.cc b/src/iceberg/expression/json_serde.cc index df8aba88f..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,6 +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 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) && @@ -326,20 +350,22 @@ 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", + return JsonParseError("Cannot parse {} as an integer value: out of range", SafeDumpJson(json)); } 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 +510,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..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(), @@ -490,11 +504,42 @@ 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()}, + // 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. Matching on the message keeps the +// assertion tied to the range check if someone later rejects these nodes elsewhere. +TEST(LiteralFromJsonTest, RejectsUnsignedOverflowUntyped) { + 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 { std::string name; std::string field_name;