From e45a6d50682d5d4b204e7d76254b1585b43c6815 Mon Sep 17 00:00:00 2001
From: Eric Salo <salo@google.com>
Date: Mon, 28 Aug 2023 18:19:05 -0700
Subject: [PATCH] upb: fix a Python memory leak in ByteSize()

https://github.com/protocolbuffers/upb/issues/1243

PiperOrigin-RevId: 560873830
---
 upb/python/message.c                 |  6 ++++--
 upb/upb/util/required_fields.c       |  4 +++-
 upb/upb/util/required_fields_test.cc | 10 +++++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/upb/python/message.c b/upb/python/message.c
index e61eebb23fde..97a5470bdc64 100644
--- 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
index e0462c069fc2..3a6e037a2e21 100644
--- 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
index 99d9ea1a7277..6bdd094f89ad 100644
--- 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.