From 960f46e3413984aea272141fb41ead298abe9556 Mon Sep 17 00:00:00 2001 From: Alexander Taepper Date: Thu, 30 Jul 2026 08:01:37 +0200 Subject: [PATCH 1/4] do not use throwing api --- cpp/src/arrow/json/from_string.cc | 55 ++++++++++++++++------ cpp/src/arrow/json/json_writer_internal.cc | 8 +++- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index 9cb14a92a647..5694fbde4c53 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -166,6 +166,16 @@ Result GetJsonResult( return typed_value; } +// Result because peeking the nonRootScalar can fail (parsed lazily) +Result IsJsonNull(sj::value& value) { + bool is_null; + if (auto error_code = value.is_null().get(is_null); error_code != simdjson::SUCCESS) { + return Status::Invalid("Error checking for JSON null: ", + simdjson::error_message(error_code)); + } + return is_null; +} + class JSONConverter { public: virtual ~JSONConverter() = default; @@ -262,7 +272,8 @@ class BooleanConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return AppendNull(); } int64_t int_value; @@ -415,7 +426,8 @@ class IntegerConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } c_type value; @@ -442,7 +454,8 @@ class FloatConverter final : public ConcreteConverterMakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } c_type value; @@ -472,7 +485,8 @@ class DecimalConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } ARROW_ASSIGN_OR_RAISE(auto string_value, GetJsonAs(json_obj)); @@ -514,7 +528,8 @@ class TimestampConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } int64_t value; @@ -548,7 +563,8 @@ class DayTimeIntervalConverter final } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } @@ -581,7 +597,8 @@ class MonthDayNanoIntervalConverter final } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } @@ -620,7 +637,8 @@ class StringConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } @@ -648,7 +666,8 @@ class FixedSizeBinaryConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs(json_obj)); @@ -691,7 +710,8 @@ class VarLengthListLikeConverter final } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); @@ -730,7 +750,8 @@ class MapConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } RETURN_NOT_OK(builder_->Append()); @@ -746,7 +767,8 @@ class MapConverter final : public ConcreteConverter { RETURN_NOT_OK(ProcessJsonArrayElements<2>( json_pair_array, "key-item pair", {[this](sj::value& key) { - if (key.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool key_is_null, IsJsonNull(key)); + if (key_is_null) { return Status::Invalid("null key is invalid"); } return key_converter_->AppendValue(key); @@ -781,7 +803,8 @@ class FixedSizeListConverter final : public ConcreteConverterAppendNull(); } RETURN_NOT_OK(builder_->Append()); @@ -829,7 +852,8 @@ class StructConverter final : public ConcreteConverter { // or an object mapping struct names to values (omitted struct members // are mapped to null). Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } sj::array array; @@ -937,7 +961,8 @@ class UnionConverter final : public ConcreteConverter { // Append a JSON value that must be a 2-long array, containing the type_id // and value of the UnionArray's slot. Status AppendValue(sj::value& json_obj) override { - if (json_obj.is_null()) { + ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + if (is_null) { return this->AppendNull(); } diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 49038f26b073..865b07f3a7eb 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -102,7 +102,13 @@ void JsonWriter::Null() { needs_comma_ = true; } -std::string_view JsonWriter::GetString() const { return builder_.view().value(); } +std::string_view JsonWriter::GetString() const { + std::string_view view; + if (builder_.view().get(view) != simdjson::SUCCESS) { + return {}; + } + return view; +} void JsonWriter::Clear() { builder_.clear(); From b8eaec8224a552cbba45dd4df499f972a8d948f2 Mon Sep 17 00:00:00 2001 From: Alexander Taepper Date: Thu, 30 Jul 2026 10:31:11 +0200 Subject: [PATCH 2/4] do not ignore errors in `JsonWriter::GetString()` --- cpp/src/arrow/extension/fixed_shape_tensor.cc | 5 ++- cpp/src/arrow/extension/opaque.cc | 5 ++- .../arrow/extension/variable_shape_tensor.cc | 5 ++- cpp/src/arrow/integration/json_integration.cc | 3 +- .../integration/json_integration_test.cc | 4 +- cpp/src/arrow/json/json_writer_internal.cc | 7 ++-- cpp/src/arrow/json/json_writer_internal.h | 3 +- .../arrow/json/json_writer_internal_test.cc | 41 ++++++++++++++----- 8 files changed, 53 insertions(+), 20 deletions(-) diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc b/cpp/src/arrow/extension/fixed_shape_tensor.cc index 697bab92b339..cd3d783479d6 100644 --- a/cpp/src/arrow/extension/fixed_shape_tensor.cc +++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc @@ -104,7 +104,10 @@ std::string FixedShapeTensorType::Serialize() const { writer.EndObject(); - return std::string(writer.GetString()); + Result json = writer.GetString(); + // can only fail in OutOfMemory scenarios + ARROW_CHECK_OK(json.status()); + return std::string(*json); } Result> FixedShapeTensorType::Deserialize( diff --git a/cpp/src/arrow/extension/opaque.cc b/cpp/src/arrow/extension/opaque.cc index 5fe904fce69c..b1ad48fdba97 100644 --- a/cpp/src/arrow/extension/opaque.cc +++ b/cpp/src/arrow/extension/opaque.cc @@ -57,7 +57,10 @@ std::string OpaqueType::Serialize() const { writer.EndObject(); - return std::string(writer.GetString()); + Result json = writer.GetString(); + // can only fail in OutOfMemory scenarios + ARROW_CHECK_OK(json.status()); + return std::string(*json); } Result> OpaqueType::Deserialize( diff --git a/cpp/src/arrow/extension/variable_shape_tensor.cc b/cpp/src/arrow/extension/variable_shape_tensor.cc index a67bd6dea88c..40171f909a9d 100644 --- a/cpp/src/arrow/extension/variable_shape_tensor.cc +++ b/cpp/src/arrow/extension/variable_shape_tensor.cc @@ -120,7 +120,10 @@ std::string VariableShapeTensorType::Serialize() const { writer.EndObject(); - return std::string(writer.GetString()); + Result json = writer.GetString(); + // can only fail in OutOfMemory scenarios + ARROW_CHECK_OK(json.status()); + return std::string(*json); } Result> VariableShapeTensorType::Deserialize( diff --git a/cpp/src/arrow/integration/json_integration.cc b/cpp/src/arrow/integration/json_integration.cc index d7c0fa59e4e2..f4c333366482 100644 --- a/cpp/src/arrow/integration/json_integration.cc +++ b/cpp/src/arrow/integration/json_integration.cc @@ -78,7 +78,8 @@ class IntegrationJsonWriter::Impl { writer_.EndArray(); // Record batches writer_.EndObject(); - return std::string(writer_.GetString()); + ARROW_ASSIGN_OR_RAISE(std::string_view json, writer_.GetString()); + return std::string(json); } Status WriteRecordBatch(const RecordBatch& batch) { diff --git a/cpp/src/arrow/integration/json_integration_test.cc b/cpp/src/arrow/integration/json_integration_test.cc index e7a55d13e0d2..477ef932d24f 100644 --- a/cpp/src/arrow/integration/json_integration_test.cc +++ b/cpp/src/arrow/integration/json_integration_test.cc @@ -732,7 +732,7 @@ void TestSchemaRoundTrip(const std::shared_ptr& schema) { ASSERT_OK(json::WriteSchema(*schema, mapper, &writer)); writer.EndObject(); - std::string json_schema(writer.GetString()); + ASSERT_OK_AND_ASSIGN(std::string_view json_schema, writer.GetString()); rj::Document d; // Pass explicit size to avoid ASAN issues with @@ -752,7 +752,7 @@ void TestArrayRoundTrip(const Array& array) { ASSERT_OK(json::WriteArray(name, array, &writer)); - std::string array_as_json(writer.GetString()); + ASSERT_OK_AND_ASSIGN(std::string_view array_as_json, writer.GetString()); rj::Document d; // Pass explicit size to avoid ASAN issues with diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 865b07f3a7eb..d18260fb7e30 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -102,10 +102,11 @@ void JsonWriter::Null() { needs_comma_ = true; } -std::string_view JsonWriter::GetString() const { +Result JsonWriter::GetString() const { std::string_view view; - if (builder_.view().get(view) != simdjson::SUCCESS) { - return {}; + if (auto error = builder_.view().get(view); error != simdjson::SUCCESS) { + return Status::OutOfMemory( + "OutOfMemory when allocating buffer to serialize json to string"); } return view; } diff --git a/cpp/src/arrow/json/json_writer_internal.h b/cpp/src/arrow/json/json_writer_internal.h index 4173cb4c512c..e6043bd7e5ac 100644 --- a/cpp/src/arrow/json/json_writer_internal.h +++ b/cpp/src/arrow/json/json_writer_internal.h @@ -22,6 +22,7 @@ #include #include +#include "arrow/result.h" #include "arrow/util/visibility.h" namespace arrow::json { @@ -55,7 +56,7 @@ class ARROW_EXPORT JsonWriter { void StringField(std::string_view key, std::string_view value); void BoolField(std::string_view key, bool value); - std::string_view GetString() const; + Result GetString() const; void Clear(); diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/json/json_writer_internal_test.cc index 01e6d7ecb8c1..abdf9be7af09 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/json/json_writer_internal_test.cc @@ -18,6 +18,7 @@ #include #include "arrow/json/json_writer_internal.h" +#include "arrow/testing/gtest_util.h" namespace arrow::json { @@ -31,7 +32,9 @@ TEST(JsonWriter, SimpleObject) { writer.String("hello"); writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"a":42,"b":"hello"})"); } TEST(JsonWriter, Array) { @@ -43,7 +46,9 @@ TEST(JsonWriter, Array) { writer.Int(3); writer.EndArray(); - EXPECT_EQ(writer.GetString(), "[1,2,3]"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, "[1,2,3]"); } TEST(JsonWriter, NestedObject) { @@ -59,7 +64,9 @@ TEST(JsonWriter, NestedObject) { writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"child":{"x":true}})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"child":{"x":true}})"); } TEST(JsonWriter, NullValue) { @@ -70,7 +77,9 @@ TEST(JsonWriter, NullValue) { writer.Null(); writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"value":null})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"value":null})"); } TEST(JsonWriter, DoubleValue) { @@ -81,7 +90,9 @@ TEST(JsonWriter, DoubleValue) { writer.Double(3.14); writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"pi":3.14})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"pi":3.14})"); } TEST(JsonWriter, UnsignedValues) { @@ -94,7 +105,9 @@ TEST(JsonWriter, UnsignedValues) { writer.Uint64(1234567890123ULL); writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"u32":42,"u64":1234567890123})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"u32":42,"u64":1234567890123})"); } TEST(JsonWriter, Int64Value) { @@ -105,7 +118,9 @@ TEST(JsonWriter, Int64Value) { writer.Int64(-1234567890123LL); writer.EndObject(); - EXPECT_EQ(writer.GetString(), R"({"i64":-1234567890123})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"i64":-1234567890123})"); } TEST(JsonWriter, Clear) { @@ -122,7 +137,9 @@ TEST(JsonWriter, Clear) { writer.Int(5); writer.EndArray(); - EXPECT_EQ(writer.GetString(), "[5]"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, "[5]"); } TEST(JsonWriter, RawValue) { @@ -133,7 +150,9 @@ TEST(JsonWriter, RawValue) { writer.RawValue("123.456"); writer.EndObject(); - ASSERT_EQ(writer.GetString(), R"({"number":123.456})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"number":123.456})"); } TEST(JsonWriter, StringWithExplicitLength) { @@ -146,7 +165,9 @@ TEST(JsonWriter, StringWithExplicitLength) { writer.String(std::string_view(value, 3)); writer.EndObject(); - ASSERT_EQ(writer.GetString(), R"({"value":"abc"})"); + ASSERT_OK_AND_ASSIGN(std::string_view json, writer.GetString()); + + EXPECT_EQ(json, R"({"value":"abc"})"); } } // namespace arrow::json From efd505c11ef9d92552c4db572ee304eeecc5a7bf Mon Sep 17 00:00:00 2001 From: Alexander Taepper Date: Thu, 30 Jul 2026 11:03:52 +0200 Subject: [PATCH 3/4] also fix GetString() users in parquet/encryption --- cpp/src/parquet/encryption/file_system_key_material_store.cc | 3 ++- cpp/src/parquet/encryption/key_material.cc | 5 ++++- cpp/src/parquet/encryption/key_metadata.cc | 5 ++++- cpp/src/parquet/encryption/local_wrap_kms_client.cc | 5 ++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc b/cpp/src/parquet/encryption/file_system_key_material_store.cc index e5b215a84609..ece14b9664dc 100644 --- a/cpp/src/parquet/encryption/file_system_key_material_store.cc +++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc @@ -87,7 +87,8 @@ std::string FileSystemKeyMaterialStore::BuildKeyMaterialMapJson() { writer.StringField(it.first, it.second); } writer.EndObject(); - return std::string(writer.GetString()); + PARQUET_ASSIGN_OR_THROW(std::string_view json, writer.GetString()); + return std::string(json); } void FileSystemKeyMaterialStore::SaveMaterial() { diff --git a/cpp/src/parquet/encryption/key_material.cc b/cpp/src/parquet/encryption/key_material.cc index 67f838c1df1b..305ad5969852 100644 --- a/cpp/src/parquet/encryption/key_material.cc +++ b/cpp/src/parquet/encryption/key_material.cc @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include + #include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" @@ -163,7 +165,8 @@ std::string KeyMaterial::SerializeToJson( } json_writer.EndObject(); - return std::string(json_writer.GetString()); + PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString()); + return std::string(json); } } // namespace parquet::encryption diff --git a/cpp/src/parquet/encryption/key_metadata.cc b/cpp/src/parquet/encryption/key_metadata.cc index ed6c62955cf2..94253c87e3e8 100644 --- a/cpp/src/parquet/encryption/key_metadata.cc +++ b/cpp/src/parquet/encryption/key_metadata.cc @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include + #include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" @@ -84,7 +86,8 @@ std::string KeyMetadata::CreateSerializedForExternalMaterial( json_writer.EndObject(); - return std::string(json_writer.GetString()); + PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString()); + return std::string(json); } } // namespace parquet::encryption diff --git a/cpp/src/parquet/encryption/local_wrap_kms_client.cc b/cpp/src/parquet/encryption/local_wrap_kms_client.cc index b2a6872af5bd..dcb6c49836e0 100644 --- a/cpp/src/parquet/encryption/local_wrap_kms_client.cc +++ b/cpp/src/parquet/encryption/local_wrap_kms_client.cc @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#include + #include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" #include "arrow/util/secure_string.h" @@ -50,7 +52,8 @@ std::string LocalWrapKmsClient::LocalKeyWrap::CreateSerialized( json_writer.EndObject(); - return std::string(json_writer.GetString()); + PARQUET_ASSIGN_OR_THROW(std::string_view json, json_writer.GetString()); + return std::string(json); } LocalWrapKmsClient::LocalKeyWrap LocalWrapKmsClient::LocalKeyWrap::Parse( From b83c2c28e96237b1cee0068fcd9902dc86be8515 Mon Sep 17 00:00:00 2001 From: Alexander Taepper Date: Thu, 30 Jul 2026 11:14:01 +0200 Subject: [PATCH 4/4] validate OUT_OF_CAPACITY error_code --- cpp/src/arrow/json/json_writer_internal.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index d18260fb7e30..446c7f06f4bf 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -105,8 +105,12 @@ void JsonWriter::Null() { Result JsonWriter::GetString() const { std::string_view view; if (auto error = builder_.view().get(view); error != simdjson::SUCCESS) { - return Status::OutOfMemory( - "OutOfMemory when allocating buffer to serialize json to string"); + if (error == simdjson::OUT_OF_CAPACITY) { + return Status::OutOfMemory( + "OutOfMemory when allocating buffer to serialize json to string"); + } + return Status::Invalid("Failed to retrieve json from string builder: ", + simdjson::error_message(error)); } return view; }