Skip to content

Commit

Permalink
Fixed a bug in upb_Map_Delete() that caused crashes in map.delete(k…
Browse files Browse the repository at this point in the history
…) for Ruby when string-keyed maps were in use.

Fixes: protocolbuffers/protobuf#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
  • Loading branch information
haberman authored and copybara-github committed May 25, 2023
1 parent 73ee41c commit 14bad4a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
6 changes: 3 additions & 3 deletions upb/collections/map.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions upb/collections/test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
}

0 comments on commit 14bad4a

Please sign in to comment.