Correctly guard the out-param write on upb_Map_Delete on the condition that a value was actually deleted.

ruby/tests/common_tests.rb

@@ -416,6 +416,33 @@ def test_map_basic
     end
   end
 
+  def test_map_delete_missing
+    m = Google::Protobuf::Map.new(:string, :int32)
+    m["a"] = 1
+    assert_nil m.delete("b")
+    assert_equal 1, m.delete("a")
+    assert_nil m.delete("a")
+
+    m_str = Google::Protobuf::Map.new(:int32, :string)
+    m_str[1] = "hello"
+    assert_nil m_str.delete(2)
+    assert_equal "hello", m_str.delete(1)
+    assert_nil m_str.delete(1)
+
+    m_bytes = Google::Protobuf::Map.new(:int32, :bytes)
+    m_bytes[1] = "world"
+    assert_nil m_bytes.delete(2)
+    assert_equal "world", m_bytes.delete(1)
+    assert_nil m_bytes.delete(1)
+
+    m_msg = Google::Protobuf::Map.new(:string, :message, proto_module::TestMessage)
+    msg = proto_module::TestMessage.new(:optional_int32 => 42)
+    m_msg["a"] = msg
+    assert_nil m_msg.delete("b")
+    assert_equal msg, m_msg.delete("a")
+    assert_nil m_msg.delete("a")
+  end
+
   # This is a regression test for a bug in Map.hash. It used to return an
   # inconsistent result when there was a collision in the map (two keys mapping
   # to the same hash table entry).

upb/message/map.c

@@ -75,7 +75,7 @@ upb_MapInsertStatus upb_Map_Insert(upb_Map* map, upb_MessageValue key,
 bool upb_Map_Delete(upb_Map* map, upb_MessageValue key, upb_MessageValue* val) {
   upb_value v;
   const bool removed = _upb_Map_Delete(map, &key, map->key_size, &v);
-  if (val) _upb_map_fromvalue(v, val, map->val_size);
+  if (removed && val) _upb_map_fromvalue(v, val, map->val_size);
   return removed;
 }
 

upb/message/map_test.cc

@@ -31,3 +31,15 @@ TEST(MapTest, DeleteRegression) {
   EXPECT_TRUE(
       upb_StringView_IsEqual(insert_value.str_val, delete_value.str_val));
 }
+
+TEST(MapTest, DeleteMissingKeyStringValue) {
+  upb::Arena arena;
+  upb_Map* map = upb_Map_New(arena.ptr(), kUpb_CType_Int32, kUpb_CType_String);
+
+  upb_MessageValue key;
+  key.int32_val = 42;
+
+  upb_MessageValue delete_value;
+  bool removed = upb_Map_Delete(map, key, &delete_value);
+  EXPECT_FALSE(removed);
+}