Fix KeyValueMetadata::DeleteMany crash with duplicate indices

Author: AdvancedUno

cpp/src/arrow/util/key_value_metadata.cc

@@ -114,6 +114,17 @@ Status KeyValueMetadata::Delete(int64_t index) {
 Status KeyValueMetadata::DeleteMany(std::vector<int64_t> indices) {
   std::sort(indices.begin(), indices.end());
   const int64_t size = static_cast<int64_t>(keys_.size());
+  if (ARROW_PREDICT_FALSE(!indices.empty() &&
+                          (indices.front() < 0 || indices.back() >= size))) {
+    const int64_t out_of_bounds = indices.front() < 0 ? indices.front() : indices.back();
+    return Status::IndexError("KeyValueMetadata::DeleteMany: index ", out_of_bounds,
+                              " is out of bounds for metadata "
+                              "of size ",
+                              size);
+  }
+  // Remove duplicate indices, otherwise the shifting logic below would move
+  // entries to incorrect (potentially negative) positions (GH-50351)
+  indices.erase(std::unique(indices.begin(), indices.end()), indices.end());
   indices.push_back(size);
 
   int64_t shift = 0;

cpp/src/arrow/util/key_value_metadata_test.cc

@@ -226,6 +226,36 @@ TEST(KeyValueMetadataTest, Delete) {
     ASSERT_OK(metadata.DeleteMany({}));
     ASSERT_TRUE(metadata.Equals(KeyValueMetadata({"bb", "dd", "ee"}, {"2", "4", "5"})));
   }
+  {
+    // GH-50351: duplicate indices must not crash and are treated as one
+    KeyValueMetadata metadata(keys, values);
+    ASSERT_OK(metadata.DeleteMany({0, 0}));
+    ASSERT_TRUE(metadata.Equals(KeyValueMetadata({"bb", "cc", "dd", "ee", "ff", "gg"},
+                                                 {"2", "3", "4", "5", "6", "7"})));
+
+    ASSERT_OK(metadata.DeleteMany({1, 3, 1, 3, 1}));
+    ASSERT_TRUE(metadata.Equals(
+        KeyValueMetadata({"bb", "dd", "ff", "gg"}, {"2", "4", "6", "7"})));
+  }
+  {
+    // GH-50351: out-of-bounds indices must error without modifying metadata
+    KeyValueMetadata metadata(keys, values);
+    std::string expected_error_message =
+        "Index error: KeyValueMetadata::DeleteMany: index 7 is out of bounds for "
+        "metadata "
+        "of size 7";
+    ASSERT_RAISES_WITH_MESSAGE(IndexError, expected_error_message,
+                               metadata.DeleteMany({7}));
+    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}));
+    ASSERT_RAISES(IndexError, metadata.DeleteMany({0, 3, 7}));
+    ASSERT_RAISES(IndexError, metadata.DeleteMany({-1, 0, 3}));
+    ASSERT_TRUE(metadata.Equals(KeyValueMetadata(keys, values)));
+  }
 }
 
 }  // namespace arrow