commit b0eeb356b84fb7e8773c9a0dd76b354ae2e3f218
author Joshua Haberman <haberman@google.com> Wed Nov 01 10:33:11 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Nov 01 10:34:44 2023 -0700
tree 461e38accff6bb2fff6d93825417920dffe54257
parent f083530a0655338b5a1b8dfa0d46727cf1048dd9

Fixed Python memory leak in map lookup.

Previously we were allocating memory on the message's arena every time we performed a `map[key]` or `map.get(key)` operation.  This is unnecessary, as the key's data is only needed ephemerally, for the duration of the lookup, and we can therefore alias the Python object's string data instead of copying it.

This required fixing a bug in the convert.c operation.  Previously in the `arena==NULL` case, if the user passes a bytes object instead of a unicode string, the code would return a pointer to a temporary Python object that had already been freed, leading to use-after-free.  I fixed this by referencing the bytes object's data directly, and using utf8_range to verify the UTF-8.

Fixes: https://github.com/protocolbuffers/protobuf/issues/14571
PiperOrigin-RevId: 578563555

python/map.c

diff --git a/python/map.c b/python/map.c
index a1d75de..6bf12af 100644
--- a/python/map.c
+++ b/python/map.c
@@ -179,7 +179,7 @@
   const upb_FieldDef* val_f = upb_MessageDef_Field(entry_m, 1);
   upb_Arena* arena = PyUpb_Arena_Get(self->arena);
   upb_MessageValue u_key, u_val;
-  if (!PyUpb_PyToUpb(key, key_f, &u_key, arena)) return -1;
+  if (!PyUpb_PyToUpb(key, key_f, &u_key, NULL)) return -1;
 
   if (val) {
     if (!PyUpb_PyToUpb(val, val_f, &u_val, arena)) return -1;
@@ -200,9 +200,8 @@
   const upb_MessageDef* entry_m = upb_FieldDef_MessageSubDef(f);
   const upb_FieldDef* key_f = upb_MessageDef_Field(entry_m, 0);
   const upb_FieldDef* val_f = upb_MessageDef_Field(entry_m, 1);
-  upb_Arena* arena = PyUpb_Arena_Get(self->arena);
   upb_MessageValue u_key, u_val;
-  if (!PyUpb_PyToUpb(key, key_f, &u_key, arena)) return NULL;
+  if (!PyUpb_PyToUpb(key, key_f, &u_key, NULL)) return NULL;
   if (!map || !upb_Map_Get(map, u_key, &u_val)) {
     map = PyUpb_MapContainer_EnsureReified(_self);
     upb_Arena* arena = PyUpb_Arena_Get(self->arena);
@@ -256,9 +255,8 @@
   const upb_MessageDef* entry_m = upb_FieldDef_MessageSubDef(f);
   const upb_FieldDef* key_f = upb_MessageDef_Field(entry_m, 0);
   const upb_FieldDef* val_f = upb_MessageDef_Field(entry_m, 1);
-  upb_Arena* arena = PyUpb_Arena_Get(self->arena);
   upb_MessageValue u_key, u_val;
-  if (!PyUpb_PyToUpb(key, key_f, &u_key, arena)) return NULL;
+  if (!PyUpb_PyToUpb(key, key_f, &u_key, NULL)) return NULL;
   if (map && upb_Map_Get(map, u_key, &u_val)) {
     return PyUpb_UpbToPy(u_val, val_f, self->arena);
   }