From 14bad4a5bfd8ff8fced91010edac7dd3747a7bd7 Mon Sep 17 00:00:00 2001
From: Joshua Haberman <haberman@google.com>
Date: Thu, 25 May 2023 08:10:06 -0700
Subject: [PATCH] Fixed a bug in `upb_Map_Delete()` that caused crashes in
 map.delete(k) for Ruby when string-keyed maps were in use.

Fixes: https://github.com/protocolbuffers/protobuf/issues/12580

When `upb_Map_Delete(map, k, &v)` was called for a string-keyed map, the returned value `v` contained garbage data instead of the true string length.

Since `map.delete(k)` in Ruby returns the deleted value, this was causing a garbage length to be used when allocating and copying data.

PiperOrigin-RevId: 535261609
---
 upb/collections/map.c   |  6 +++---
 upb/collections/test.cc | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/upb/collections/map.c b/upb/collections/map.c
index f076defa4a..5c3da72e19 100644
--- a/upb/collections/map.c
+++ b/upb/collections/map.c
@@ -73,9 +73,9 @@ 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 ok = _upb_Map_Delete(map, &key, map->key_size, &v);
-  if (val) val->uint64_val = v.val;
-  return ok;
+  const bool removed = _upb_Map_Delete(map, &key, map->key_size, &v);
+  if (val) _upb_map_fromvalue(v, val, map->val_size);
+  return removed;
 }
 
 bool upb_Map_Next(const upb_Map* map, upb_MessageValue* key,
diff --git a/upb/collections/test.cc b/upb/collections/test.cc
index d82c7f8a2c..7046cedf82 100644
--- a/upb/collections/test.cc
+++ b/upb/collections/test.cc
@@ -27,7 +27,9 @@
 
 #include "gtest/gtest.h"
 #include "upb/base/descriptor_constants.h"
+#include "upb/base/string_view.h"
 #include "upb/collections/array.h"
+#include "upb/collections/map.h"
 #include "upb/upb.hpp"
 
 TEST(CollectionsTest, Arrays) {
@@ -58,3 +60,20 @@ TEST(CollectionsTest, Arrays) {
   EXPECT_EQ(upb_Array_Get(array, 4).int32_val, 0);
   EXPECT_EQ(upb_Array_Get(array, 5).int32_val, 0);
 }
+
+TEST(CollectionsTest, MapDeleteRegression) {
+  upb::Arena arena;
+  upb_Map* map = upb_Map_New(arena.ptr(), kUpb_CType_Int32, kUpb_CType_String);
+  upb_MessageValue key = {.int32_val = 0};
+  const char str[] = "abcde";
+  upb_MessageValue insert_value = {.str_val = upb_StringView_FromString(str)};
+  upb_MapInsertStatus st = upb_Map_Insert(map, key, insert_value, arena.ptr());
+  EXPECT_EQ(kUpb_MapInsertStatus_Inserted, st);
+
+  upb_MessageValue delete_value;
+  bool removed = upb_Map_Delete(map, key, &delete_value);
+  EXPECT_TRUE(removed);
+
+  EXPECT_TRUE(
+      upb_StringView_IsEqual(insert_value.str_val, delete_value.str_val));
+}