diff --git a/src/iceberg/json_serde.cc b/src/iceberg/json_serde.cc index e852060ffc..37f6c08c94 100644 --- a/src/iceberg/json_serde.cc +++ b/src/iceberg/json_serde.cc @@ -1705,13 +1705,7 @@ Result> TableMetadataFromJson(const nlohmann::jso } Result FromJsonString(const std::string& json_string) { - auto json = - nlohmann::json::parse(json_string, /*cb=*/nullptr, /*allow_exceptions=*/false); - if (json.is_discarded()) [[unlikely]] { - // Report the size only, not the raw input. - return JsonParseError("Failed to parse JSON string of {} bytes", json_string.size()); - } - return json; + return ParseJsonString(json_string); } Result ToJsonString(const nlohmann::json& json) { diff --git a/src/iceberg/puffin/json_serde.cc b/src/iceberg/puffin/json_serde.cc index 53f591e323..e2006317f5 100644 --- a/src/iceberg/puffin/json_serde.cc +++ b/src/iceberg/puffin/json_serde.cc @@ -116,12 +116,8 @@ Result FileMetadataFromJsonString(std::string_view json_string) { if (json_string.empty()) { return JsonParseError("Cannot parse empty JSON string"); } - try { - auto json = nlohmann::json::parse(json_string); - return FileMetadataFromJson(json); - } catch (const nlohmann::json::parse_error& e) { - return JsonParseError("Failed to parse JSON: {}", e.what()); - } + ICEBERG_ASSIGN_OR_RAISE(auto json, ParseJsonString(json_string)); + return FileMetadataFromJson(json); } } // namespace iceberg::puffin diff --git a/src/iceberg/test/puffin_json_test.cc b/src/iceberg/test/puffin_json_test.cc index f8faf8dd1e..df8869b62a 100644 --- a/src/iceberg/test/puffin_json_test.cc +++ b/src/iceberg/test/puffin_json_test.cc @@ -25,6 +25,7 @@ #include "iceberg/puffin/file_metadata.h" #include "iceberg/puffin/json_serde_internal.h" #include "iceberg/test/matchers.h" +#include "iceberg/util/json_util_internal.h" namespace iceberg::puffin { @@ -198,4 +199,13 @@ TEST(PuffinJsonTest, FileMetadataFromInvalidString) { IsError(ErrorKind::kJsonParseError)); } +TEST(PuffinJsonTest, FileMetadataRejectsDeeplyNestedJson) { + const std::string json = R"({"blobs":[],"properties":)" + + std::string(kMaxJsonNestingDepth, '[') + + std::string(kMaxJsonNestingDepth, ']') + "}"; + 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..5067191bf9 100644 --- a/src/iceberg/test/schema_json_test.cc +++ b/src/iceberg/test/schema_json_test.cc @@ -30,6 +30,7 @@ #include "iceberg/schema_field.h" #include "iceberg/test/matchers.h" #include "iceberg/type.h" +#include "iceberg/util/json_util_internal.h" namespace iceberg { @@ -379,4 +380,25 @@ TEST(SchemaJsonTest, IdentifierFieldIds) { ASSERT_EQ(json_multi_identifiers_out, json_multi_identifiers); } +TEST(JsonNestingDepthTest, EnforcesConfiguredLimit) { + EXPECT_THAT(ParseJsonString("[[[]]]", /*max_depth=*/3), IsOk()); + EXPECT_THAT(ParseJsonString(R"({"a":[{"value":"[{}]"}]})", /*max_depth=*/3), IsOk()); + // Reject excessive nesting before reading the invalid trailing input. + for (const auto* json : {"[[[[invalid", R"({"a":{"b":{"c":{invalid)"}) { + SCOPED_TRACE(json); + auto result = ParseJsonString(json, /*max_depth=*/3); + EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(result, HasErrorMessage("nesting depth exceeds the maximum of 3")); + } +} + +TEST(JsonNestingDepthTest, StringEntryPointEnforcesDefaultLimit) { + const std::string at_limit = + std::string(kMaxJsonNestingDepth, '[') + std::string(kMaxJsonNestingDepth, ']'); + EXPECT_THAT(FromJsonString(at_limit), IsOk()); + auto result = FromJsonString("[" + at_limit + "]"); + EXPECT_THAT(result, IsError(ErrorKind::kJsonParseError)); + EXPECT_THAT(result, HasErrorMessage("nesting depth")); +} + } // namespace iceberg diff --git a/src/iceberg/util/json_util_internal.h b/src/iceberg/util/json_util_internal.h index adb6b2faba..f49a20e818 100644 --- a/src/iceberg/util/json_util_internal.h +++ b/src/iceberg/util/json_util_internal.h @@ -20,9 +20,11 @@ #pragma once #include +#include #include #include #include +#include #include @@ -57,6 +59,37 @@ inline void SetOptionalStringField(nlohmann::json& json, std::string_view key, } } +/// \brief Default object/array nesting limit for parsed JSON documents. +inline constexpr int32_t kMaxJsonNestingDepth = 200; + +/// \brief Parse JSON and stop before constructing containers beyond max_depth. +inline Result ParseJsonString(std::string_view json_string, + int32_t max_depth = kMaxJsonNestingDepth) { + struct NestingDepthExceeded {}; + const auto check_depth = [max_depth](int depth, nlohmann::json::parse_event_t event, + nlohmann::json&) { + if ((event == nlohmann::json::parse_event_t::object_start || + event == nlohmann::json::parse_event_t::array_start) && + depth >= max_depth) { + // Returning false only discards a value; throw to stop parsing immediately. + throw NestingDepthExceeded{}; + } + return true; + }; + + try { + auto json = nlohmann::json::parse(json_string, check_depth, + /*allow_exceptions=*/false); + if (json.is_discarded()) [[unlikely]] { + return JsonParseError("Failed to parse JSON string of {} bytes", + json_string.size()); + } + return json; + } catch (const NestingDepthExceeded&) { + return JsonParseError("JSON nesting depth exceeds the maximum of {}", max_depth); + } +} + /// \brief Dump a JSON document to a string, ignoring encoding errors. /// /// Not intended for error messages: parse errors describe the offending key