From 7d5a372356a04680fac0a8fd73c7c763244971a6 Mon Sep 17 00:00:00 2001 From: "shuxu.li" Date: Thu, 15 Oct 2026 21:32:41 +0800 Subject: [PATCH] fix(json): limit nesting depth of parsed JSON documents Type, name-mapping and expression parsers walk parsed JSON recursively, so a deeply nested document could exhaust the stack. Check the nesting depth of objects and arrays iteratively right after parsing in FromJsonString and FileMetadataFromJsonString, and reject documents nested deeper than 1000 levels, matching the default limit of Jackson's StreamReadConstraints. --- src/iceberg/json_serde.cc | 1 + src/iceberg/puffin/json_serde.cc | 1 + src/iceberg/test/puffin_json_test.cc | 8 +++ src/iceberg/test/schema_json_test.cc | 100 ++++++++++++++++++++++++++ src/iceberg/util/json_util_internal.h | 36 ++++++++++ 5 files changed, 146 insertions(+) diff --git a/src/iceberg/json_serde.cc b/src/iceberg/json_serde.cc index 0cc4226c6c..5d2e5799f9 100644 --- a/src/iceberg/json_serde.cc +++ b/src/iceberg/json_serde.cc @@ -1703,6 +1703,7 @@ Result FromJsonString(const std::string& json_string) { if (json.is_discarded()) [[unlikely]] { return JsonParseError("Failed to parse JSON string: {}", json_string); } + ICEBERG_RETURN_UNEXPECTED(CheckJsonNestingDepth(json)); return json; } diff --git a/src/iceberg/puffin/json_serde.cc b/src/iceberg/puffin/json_serde.cc index 53f591e323..20aaa9b31a 100644 --- a/src/iceberg/puffin/json_serde.cc +++ b/src/iceberg/puffin/json_serde.cc @@ -118,6 +118,7 @@ Result FileMetadataFromJsonString(std::string_view json_string) { } try { auto json = nlohmann::json::parse(json_string); + ICEBERG_RETURN_UNEXPECTED(CheckJsonNestingDepth(json)); return FileMetadataFromJson(json); } catch (const nlohmann::json::parse_error& e) { return JsonParseError("Failed to parse JSON: {}", e.what()); diff --git a/src/iceberg/test/puffin_json_test.cc b/src/iceberg/test/puffin_json_test.cc index f8faf8dd1e..1c9b00abd0 100644 --- a/src/iceberg/test/puffin_json_test.cc +++ b/src/iceberg/test/puffin_json_test.cc @@ -198,4 +198,12 @@ TEST(PuffinJsonTest, FileMetadataFromInvalidString) { IsError(ErrorKind::kJsonParseError)); } +TEST(PuffinJsonTest, FileMetadataRejectsDeeplyNestedJson) { + const std::string json = R"({"blobs":[],"properties":)" + std::string(100'000, '[') + + std::string(100'000, ']') + "}"; + auto result = FileMetadataFromJsonString(json); + EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(result, HasErrorMessage("nesting depth")); +} + } // namespace iceberg::puffin diff --git a/src/iceberg/test/schema_json_test.cc b/src/iceberg/test/schema_json_test.cc index f90a0c2caf..ecdfb7ba62 100644 --- a/src/iceberg/test/schema_json_test.cc +++ b/src/iceberg/test/schema_json_test.cc @@ -17,6 +17,8 @@ * under the License. */ +#include +#include #include #include @@ -30,6 +32,11 @@ #include "iceberg/schema_field.h" #include "iceberg/test/matchers.h" #include "iceberg/type.h" +#include "iceberg/util/json_util_internal.h" + +#ifndef _WIN32 +# include +#endif namespace iceberg { @@ -379,4 +386,97 @@ TEST(SchemaJsonTest, IdentifierFieldIds) { ASSERT_EQ(json_multi_identifiers_out, json_multi_identifiers); } +namespace { + +std::string NestedArrayJson(int32_t depth) { + return std::string(depth, '[') + std::string(depth, ']'); +} + +std::string NestedObjectJson(int32_t depth) { + std::string json; + for (int32_t i = 0; i < depth; ++i) { + json += R"({"a":)"; + } + json += "1"; + json += std::string(depth, '}'); + return json; +} + +/// \brief A schema with one field whose type is `list_depth` nested lists of int. +/// The resulting JSON document nests objects and arrays `list_depth + 3` deep. +std::string NestedListSchemaJson(int32_t list_depth) { + std::string json = + R"({"type":"struct","schema-id":0,"fields":[{"id":1,"name":"f","required":true,"type":)"; + for (int32_t i = 0; i < list_depth; ++i) { + json += std::format( + R"({{"type":"list","element-id":{},"element-required":true,"element":)", i + 2); + } + json += R"("int")"; + json += std::string(list_depth, '}'); + json += "}]}"; + return json; +} + +} // namespace + +TEST(JsonNestingDepthTest, RejectsDeeplyNestedDocuments) { + for (const auto& json : {NestedArrayJson(100'000), NestedObjectJson(100'000)}) { + auto result = FromJsonString(json); + EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(result, HasErrorMessage("nesting depth")); + } +} + +TEST(JsonNestingDepthTest, AcceptsDocumentAtTheLimit) { + EXPECT_THAT(FromJsonString(NestedArrayJson(kMaxJsonNestingDepth)), IsOk()); + EXPECT_THAT(FromJsonString(NestedObjectJson(kMaxJsonNestingDepth)), IsOk()); + EXPECT_THAT(FromJsonString(NestedArrayJson(kMaxJsonNestingDepth + 1)), + IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(FromJsonString(NestedObjectJson(kMaxJsonNestingDepth + 1)), + IsError(ErrorKind::kJsonParseError)); +} + +TEST(JsonNestingDepthTest, RejectsDeeplyNestedSchema) { + auto result = FromJsonString(NestedListSchemaJson(100'000)); + EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(result, HasErrorMessage("nesting depth")); +} + +// The recursive type parser, schema validation and destruction must fit in an +// 8 MiB stack (the default main-thread stack on Linux and macOS) for the +// deepest schema the depth check lets through. +TEST(JsonNestingDepthTest, SchemaAtTheLimitParsesOn8MiBStack) { +#ifdef _WIN32 + GTEST_SKIP() << "Explicit thread stack size is only set up for POSIX"; +#else + struct Context { + std::string json; + bool parsed = false; + bool valid = false; + } context{.json = NestedListSchemaJson(kMaxJsonNestingDepth - 3)}; + + auto run = [](void* arg) -> void* { + auto* ctx = static_cast(arg); + auto document = FromJsonString(ctx->json); + if (!document.has_value()) { + return nullptr; + } + auto schema = SchemaFromJson(*document); + ctx->parsed = schema.has_value(); + ctx->valid = ctx->parsed && (*schema)->Validate(/*format_version=*/2).has_value(); + return nullptr; + }; + + pthread_attr_t attr; + ASSERT_EQ(pthread_attr_init(&attr), 0); + ASSERT_EQ(pthread_attr_setstacksize(&attr, size_t{8} * 1024 * 1024), 0); + pthread_t thread; + ASSERT_EQ(pthread_create(&thread, &attr, run, &context), 0); + pthread_attr_destroy(&attr); + ASSERT_EQ(pthread_join(thread, nullptr), 0); + + EXPECT_TRUE(context.parsed); + EXPECT_TRUE(context.valid); +#endif +} } // namespace iceberg diff --git a/src/iceberg/util/json_util_internal.h b/src/iceberg/util/json_util_internal.h index b1edf583d9..2bb92c59fe 100644 --- a/src/iceberg/util/json_util_internal.h +++ b/src/iceberg/util/json_util_internal.h @@ -20,9 +20,12 @@ #pragma once #include +#include #include #include #include +#include +#include #include @@ -57,6 +60,39 @@ inline void SetOptionalStringField(nlohmann::json& json, std::string_view key, } } +/// \brief Maximum nesting depth of objects and arrays accepted in a JSON +/// document, matching the default limit of Jackson's StreamReadConstraints. +/// +/// Documents at this depth are parsed by recursive code. A schema nested close +/// to the limit fits in an 8 MiB stack, but may not fit in a much smaller one +/// (e.g. the 512 KiB default for secondary threads on macOS). +inline constexpr int32_t kMaxJsonNestingDepth = 1000; + +/// \brief Check that a JSON document does not nest objects and arrays deeper +/// than max_depth. +/// +/// The document is walked iteratively, so arbitrarily deep input is handled +/// without recursion. Callers that hand parsed documents to recursive code +/// (type, name-mapping and expression parsers) must run this check first. +inline Status CheckJsonNestingDepth(const nlohmann::json& json, + int32_t max_depth = kMaxJsonNestingDepth) { + std::vector> pending{{&json, 1}}; + while (!pending.empty()) { + auto [node, depth] = pending.back(); + pending.pop_back(); + if (!node->is_structured()) { + continue; + } + if (depth > max_depth) { + return JsonParseError("JSON nesting depth exceeds the maximum of {}", max_depth); + } + for (const auto& child : *node) { + pending.emplace_back(&child, depth + 1); + } + } + return {}; +} + inline std::string SafeDumpJson(const nlohmann::json& json) { return json.dump(/*indent=*/-1, /*indent_char=*/' ', /*ensure_ascii=*/false, nlohmann::detail::error_handler_t::ignore);