upb: fix a Python memory leak in ByteSize()

https://github.com/protocolbuffers/upb/issues/1243 PiperOrigin-RevId: 560873830
protocolbuffers · Aug 29, 2023 · e45a6d5 · e45a6d5
1 parent 9274c2b
commit e45a6d5
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
diff --git a/upb/python/message.c b/upb/python/message.c
@@ -1431,9 +1431,10 @@ static PyObject* PyUpb_Message_FindInitializationErrors(PyObject* _self,
   upb_Message* msg = PyUpb_Message_GetIfReified(_self);
   const upb_MessageDef* msgdef = _PyUpb_Message_GetMsgdef(self);
   const upb_DefPool* ext_pool = upb_FileDef_Pool(upb_MessageDef_File(msgdef));
-  upb_FieldPathEntry* fields;
+  upb_FieldPathEntry* fields_base;
   PyObject* ret = PyList_New(0);
-  if (upb_util_HasUnsetRequired(msg, msgdef, ext_pool, &fields)) {
+  if (upb_util_HasUnsetRequired(msg, msgdef, ext_pool, &fields_base)) {
+    upb_FieldPathEntry* fields = fields_base;
     char* buf = NULL;
     size_t size = 0;
     assert(fields->field);
@@ -1453,6 +1454,7 @@ static PyObject* PyUpb_Message_FindInitializationErrors(PyObject* _self,
       Py_DECREF(str);
     }
     free(buf);
+    free(fields_base);
   }
   return ret;
 }

diff --git a/upb/upb/util/required_fields.c b/upb/upb/util/required_fields.c
@@ -303,11 +303,13 @@ bool upb_util_HasUnsetRequired(const upb_Message* msg, const upb_MessageDef* m,
   upb_FieldPathVector_Init(&ctx.out_fields);
   upb_util_FindUnsetRequiredInternal(&ctx, msg, m);
   free(ctx.stack.path);
-  if (fields) {
+
+  if (ctx.has_unset_required && fields) {
     upb_FieldPathVector_Reserve(&ctx, &ctx.out_fields, 1);
     ctx.out_fields.path[ctx.out_fields.size] =
         (upb_FieldPathEntry){.field = NULL};
     *fields = ctx.out_fields.path;
   }
+
   return ctx.has_unset_required;
 }
diff --git a/upb/upb/util/required_fields_test.cc b/upb/upb/util/required_fields_test.cc
@@ -30,6 +30,8 @@
 
 #include "upb/util/required_fields.h"
 
+#include <stdlib.h>
+
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include "absl/strings/string_view.h"
@@ -73,11 +75,13 @@ void CheckRequired(absl::string_view json,
   EXPECT_TRUE(upb_JsonDecode(json.data(), json.size(), test_msg, m.ptr(),
                              defpool.ptr(), 0, arena.ptr(), status.ptr()))
       << status.error_message();
-  upb_FieldPathEntry* entries;
+  upb_FieldPathEntry* entries = nullptr;
   EXPECT_EQ(!missing.empty(), upb_util_HasUnsetRequired(
                                   test_msg, m.ptr(), defpool.ptr(), &entries));
-  EXPECT_EQ(missing, PathsToText(entries));
-  free(entries);
+  if (entries) {
+    EXPECT_EQ(missing, PathsToText(entries));
+    free(entries);
+  }
 
   // Verify that we can pass a NULL pointer to entries when we don't care about
   // them.