From 422930fdae786374a6ebe1c1f1a4f35f8576d2fb Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 5 Oct 2026 15:22:47 +0200 Subject: [PATCH 1/5] GH-51757: [C++] Limit max JSON nesting depth in JSON parser Our JSON parser is inherently recursive, and a crafted JSON document with huge nesting can send it into a stack overflow. Given that real-world JSON files will not have thousands levels of nesting, we limit the JSON nesting to an arbitrary value of 1000. --- cpp/src/arrow/json/parser.cc | 79 ++++++++++++++++++------------- cpp/src/arrow/json/parser_test.cc | 47 ++++++++++++++++++ python/pyarrow/tests/test_json.py | 27 +++++++++++ 3 files changed, 120 insertions(+), 33 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index 2d2363fe4318..b7dd66c2f263 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -17,7 +17,6 @@ #include "arrow/json/parser.h" -#include #include #include #include @@ -47,11 +46,6 @@ namespace json { namespace sj = simdjson::ondemand; -template -static Status ParseError(T&&... t) { - return Status::Invalid("JSON parse error: ", std::forward(t)...); -} - const std::string& Kind::Name(Kind::type kind) { static const std::string names[] = { "null", "boolean", "number", "string", "array", "object", "number_or_string", @@ -121,6 +115,16 @@ Status Kind::ForType(const DataType& type, Kind::type* kind) { return VisitTypeInline(type, &visitor); } +namespace { + +// Tuned to not crash on any of our CI platforms in release mode +constexpr int kMaxNestingDepth = 300; + +template +Status ParseError(T&&... t) { + return Status::Invalid("JSON parse error: ", std::forward(t)...); +} + /// \brief ArrayBuilder for parsed but unconverted arrays template class RawArrayBuilder; @@ -305,20 +309,17 @@ class RawArrayBuilder { return null_bitmap_builder_.Append(count, false); } - Status Finish(std::function*)> finish_child, - std::shared_ptr* out) { + Status Finish(std::shared_ptr child_values, std::shared_ptr* out) { RETURN_NOT_OK(offset_builder_.Append(offset_)); auto size = length(); auto null_count = null_bitmap_builder_.false_count(); std::shared_ptr offsets, null_bitmap; RETURN_NOT_OK(offset_builder_.Finish(&offsets)); RETURN_NOT_OK(null_bitmap_builder_.Finish(&null_bitmap)); - std::shared_ptr values; - RETURN_NOT_OK(finish_child(value_builder_, &values)); - auto type = list(field("item", values->type(), value_builder_.nullable, + auto type = list(field("item", child_values->type(), value_builder_.nullable, Kind::Tag(value_builder_.kind))); - *out = MakeArray(ArrayData::Make(type, size, {null_bitmap, offsets}, {values->data()}, - null_count)); + *out = MakeArray(ArrayData::Make(type, size, {null_bitmap, offsets}, + {child_values->data()}, null_count)); return Status::OK(); } @@ -404,21 +405,18 @@ class RawArrayBuilder { field_infos_[index].builder = builder; } - Status Finish(std::function*)> finish_child, + Status Finish(std::vector> child_data, std::shared_ptr* out) { auto size = length(); auto null_count = null_bitmap_builder_.false_count(); std::shared_ptr null_bitmap; RETURN_NOT_OK(null_bitmap_builder_.Finish(&null_bitmap)); + DCHECK_EQ(child_data.size(), static_cast(num_fields())); std::vector> fields(num_fields()); - std::vector> child_data(num_fields()); for (int i = 0; i < num_fields(); ++i) { const auto& info = field_infos_[i]; - std::shared_ptr field_values; - RETURN_NOT_OK(finish_child(info.builder, &field_values)); - child_data[i] = field_values->data(); - fields[i] = field(std::string(info.name), field_values->type(), + fields[i] = field(std::string(info.name), child_data[i]->type, info.builder.nullable, Kind::Tag(info.builder.kind)); } @@ -581,10 +579,6 @@ class RawBuilderSet { Status Finish(const std::shared_ptr& scalar_values, BuilderPtr builder, std::shared_ptr* out) { - auto finish_children = [this, &scalar_values](BuilderPtr child, - std::shared_ptr* out) { - return Finish(scalar_values, child, out); - }; switch (builder.kind) { case Kind::kNull: { auto length = static_cast(builder.index); @@ -603,11 +597,25 @@ class RawBuilderSet { case Kind::kNumberOrString: return FinishScalar(scalar_values, Cast(builder), out); - case Kind::kArray: - return Cast(builder)->Finish(std::move(finish_children), out); + case Kind::kArray: { + auto array_builder = Cast(builder); + auto child_builder = array_builder->value_builder(); + std::shared_ptr child_values; + RETURN_NOT_OK(Finish(scalar_values, child_builder, &child_values)); + return array_builder->Finish(std::move(child_values), out); + } - case Kind::kObject: - return Cast(builder)->Finish(std::move(finish_children), out); + case Kind::kObject: { + auto object_builder = Cast(builder); + std::vector> child_data(object_builder->num_fields()); + for (int i = 0; i < object_builder->num_fields(); ++i) { + auto child_builder = object_builder->field_builder(i); + std::shared_ptr child_values; + RETURN_NOT_OK(Finish(scalar_values, child_builder, &child_values)); + child_data[i] = child_values->data(); + } + return object_builder->Finish(std::move(child_data), out); + } default: return Status::NotImplemented("invalid builder kind"); @@ -893,15 +901,12 @@ class ParseImpl : public BlockParser { return IllegallyChangedTo(kind); } - StartNested(); - + RETURN_NOT_OK(StartNested()); builder_ = Cast(builder_)->value_builder(); ARROW_ASSIGN_OR_RAISE(auto array, arrow::internal::ResolveSimdjsonResult( value.get_array(), "Failed to get JSON array")); - int64_t size = 0; - for (auto element_result : array) { ARROW_ASSIGN_OR_RAISE(auto element, arrow::internal::ResolveSimdjsonResult( @@ -926,7 +931,7 @@ class ParseImpl : public BlockParser { auto struct_builder = Cast(builder_); absent_fields_stack_.Push(struct_builder->num_fields(), true); - StartNested(); + RETURN_NOT_OK(StartNested()); RETURN_NOT_OK(struct_builder->Append()); ARROW_ASSIGN_OR_RAISE( @@ -1002,10 +1007,16 @@ class ParseImpl : public BlockParser { /// helper method for ParseArray and ParseObject /// adds the current builder to a stack so its /// children can be visited and parsed. - void StartNested() { + Status StartNested() { + if (ARROW_PREDICT_FALSE(builder_stack_.size() >= + static_cast(kMaxNestingDepth))) { + return Status::Invalid("JSON too deeply nested: max nesting depth is ", + kMaxNestingDepth); + } field_index_stack_.push_back(field_index_); field_index_ = -1; builder_stack_.push_back(builder_); + return Status::OK(); } /// helper method for EndArray and EndObject @@ -1049,6 +1060,8 @@ class ParseImpl : public BlockParser { sj::parser parser_; }; +} // namespace + Status BlockParser::Make(MemoryPool* pool, const ParseOptions& options, std::unique_ptr* out) { DCHECK(options.unexpected_field_behavior == UnexpectedFieldBehavior::InferType || diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 28c54b3c5cd5..37861e1dbfe7 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -355,5 +356,51 @@ TEST(BlockParser, NullTypeRejectsNonNullUnderError) { ASSERT_RAISES(Invalid, ParseFromString(options, R"({"a": 5})", &parsed)); } +TEST(BlockParser, NestingDepth) { + auto deeply_nested_json_object = [](int depth) { + std::stringstream ss; + for (int i = 0; i < depth; ++i) { + ss << "{\"a\":"; + } + ss << "1"; + for (int i = 0; i < depth; ++i) { + ss << "}"; + } + return std::move(ss).str(); + }; + + auto deeply_nested_json_array = [](int depth) { + std::stringstream ss; + ss << "{\"a\":"; + for (int i = 0; i < depth - 1; ++i) { + ss << "["; + } + ss << "1"; + for (int i = 0; i < depth - 1; ++i) { + ss << "]"; + } + ss << "}"; + return std::move(ss).str(); + }; + + const int kMaxDepth = 300; // hard-coded in parser.cc + std::shared_ptr parsed; + ASSERT_OK(ParseFromString(ParseOptions::Defaults(), + deeply_nested_json_object(kMaxDepth), &parsed)); + ASSERT_OK(parsed->ValidateFull()); + ASSERT_OK(ParseFromString(ParseOptions::Defaults(), deeply_nested_json_array(kMaxDepth), + &parsed)); + ASSERT_OK(parsed->ValidateFull()); + + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ParseFromString(ParseOptions::Defaults(), deeply_nested_json_object(kMaxDepth + 1), + &parsed)); + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ParseFromString(ParseOptions::Defaults(), deeply_nested_json_array(kMaxDepth + 1), + &parsed)); +} + } // namespace json } // namespace arrow diff --git a/python/pyarrow/tests/test_json.py b/python/pyarrow/tests/test_json.py index e7ab574ead48..3b271fe05a70 100644 --- a/python/pyarrow/tests/test_json.py +++ b/python/pyarrow/tests/test_json.py @@ -22,6 +22,7 @@ import itertools import json import string +import sys import unittest try: @@ -344,6 +345,32 @@ def test_stress_block_sizes(self): # Better error output assert table.to_pydict() == expected.to_pydict() + def test_max_nesting_depth(self): + if pa.build_info.build_type == 'debug' and sys.platform == 'darwin': + pytest.skip('test crashes in debug mode on macOS ' + 'due to small default thread stack size') + + def deeply_nested_json_object(depth): + return b'{"a":' * depth + b'1' + b'}' * depth + + def deeply_nested_json_array(depth): + return b'{"a":' + b'[' * (depth - 1) + b'1' + b']' * (depth - 1) + b'}' + + max_depth = 300 # Hard-coded in arrow/json/parser.cc + table = self.read_bytes(deeply_nested_json_object(max_depth)) + table.validate(full=True) + table = self.read_bytes(deeply_nested_json_array(max_depth)) + table.validate(full=True) + + with pytest.raises( + ValueError, + match=f"JSON too deeply nested: max nesting depth is {max_depth}"): + self.read_bytes(deeply_nested_json_object(max_depth + 1)) + with pytest.raises( + ValueError, + match=f"JSON too deeply nested: max nesting depth is {max_depth}"): + self.read_bytes(deeply_nested_json_array(max_depth + 1)) + class BaseTestJSONRead(BaseTestJSON): From 1d3da0dfe8e0f315e1f88fd7ef212b73035ddfc3 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 6 Oct 2026 16:07:35 +0200 Subject: [PATCH 2/5] Also guard ConsumeJsonValue against stack overflow --- cpp/src/arrow/json/parser.cc | 4 +- cpp/src/arrow/json/parser_test.cc | 41 ++++++++++------ cpp/src/arrow/util/simdjson_internal.cc | 48 +++++++++++-------- cpp/src/arrow/util/simdjson_internal.h | 9 ++-- .../parquet/geospatial/util_json_internal.cc | 8 +++- cpp/src/parquet/reader_test.cc | 3 +- 6 files changed, 69 insertions(+), 44 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index b7dd66c2f263..bb20a48dd157 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -676,7 +676,9 @@ class ParseImpl : public BlockParser { return ParseError("unexpected field"); case UnexpectedFieldBehavior::Ignore: - return internal::ConsumeJsonValue(value); + return internal::ConsumeJsonValue( + value, kMaxNestingDepth, + /*depth=*/static_cast(builder_stack_.size() - 1)); case UnexpectedFieldBehavior::InferType: { // If an unexpected field is encountered, add a NullBuilder with leading nulls. diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 37861e1dbfe7..7958f671d8fe 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -384,22 +384,33 @@ TEST(BlockParser, NestingDepth) { }; const int kMaxDepth = 300; // hard-coded in parser.cc + ParseOptions options = ParseOptions::Defaults(); std::shared_ptr parsed; - ASSERT_OK(ParseFromString(ParseOptions::Defaults(), - deeply_nested_json_object(kMaxDepth), &parsed)); - ASSERT_OK(parsed->ValidateFull()); - ASSERT_OK(ParseFromString(ParseOptions::Defaults(), deeply_nested_json_array(kMaxDepth), - &parsed)); - ASSERT_OK(parsed->ValidateFull()); - - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), - ParseFromString(ParseOptions::Defaults(), deeply_nested_json_object(kMaxDepth + 1), - &parsed)); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), - ParseFromString(ParseOptions::Defaults(), deeply_nested_json_array(kMaxDepth + 1), - &parsed)); + + for (UnexpectedFieldBehavior unexpected_field_behavior : + {UnexpectedFieldBehavior::Ignore, UnexpectedFieldBehavior::InferType}) { + options.unexpected_field_behavior = unexpected_field_behavior; + options.explicit_schema = schema({{"not_here", int32()}}); + ASSERT_OK(ParseFromString(options, deeply_nested_json_object(kMaxDepth), &parsed)); + ASSERT_OK(parsed->ValidateFull()); + ASSERT_OK(ParseFromString(options, deeply_nested_json_array(kMaxDepth), &parsed)); + ASSERT_OK(parsed->ValidateFull()); + + // `kMaxDepth + 1` is the first value that triggers an error (but wouldn't trigger + // a stack overflow otherwise). + // 100'000 would definitely trigger a stack overflow, validate that the error is + // detected before that would happen. + for (int depth : {kMaxDepth + 1, 100'000}) { + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, + ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ParseFromString(options, deeply_nested_json_object(depth), &parsed)); + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, + ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ParseFromString(options, deeply_nested_json_array(depth), &parsed)); + } + } } } // namespace json diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index d845eab0d473..32f17526775e 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -538,47 +538,57 @@ Result MinifyJson(std::string_view json) { return minified; } -Status ConsumeJsonValue(simdjson::ondemand::value value) { - return VisitJsonValue( - value, ValidateJsonObject, ValidateJsonArray, - [](std::string_view) { return Status::OK(); }, [](bool) { return Status::OK(); }, - []() { return Status::OK(); }, [](int64_t) { return Status::OK(); }, - [](uint64_t) { return Status::OK(); }, [](double) { return Status::OK(); }, - [](simdjson::ondemand::value) { return Status::OK(); }); -} - -Status ValidateJsonObject(simdjson::ondemand::object object) { +static Status ValidateJsonObject(simdjson::ondemand::object object, int max_depth, + int depth) { + if (ARROW_PREDICT_FALSE(depth >= max_depth)) { + return Status::Invalid("JSON too deeply nested: max nesting depth is ", max_depth); + } for (auto field_result : object) { ARROW_ASSIGN_OR_RAISE( auto field, ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); - - RETURN_NOT_OK(ConsumeJsonValue(field.value())); + RETURN_NOT_OK(ConsumeJsonValue(field.value(), max_depth, depth)); } - return Status::OK(); } -Status ValidateJsonArray(simdjson::ondemand::array array) { +static Status ValidateJsonArray(simdjson::ondemand::array array, int max_depth, + int depth) { + if (ARROW_PREDICT_FALSE(depth >= max_depth)) { + return Status::Invalid("JSON too deeply nested: max nesting depth is ", max_depth); + } for (auto element_result : array) { ARROW_ASSIGN_OR_RAISE( auto value, ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); - - RETURN_NOT_OK(ConsumeJsonValue(value)); + RETURN_NOT_OK(ConsumeJsonValue(value, max_depth, depth)); } - return Status::OK(); } +Status ConsumeJsonValue(simdjson::ondemand::value value, int max_depth, int depth) { + return VisitJsonValue( + value, + [depth, max_depth](simdjson::ondemand::object object) { + return ValidateJsonObject(object, max_depth, depth + 1); + }, + [depth, max_depth](simdjson::ondemand::array array) { + return ValidateJsonArray(array, max_depth, depth + 1); + }, + [](std::string_view) { return Status::OK(); }, [](bool) { return Status::OK(); }, + []() { return Status::OK(); }, [](int64_t) { return Status::OK(); }, + [](uint64_t) { return Status::OK(); }, [](double) { return Status::OK(); }, + [](simdjson::ondemand::value) { return Status::OK(); }); +} + Status ValidateJsonDocument(simdjson::ondemand::parser& parser, - simdjson::padded_string& json) { + simdjson::padded_string& json, int max_depth) { ARROW_ASSIGN_OR_RAISE( auto document, ResolveSimdjsonResult(parser.iterate(json), "Failed to parse JSON")); ARROW_ASSIGN_OR_RAISE(auto value, ResolveSimdjsonResult(document.get_value(), "Failed to get JSON value")); - return ConsumeJsonValue(value); + return ConsumeJsonValue(value, max_depth); } /// Returns the position of the first non-whitespace character when trailing is false, diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index d5816522bee0..d4ca184d00bf 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -341,14 +341,11 @@ Result GetJsonField(simdjson::ondemand::object& object, std::string_view key) ARROW_EXPORT Result MinifyJson(std::string_view json); -ARROW_EXPORT Status ValidateJsonObject(simdjson::ondemand::object object); - -ARROW_EXPORT Status ValidateJsonArray(simdjson::ondemand::array array); - -ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value); +ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value, int max_depth, + int depth = 0); ARROW_EXPORT Status ValidateJsonDocument(simdjson::ondemand::parser& parser, - simdjson::padded_string& json); + simdjson::padded_string& json, int max_depth); ARROW_EXPORT int64_t ConsumeJsonWhitespace(std::string_view view, bool trailing); diff --git a/cpp/src/parquet/geospatial/util_json_internal.cc b/cpp/src/parquet/geospatial/util_json_internal.cc index 6933d692795f..308c882fddc8 100644 --- a/cpp/src/parquet/geospatial/util_json_internal.cc +++ b/cpp/src/parquet/geospatial/util_json_internal.cc @@ -30,6 +30,10 @@ namespace parquet { namespace { + +// An upper bound on the expected nesting depth of a geospatial JSON metadata object +constexpr int kMaxJsonDepth = 20; + ::arrow::Result GeospatialGeoArrowCrsToParquetCrs( simdjson::ondemand::object object) { auto json_crs_result = @@ -195,7 +199,7 @@ ::arrow::Result EscapeCrsAsJsonIfRequired(std::string_view crs) { simdjson::ondemand::parser parser; simdjson::padded_string json(crs); - if (!::arrow::internal::ValidateJsonDocument(parser, json).ok()) { + if (!::arrow::internal::ValidateJsonDocument(parser, json, kMaxJsonDepth).ok()) { return EscapeJsonString(crs); } @@ -215,7 +219,7 @@ LogicalTypeFromGeoArrowMetadata(std::string_view serialized_data) { simdjson::ondemand::parser parser; simdjson::padded_string json(serialized_data); - RETURN_NOT_OK(::arrow::internal::ValidateJsonDocument(parser, json)); + RETURN_NOT_OK(::arrow::internal::ValidateJsonDocument(parser, json, kMaxJsonDepth)); // Reparse because validation consumes the On-Demand document. ARROW_ASSIGN_OR_RAISE(auto document, ::arrow::internal::ResolveSimdjsonResult( diff --git a/cpp/src/parquet/reader_test.cc b/cpp/src/parquet/reader_test.cc index b5487fa46899..05ce29848737 100644 --- a/cpp/src/parquet/reader_test.cc +++ b/cpp/src/parquet/reader_test.cc @@ -1445,7 +1445,8 @@ ::arrow::Status CheckJsonValid(std::string_view json_string) { simdjson::ondemand::parser parser; auto padded_json = simdjson::padded_string(json_string); - RETURN_NOT_OK(::arrow::internal::ValidateJsonDocument(parser, padded_json)); + RETURN_NOT_OK( + ::arrow::internal::ValidateJsonDocument(parser, padded_json, /*max_depth=*/10)); return ::arrow::Status::OK(); } From a7c54294c1c97e3b9757c6aabf961f21ff2d975a Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 6 Oct 2026 16:41:35 +0200 Subject: [PATCH 3/5] Further reduce limit --- ci/scripts/python_wheel_unix_test.sh | 2 +- cpp/src/arrow/json/parser.cc | 32 +++++++++++++++------------- cpp/src/arrow/json/parser_test.cc | 6 +++--- python/pyarrow/tests/test_json.py | 2 +- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/ci/scripts/python_wheel_unix_test.sh b/ci/scripts/python_wheel_unix_test.sh index cb445611e233..d3138c5e9889 100755 --- a/ci/scripts/python_wheel_unix_test.sh +++ b/ci/scripts/python_wheel_unix_test.sh @@ -106,5 +106,5 @@ if [ "${CHECK_UNITTESTS}" == "ON" ]; then # Execute unittest, test dependencies must be installed python -c 'import pyarrow; pyarrow.create_library_symlinks()' - python -m pytest -r s --pyargs pyarrow + python -m pytest -r s --pyargs pyarrow.tests.test_json fi diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index bb20a48dd157..14553c723678 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -117,8 +117,10 @@ Status Kind::ForType(const DataType& type, Kind::type* kind) { namespace { -// Tuned to not crash on any of our CI platforms in release mode -constexpr int kMaxNestingDepth = 300; +// Tuned to not crash on any of our CI platforms in release mode. +// This may seem low but some platforms have very low thread stack sizes +// (128 kiB on musllinux). +constexpr int kMaxNestingDepth = 100; template Status ParseError(T&&... t) { @@ -670,14 +672,14 @@ class ParseImpl : public BlockParser { return builder_set_.AppendNull(builder_stack_.back(), field_index_, builder_); } - Status HandleUnexpectedField(std::string_view key, sj::value value) { + Status HandleUnexpectedField(std::string_view key, sj::value& value) { switch (unexpected_field_behavior_) { case UnexpectedFieldBehavior::Error: return ParseError("unexpected field"); case UnexpectedFieldBehavior::Ignore: return internal::ConsumeJsonValue( - value, kMaxNestingDepth, + std::move(value), kMaxNestingDepth, /*depth=*/static_cast(builder_stack_.size() - 1)); case UnexpectedFieldBehavior::InferType: { @@ -843,7 +845,7 @@ class ParseImpl : public BlockParser { return Status::OK(); } - Status ParseValue(sj::value value) { + Status ParseValue(sj::value& value) { ARROW_ASSIGN_OR_RAISE(auto type, arrow::internal::ResolveSimdjsonResult( value.type(), "Failed to determine JSON type")); @@ -897,7 +899,7 @@ class ParseImpl : public BlockParser { return Status::OK(); } - Status ParseArray(sj::value value) { + Status ParseArray(sj::value& value) { constexpr auto kind = Kind::kArray; if (ARROW_PREDICT_FALSE(builder_.kind != kind)) { return IllegallyChangedTo(kind); @@ -910,9 +912,9 @@ class ParseImpl : public BlockParser { value.get_array(), "Failed to get JSON array")); int64_t size = 0; for (auto element_result : array) { - ARROW_ASSIGN_OR_RAISE(auto element, - arrow::internal::ResolveSimdjsonResult( - element_result, "Failed to iterate JSON array")); + ARROW_ASSIGN_OR_RAISE( + auto element, arrow::internal::ResolveSimdjsonResult( + std::move(element_result), "Failed to iterate JSON array")); RETURN_NOT_OK(ParseValue(element)); ++size; @@ -925,7 +927,7 @@ class ParseImpl : public BlockParser { return list_builder->Append(static_cast(size)); } - Status ParseObject(sj::value value) { + Status ParseObject(sj::value& value) { constexpr auto kind = Kind::kObject; if (ARROW_PREDICT_FALSE(builder_.kind != kind)) { return IllegallyChangedTo(kind); @@ -941,10 +943,10 @@ class ParseImpl : public BlockParser { "Failed to get JSON object")); for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE( - auto field, - arrow::internal::ResolveSimdjsonResult( - field_result, "JSON parse error: Failed to iterate JSON object")); + ARROW_ASSIGN_OR_RAISE(auto field, + arrow::internal::ResolveSimdjsonResult( + std::move(field_result), + "JSON parse error: Failed to iterate JSON object")); ARROW_ASSIGN_OR_RAISE(auto key, arrow::internal::ResolveSimdjsonResult( @@ -971,7 +973,7 @@ class ParseImpl : public BlockParser { return Status::OK(); } - Status ParseObjectField(std::string_view key, sj::value value) { + Status ParseObjectField(std::string_view key, sj::value& value) { auto parent = Cast(builder_stack_.back()); field_index_ = parent->GetFieldIndex(key); if (ARROW_PREDICT_FALSE(field_index_ == -1)) { diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 7958f671d8fe..6c32fd7cab3d 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -383,7 +383,7 @@ TEST(BlockParser, NestingDepth) { return std::move(ss).str(); }; - const int kMaxDepth = 300; // hard-coded in parser.cc + const int kMaxDepth = 100; // hard-coded in parser.cc ParseOptions options = ParseOptions::Defaults(); std::shared_ptr parsed; @@ -403,11 +403,11 @@ TEST(BlockParser, NestingDepth) { for (int depth : {kMaxDepth + 1, 100'000}) { EXPECT_RAISES_WITH_MESSAGE_THAT( Invalid, - ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 100"), ParseFromString(options, deeply_nested_json_object(depth), &parsed)); EXPECT_RAISES_WITH_MESSAGE_THAT( Invalid, - ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 300"), + ::testing::HasSubstr("JSON too deeply nested: max nesting depth is 100"), ParseFromString(options, deeply_nested_json_array(depth), &parsed)); } } diff --git a/python/pyarrow/tests/test_json.py b/python/pyarrow/tests/test_json.py index 3b271fe05a70..2239c2f1bb8b 100644 --- a/python/pyarrow/tests/test_json.py +++ b/python/pyarrow/tests/test_json.py @@ -356,7 +356,7 @@ def deeply_nested_json_object(depth): def deeply_nested_json_array(depth): return b'{"a":' + b'[' * (depth - 1) + b'1' + b']' * (depth - 1) + b'}' - max_depth = 300 # Hard-coded in arrow/json/parser.cc + max_depth = 100 # Hard-coded in arrow/json/parser.cc table = self.read_bytes(deeply_nested_json_object(max_depth)) table.validate(full=True) table = self.read_bytes(deeply_nested_json_array(max_depth)) From bfc2f9f7cd3e10d3da35a3a919603f9447531c33 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 6 Oct 2026 16:45:11 +0200 Subject: [PATCH 4/5] Rename function --- cpp/src/arrow/json/parser.cc | 2 +- cpp/src/arrow/util/simdjson_internal.cc | 8 ++++---- cpp/src/arrow/util/simdjson_internal.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index 14553c723678..69373d07fb63 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -678,7 +678,7 @@ class ParseImpl : public BlockParser { return ParseError("unexpected field"); case UnexpectedFieldBehavior::Ignore: - return internal::ConsumeJsonValue( + return internal::ValidateJsonValue( std::move(value), kMaxNestingDepth, /*depth=*/static_cast(builder_stack_.size() - 1)); diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index 32f17526775e..d8cea1a5536c 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -546,7 +546,7 @@ static Status ValidateJsonObject(simdjson::ondemand::object object, int max_dept for (auto field_result : object) { ARROW_ASSIGN_OR_RAISE( auto field, ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); - RETURN_NOT_OK(ConsumeJsonValue(field.value(), max_depth, depth)); + RETURN_NOT_OK(ValidateJsonValue(field.value(), max_depth, depth)); } return Status::OK(); } @@ -560,12 +560,12 @@ static Status ValidateJsonArray(simdjson::ondemand::array array, int max_depth, ARROW_ASSIGN_OR_RAISE( auto value, ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); - RETURN_NOT_OK(ConsumeJsonValue(value, max_depth, depth)); + RETURN_NOT_OK(ValidateJsonValue(value, max_depth, depth)); } return Status::OK(); } -Status ConsumeJsonValue(simdjson::ondemand::value value, int max_depth, int depth) { +Status ValidateJsonValue(simdjson::ondemand::value value, int max_depth, int depth) { return VisitJsonValue( value, [depth, max_depth](simdjson::ondemand::object object) { @@ -588,7 +588,7 @@ Status ValidateJsonDocument(simdjson::ondemand::parser& parser, ARROW_ASSIGN_OR_RAISE(auto value, ResolveSimdjsonResult(document.get_value(), "Failed to get JSON value")); - return ConsumeJsonValue(value, max_depth); + return ValidateJsonValue(value, max_depth); } /// Returns the position of the first non-whitespace character when trailing is false, diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index d4ca184d00bf..fbdf8639dad6 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -341,8 +341,8 @@ Result GetJsonField(simdjson::ondemand::object& object, std::string_view key) ARROW_EXPORT Result MinifyJson(std::string_view json); -ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value, int max_depth, - int depth = 0); +ARROW_EXPORT Status ValidateJsonValue(simdjson::ondemand::value value, int max_depth, + int depth = 0); ARROW_EXPORT Status ValidateJsonDocument(simdjson::ondemand::parser& parser, simdjson::padded_string& json, int max_depth); From e71ab61efa6863d73fd440d8361249fc5abb1b58 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 6 Oct 2026 16:46:18 +0200 Subject: [PATCH 5/5] Undo debug change --- ci/scripts/python_wheel_unix_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/python_wheel_unix_test.sh b/ci/scripts/python_wheel_unix_test.sh index d3138c5e9889..cb445611e233 100755 --- a/ci/scripts/python_wheel_unix_test.sh +++ b/ci/scripts/python_wheel_unix_test.sh @@ -106,5 +106,5 @@ if [ "${CHECK_UNITTESTS}" == "ON" ]; then # Execute unittest, test dependencies must be installed python -c 'import pyarrow; pyarrow.create_library_symlinks()' - python -m pytest -r s --pyargs pyarrow.tests.test_json + python -m pytest -r s --pyargs pyarrow fi