Skip to content

GH-51757: [C++] Limit max JSON nesting depth in JSON parser #51758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pitrou wants to merge 5 commits into
base: main
Choose a base branch
from
Open

GH-51757: [C++] Limit max JSON nesting depth in JSON parser #51758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
422930f
GH-51757: [C++] Limit max JSON nesting depth in JSON parser
pitrou Oct 5, 2026
1d3da0d
Also guard ConsumeJsonValue against stack overflow
pitrou Oct 6, 2026
a7c5429
Further reduce limit
pitrou Oct 6, 2026
bfc2f9f
Rename function
pitrou Oct 6, 2026
e71ab61
Undo debug change
pitrou Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 63 additions & 46 deletions cpp/src/arrow/json/parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "arrow/json/parser.h"

#include <functional>
#include <limits>
#include <memory>
#include <string_view>
Expand Down Expand Up @@ -47,11 +46,6 @@ namespace json {

namespace sj = simdjson::ondemand;

template <typename... T>
static Status ParseError(T&&... t) {
return Status::Invalid("JSON parse error: ", std::forward<T>(t)...);
}

const std::string& Kind::Name(Kind::type kind) {
static const std::string names[] = {
"null", "boolean", "number", "string", "array", "object", "number_or_string",
Expand Down Expand Up @@ -121,6 +115,18 @@ 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.
// This may seem low but some platforms have very low thread stack sizes
// (128 kiB on musllinux).
constexpr int kMaxNestingDepth = 100;

template <typename... T>
Status ParseError(T&&... t) {
return Status::Invalid("JSON parse error: ", std::forward<T>(t)...);
}

/// \brief ArrayBuilder for parsed but unconverted arrays
template <Kind::type>
class RawArrayBuilder;
Expand Down Expand Up @@ -305,20 +311,17 @@ class RawArrayBuilder<Kind::kArray> {
return null_bitmap_builder_.Append(count, false);
}

Status Finish(std::function<Status(BuilderPtr, std::shared_ptr<Array>*)> finish_child,
std::shared_ptr<Array>* out) {
Status Finish(std::shared_ptr<Array> child_values, std::shared_ptr<Array>* out) {
RETURN_NOT_OK(offset_builder_.Append(offset_));
auto size = length();
auto null_count = null_bitmap_builder_.false_count();
std::shared_ptr<Buffer> offsets, null_bitmap;
RETURN_NOT_OK(offset_builder_.Finish(&offsets));
RETURN_NOT_OK(null_bitmap_builder_.Finish(&null_bitmap));
std::shared_ptr<Array> 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();
}

Expand Down Expand Up @@ -404,21 +407,18 @@ class RawArrayBuilder<Kind::kObject> {
field_infos_[index].builder = builder;
}

Status Finish(std::function<Status(BuilderPtr, std::shared_ptr<Array>*)> finish_child,
Status Finish(std::vector<std::shared_ptr<ArrayData>> child_data,
std::shared_ptr<Array>* out) {
auto size = length();
auto null_count = null_bitmap_builder_.false_count();
std::shared_ptr<Buffer> null_bitmap;
RETURN_NOT_OK(null_bitmap_builder_.Finish(&null_bitmap));

DCHECK_EQ(child_data.size(), static_cast<size_t>(num_fields()));
std::vector<std::shared_ptr<Field>> fields(num_fields());
std::vector<std::shared_ptr<ArrayData>> child_data(num_fields());
for (int i = 0; i < num_fields(); ++i) {
const auto& info = field_infos_[i];
std::shared_ptr<Array> 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));
}

Expand Down Expand Up @@ -581,10 +581,6 @@ class RawBuilderSet {

Status Finish(const std::shared_ptr<Array>& scalar_values, BuilderPtr builder,
std::shared_ptr<Array>* out) {
auto finish_children = [this, &scalar_values](BuilderPtr child,
std::shared_ptr<Array>* out) {
return Finish(scalar_values, child, out);
};
switch (builder.kind) {
case Kind::kNull: {
auto length = static_cast<int64_t>(builder.index);
Expand All @@ -603,11 +599,25 @@ class RawBuilderSet {
case Kind::kNumberOrString:
return FinishScalar(scalar_values, Cast<Kind::kNumberOrString>(builder), out);

case Kind::kArray:
return Cast<Kind::kArray>(builder)->Finish(std::move(finish_children), out);
case Kind::kArray: {
auto array_builder = Cast<Kind::kArray>(builder);
auto child_builder = array_builder->value_builder();
std::shared_ptr<Array> 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<Kind::kObject>(builder)->Finish(std::move(finish_children), out);
case Kind::kObject: {
auto object_builder = Cast<Kind::kObject>(builder);
std::vector<std::shared_ptr<ArrayData>> 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<Array> 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");
Expand Down Expand Up @@ -662,13 +672,15 @@ 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);
return internal::ValidateJsonValue(
std::move(value), kMaxNestingDepth,
/*depth=*/static_cast<int>(builder_stack_.size() - 1));

case UnexpectedFieldBehavior::InferType: {
// If an unexpected field is encountered, add a NullBuilder with leading nulls.
Expand Down Expand Up @@ -833,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"));

Expand Down Expand Up @@ -887,25 +899,22 @@ 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);
}

StartNested();

RETURN_NOT_OK(StartNested());
builder_ = Cast<kind>(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(
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;
Expand All @@ -918,26 +927,26 @@ class ParseImpl : public BlockParser {
return list_builder->Append(static_cast<int32_t>(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);
}

auto struct_builder = Cast<kind>(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(
auto object, arrow::internal::ResolveSimdjsonResult(value.get_object(),
"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(
Expand All @@ -964,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<Kind::kObject>(builder_stack_.back());
field_index_ = parent->GetFieldIndex(key);
if (ARROW_PREDICT_FALSE(field_index_ == -1)) {
Expand Down Expand Up @@ -1002,10 +1011,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<size_t>(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
Expand Down Expand Up @@ -1049,6 +1064,8 @@ class ParseImpl : public BlockParser {
sj::parser parser_;
};

} // namespace

Status BlockParser::Make(MemoryPool* pool, const ParseOptions& options,
std::unique_ptr<BlockParser>* out) {
DCHECK(options.unexpected_field_behavior == UnexpectedFieldBehavior::InferType ||
Expand Down
58 changes: 58 additions & 0 deletions cpp/src/arrow/json/parser_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <gmock/gmock-matchers.h>
#include <gtest/gtest.h>

#include <sstream>
#include <string>
#include <string_view>
#include <utility>
Expand Down Expand Up @@ -355,5 +356,62 @@ 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 = 100; // hard-coded in parser.cc
ParseOptions options = ParseOptions::Defaults();
std::shared_ptr<Array> 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.
Comment on lines +401 to +402

@taepper taepper Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: in debug builds we probably did not run into stack overflows, but simdjson assertions.

simdjson::ondemand::parser also has a depth limit (default 1024), and when development checks are enabled, it will assert and thus possibly abort the program, if we consume objects deeper than that limit:

https://github.com/simdjson/simdjson/blob/7fa77b1e4ba2a21c97d97adc48e4cd8b9d1e7faa/doc/basics.md?plain=1#L3985-L4032

I did not quite understand the purpose of this limit (other than aborting debug code using the parser). I will open an issue in simdjson to find out a bit more.

Not any change request, but just for context, which we should maybe document somewhere?

for (int depth : {kMaxDepth + 1, 100'000}) {
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid,
::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 100"),
ParseFromString(options, deeply_nested_json_array(depth), &parsed));
}
}
}

} // namespace json
} // namespace arrow
Loading
Loading