You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ga...@apache.org on 2026/08/15 16:41:53 UTC
(iceberg-cpp) branch main updated: fix: reject out-of-range integers when parsing JSON literals (#878)
This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 3165b580 fix: reject out-of-range integers when parsing JSON literals (#878)
3165b580 is described below
commit 3165b58090b5ca647b0a0aa642aace9267dbc7f1
Author: YangJie <ya...@baidu.com>
AuthorDate: Sun Aug 16 00:41:48 2026 +0800
fix: reject out-of-range integers when parsing JSON literals (#878)
## What
`LiteralFromJson` accepted JSON integers beyond the signed 64-bit range and produced a wrong literal instead of a parse error. nlohmann reports unsigned integers as `is_number_integer()`, and `get<int64_t>()` converts values above INT64_MAX silently rather than throwing. So 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)`, and `9223372036854775808` as `Literal [...]
Table metadata is the path where this changes returned data. `initial-default` / `write-default` go through this parser, `ValidateDefault` does not check integer range, and the value later gets materialized into a column. A metadata file that Java rejects reads back as `-1` here. The expression path is latent for now, since nothing evaluates `ReaderOptions::filter` yet.
Fixes #877.
## How
`GetInt64Checked` rejects unsigned nodes above INT64_MAX before the conversion, and the `kInt` branch, the `kLong` branch and the untyped overload all go through it. Java guards the same paths with `canConvertToInt()` / `canConvertToLong()` in `SingleValueParser` and `ExpressionParser`.
The `is_number_unsigned()` half of the guard is load-bearing: `get<uint64_t>()` on a negative node yields its two's-complement value, which compares above INT64_MAX and would reject every negative literal.
Two smaller things on lines this fix already touches: the int32 narrowing on the `kInt` path gained the coverage it never had, and the two out-of-range messages now use the same wording instead of saying "int" in one and "long" in the other.
## Testing
`expression_test` (526 tests) and the full `ctest` suite (18/18) pass. Each new test was checked against a mutation of the code it guards:
- `>` to `>=` in the guard: only `LongMax` fails. The `ULL` suffix there matters, since a signed INT64_MAX node skips the unsigned branch entirely.
- `is_number_unsigned() &&` dropped: `LongMin`, `IntNegative` and `AcceptsNegativeIntegerUntyped` fail.
- int32 range check deleted: `IntAboveInt32Max` and `IntBelowInt32Min` fail.
The three overflow-rejection cases were also confirmed to fail without the fix and pass with it.
## Out of scope
`GetTypedJsonValue` in `src/iceberg/util/json_util_internal.h` truncates out-of-range integers the same way, so `FieldFromJson({"id": 2147483648, ...})` yields `field_id = -2147483648` silently. That helper has roughly 80 call sites, so it belongs in its own PR.
---
src/iceberg/expression/json_serde.cc | 37 +++++++++++++++++++++----
src/iceberg/test/expression_json_test.cc | 47 +++++++++++++++++++++++++++++++-
2 files changed, 78 insertions(+), 6 deletions(-)
diff --git a/src/iceberg/expression/json_serde.cc b/src/iceberg/expression/json_serde.cc
index df8aba88..1afed021 100644
--- a/src/iceberg/expression/json_serde.cc
+++ b/src/iceberg/expression/json_serde.cc
@@ -17,6 +17,7 @@
* under the License.
*/
+#include <cstdint>
#include <limits>
#include <string>
#include <vector>
@@ -305,6 +306,29 @@ Result<nlohmann::json> 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<int64_t>()
+/// 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<uint64_t>() on a
+/// negative node yields its two's-complement value, which would compare above
+/// INT64_MAX and reject every negative literal.
+Result<int64_t> GetInt64Checked(const nlohmann::json& json) {
+ if (json.is_number_unsigned() &&
+ json.get<uint64_t>() > static_cast<uint64_t>(std::numeric_limits<int64_t>::max()))
+ [[unlikely]] {
+ return JsonParseError("Cannot parse {} as an integer value: out of range",
+ SafeDumpJson(json));
+ }
+ return json.get<int64_t>();
+}
+
+} // namespace
+
Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
// If {"type": "literal", "value": <actual>} wrapper is present, unwrap it first.
if (json.is_object() && json.contains(kType) &&
@@ -326,20 +350,22 @@ Result<Literal> 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<int64_t>();
+ ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json));
if (val < std::numeric_limits<int32_t>::min() ||
val > std::numeric_limits<int32_t>::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<int32_t>(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<int64_t>());
+ 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<Literal> LiteralFromJson(const nlohmann::json& json) {
return Literal::Boolean(json.get<bool>());
}
if (json.is_number_integer()) {
- return Literal::Long(json.get<int64_t>());
+ ICEBERG_ASSIGN_OR_RAISE(auto val, GetInt64Checked(json));
+ return Literal::Long(val);
}
if (json.is_number_float()) {
return Literal::Double(json.get<double>());
diff --git a/src/iceberg/test/expression_json_test.cc b/src/iceberg/test/expression_json_test.cc
index 7b978ef7..1c993cc1 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 <cstdint>
+#include <limits>
#include <memory>
#include <optional>
#include <string>
@@ -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<int64_t>::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<int64_t>() 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<InvalidLiteralFromJsonTypedParam>& 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<uint64_t>() 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;