GH-50351: [C++] KeyValueMetadata::DeleteMany method crashes

cpp/src/arrow/util/key_value_metadata.cc

@@ -111,20 +111,54 @@ Status KeyValueMetadata::Delete(int64_t index) {
   return Status::OK();
 }
 
+Status KeyValueMetadata::KeyValueMetaDataValidation(const std::vector<int64_t>& indices,
+                                                    const int64_t& start,
+                                                    const int64_t& stop,
+                                                    const int64_t& current_index,
+                                                    const int64_t& size) {
+  if (ARROW_PREDICT_FALSE(indices[current_index] < 0 || current_index >= size)) {
+    return Status::IndexError("KeyValueMetadata::DeleteMany: index ",
+                              indices[current_index],
+                              " is out of bounds for metadata "
+                              "of size ",
+                              size);
+  }
+
+  if (ARROW_PREDICT_FALSE(indices[current_index] == stop)) {
+    return Status::IndexError("KeyValueMetadata::DeleteMany: duplicate index ",
+                              indices[current_index], " in indices to delete");
+  }
+
+  if (ARROW_PREDICT_FALSE(start < 0 || start > size)) {
+    return Status::IndexError("KeyValueMetadata::DeleteMany: Start index ", start - 1,
+                              " out of bounds for metadata of size ", size);
+  }
+
+  if (ARROW_PREDICT_FALSE(stop < 0 || stop > size)) {
+    return Status::IndexError("KeyValueMetadata::DeleteMany: Stop index ", stop,
+                              " out of bounds for metadata of size ", size);
+  }
+
+  return Status::OK();
+}
+
 Status KeyValueMetadata::DeleteMany(std::vector<int64_t> indices) {
+  if (indices.size() == 1) {
+    return Delete(indices[0]);
+  }
   std::sort(indices.begin(), indices.end());
   const int64_t size = static_cast<int64_t>(keys_.size());
   indices.push_back(size);
 
   int64_t shift = 0;
   for (int64_t i = 0; i < static_cast<int64_t>(indices.size() - 1); ++i) {
-    ++shift;
     const auto start = indices[i] + 1;
     const auto stop = indices[i + 1];
-    DCHECK_GE(start, 0);
-    DCHECK_LE(start, size);
-    DCHECK_GE(stop, 0);
-    DCHECK_LE(stop, size);
+    ++shift;
+    Status validation_status = KeyValueMetaDataValidation(indices, start, stop, i, size);
+    if (ARROW_PREDICT_FALSE(!validation_status.ok())) {
+      return validation_status;
+    }
     for (int64_t index = start; index < stop; ++index) {
       keys_[index - shift] = std::move(keys_[index]);
       values_[index - shift] = std::move(values_[index]);

cpp/src/arrow/util/key_value_metadata.h

@@ -50,7 +50,16 @@ class ARROW_EXPORT KeyValueMetadata {
   // Note that deleting may invalidate known indices
   Status Delete(std::string_view key);
   Status Delete(int64_t index);
+
+  /// \brief Delete metadata entries at specified index in keys and values array
+  /// \param indices Vector of distinct indices identifying the entries to
+  ///        remove from the metadata.
+  /// \return Status indicating success or failure.
+
   Status DeleteMany(std::vector<int64_t> indices);
+  Status KeyValueMetaDataValidation(const std::vector<int64_t>& indices,
+                                    const int64_t& start, const int64_t& stop,
+                                    const int64_t& current_index, const int64_t& size);
   Status Set(std::string key, std::string value);
 
   void reserve(int64_t n);

cpp/src/arrow/util/key_value_metadata_test.cc

@@ -226,6 +226,85 @@ TEST(KeyValueMetadataTest, Delete) {
     ASSERT_OK(metadata.DeleteMany({}));
     ASSERT_TRUE(metadata.Equals(KeyValueMetadata({"bb", "dd", "ee"}, {"2", "4", "5"})));
   }
+  {
+    KeyValueMetadata metadata(keys, values);
+
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: index -3 is out of bounds for "
+        "metadata of size 7";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({-2, -3}));
+  }
+
+  {
+    KeyValueMetadata metadata(keys, values);
+
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::Delete: index -1 is out of bounds for metadata "
+        "of size 7";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({-1}));
+  }
+
+  {
+    KeyValueMetadata metadata(keys, values);
+
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: Stop index 8 out of bounds for "
+        "metadata of size 7";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({0, 8}));
+  }
+  {
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: duplicate index 0 in indices to "
+        "delete";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({0, 0, 5, 2}));
+  }
+  {
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: duplicate index 6 in indices to "
+        "delete";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({6, 6}));
+  }
+  {
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: Stop index 8 out of bounds for "
+        "metadata of size 7";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({8, 8, 6}));
+  }
+
+  {
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: duplicate index 2 in indices to "
+        "delete";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({0, 2, 2}));
+  }
+
+  {
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: index -1 is out of bounds for "
+        "metadata of size 7";
+
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({-1, 0}));
+  }
 }
 
 }  // namespace arrow