From 15fb3bfe65f79bbb11bc55d15c784c5e22915d2f Mon Sep 17 00:00:00 2001 From: OmBiradar Date: Wed, 1 Jul 2026 20:41:44 +0530 Subject: [PATCH 1/3] Add Out of bounds IndexError for `KeyValueMetadata::Delete` * The Delete function now catches out of bound index and does not throw a segmentation fault. Signed-off-by: OmBiradar --- cpp/src/arrow/util/key_value_metadata.cc | 14 +++++++++++++ cpp/src/arrow/util/key_value_metadata_test.cc | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 48e02c612020..15c67066c0ed 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -100,6 +100,20 @@ Result KeyValueMetadata::Get(std::string_view key) const { } Status KeyValueMetadata::Delete(int64_t index) { + if (ARROW_PREDICT_FALSE(index < 0 || std::cmp_greater_equal(index, values_.size()))) { + const size_t n = keys_.size(); + if (n == 0LL) { + return Status::IndexError("KeyValueMetadata::Delete: index ", std::to_string(index), + " is out of bounds for metadata " + "of size ", + std::to_string(n), ". Metadata is empty."); + } + return Status::IndexError("KeyValueMetadata::Delete: index ", std::to_string(index), + " is out of bounds for metadata " + "of size ", + std::to_string(n), " (valid range: [0, ", + std::to_string(n - 1), "])"); + } keys_.erase(keys_.begin() + index); values_.erase(values_.begin() + index); return Status::OK(); diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index 3cdcf9475799..26dd211a18b3 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -187,6 +187,26 @@ TEST(KeyValueMetadataTest, Delete) { ASSERT_OK(metadata.Delete(3)); ASSERT_TRUE(metadata.Equals( KeyValueMetadata({"aa", "bb", "dd", "ff", "gg"}, {"1", "2", "4", "6", "7"}))); + + std::string expected_error_message = + "Index error: KeyValueMetadata::Delete: index -1 is out of bounds for metadata " + "of size 5 (valid range: [0, 4])"; + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(-1)); + expected_error_message = + "Index error: KeyValueMetadata::Delete: index 7 is out of bounds for metadata " + "of size 5 (valid range: [0, 4])"; + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(7)); + + ASSERT_OK(metadata.Delete(4)); + ASSERT_OK(metadata.Delete(3)); + ASSERT_OK(metadata.Delete(2)); + ASSERT_OK(metadata.Delete(1)); + ASSERT_OK(metadata.Delete(0)); + + expected_error_message = + "Index error: KeyValueMetadata::Delete: index 7 is out of bounds for metadata " + "of size 0. Metadata is empty."; + ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(7)); } { KeyValueMetadata metadata(keys, values); From 926c6c4810b952c8b20e39205f4e5f89f2cb98b2 Mon Sep 17 00:00:00 2001 From: Om Biradar Date: Thu, 2 Jul 2026 14:11:20 +0530 Subject: [PATCH 2/3] Update KeyValueMetadata::Delete IndexError message Co-authored-by: Antoine Pitrou --- cpp/src/arrow/util/key_value_metadata.cc | 14 +++----------- cpp/src/arrow/util/key_value_metadata_test.cc | 6 +++--- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 15c67066c0ed..3682fae93686 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -100,19 +100,11 @@ Result KeyValueMetadata::Get(std::string_view key) const { } Status KeyValueMetadata::Delete(int64_t index) { - if (ARROW_PREDICT_FALSE(index < 0 || std::cmp_greater_equal(index, values_.size()))) { - const size_t n = keys_.size(); - if (n == 0LL) { - return Status::IndexError("KeyValueMetadata::Delete: index ", std::to_string(index), - " is out of bounds for metadata " - "of size ", - std::to_string(n), ". Metadata is empty."); - } - return Status::IndexError("KeyValueMetadata::Delete: index ", std::to_string(index), + if (ARROW_PREDICT_FALSE(index < 0 || std::cmp_greater_equal(index, keys_.size()))) { + return Status::IndexError("KeyValueMetadata::Delete: index ", index, " is out of bounds for metadata " "of size ", - std::to_string(n), " (valid range: [0, ", - std::to_string(n - 1), "])"); + keys_.size()); } keys_.erase(keys_.begin() + index); values_.erase(values_.begin() + index); diff --git a/cpp/src/arrow/util/key_value_metadata_test.cc b/cpp/src/arrow/util/key_value_metadata_test.cc index 26dd211a18b3..7c021429ac17 100644 --- a/cpp/src/arrow/util/key_value_metadata_test.cc +++ b/cpp/src/arrow/util/key_value_metadata_test.cc @@ -190,11 +190,11 @@ TEST(KeyValueMetadataTest, Delete) { std::string expected_error_message = "Index error: KeyValueMetadata::Delete: index -1 is out of bounds for metadata " - "of size 5 (valid range: [0, 4])"; + "of size 5"; ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(-1)); expected_error_message = "Index error: KeyValueMetadata::Delete: index 7 is out of bounds for metadata " - "of size 5 (valid range: [0, 4])"; + "of size 5"; ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(7)); ASSERT_OK(metadata.Delete(4)); @@ -205,7 +205,7 @@ TEST(KeyValueMetadataTest, Delete) { expected_error_message = "Index error: KeyValueMetadata::Delete: index 7 is out of bounds for metadata " - "of size 0. Metadata is empty."; + "of size 0"; ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message, metadata.Delete(7)); } { From a5f1bd9e12006110f35d1a989f976ae95ea96653 Mon Sep 17 00:00:00 2001 From: OmBiradar Date: Thu, 2 Jul 2026 16:32:14 +0530 Subject: [PATCH 3/3] Use static_cast instead of std::cmp_greater_equal Signed-off-by: OmBiradar --- cpp/src/arrow/util/key_value_metadata.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/key_value_metadata.cc b/cpp/src/arrow/util/key_value_metadata.cc index 3682fae93686..67bf02b4e1ac 100644 --- a/cpp/src/arrow/util/key_value_metadata.cc +++ b/cpp/src/arrow/util/key_value_metadata.cc @@ -100,7 +100,7 @@ Result KeyValueMetadata::Get(std::string_view key) const { } Status KeyValueMetadata::Delete(int64_t index) { - if (ARROW_PREDICT_FALSE(index < 0 || std::cmp_greater_equal(index, keys_.size()))) { + if (ARROW_PREDICT_FALSE(index < 0 || index >= static_cast(keys_.size()))) { return Status::IndexError("KeyValueMetadata::Delete: index ", index, " is out of bounds for metadata " "of size ",