Fixed upb's parser to detect missing required fields in missing map values.

PiperOrigin-RevId: 855944266

Author: haberman

upb/test/BUILD

@@ -292,6 +292,7 @@ cc_test(
         "//upb/mem",
         "//upb/message",
         "//upb/port",
+        "//upb/wire",
         "@googletest//:gtest",
         "@googletest//:gtest_main",
     ],

upb/test/test.proto

@@ -90,12 +90,17 @@ message ModelWithSubMessages {
   repeated ModelWithExtensions items = 6;
 }
 
+message ModelWithRequiredFields {
+  required int32 id = 1;
+}
+
 message ModelWithMaps {
   optional int32 id = 1;
   map<string, bytes> map_sb = 2;
   map<string, string> map_ss = 3;
   map<int32, int32> map_ii = 4;
   map<int32, ModelWithExtensions> map_im = 5;
+  map<int32, ModelWithRequiredFields> map_im_required = 6;
 }
 
 message ExtremeDefaults {

upb/test/test_generated_code.cc

@@ -12,16 +12,23 @@
 
 #include <cstddef>
 #include <cstdint>
+#include <cstdlib>
+#include <cstring>
 
 #include <gtest/gtest.h>
 #include "google/protobuf/test_messages_proto2.upb.h"
 #include "google/protobuf/test_messages_proto3.upb.h"
 #include "upb/base/status.h"
 #include "upb/base/string_view.h"
+#include "upb/mem/alloc.h"
+#include "upb/mem/arena.h"
 #include "upb/mem/arena.hpp"
 #include "upb/message/array.h"
 #include "upb/message/map.h"
+#include "upb/message/message.h"
 #include "upb/test/test.upb.h"
+#include "upb/wire/decode.h"
+#include "upb/wire/encode.h"
 
 // Must be last.
 #include "upb/port/def.inc"
@@ -910,3 +917,40 @@ TEST(GeneratedCode, Maps) {
 
   upb_Map_Set(sb, key, val, arena.ptr());
 }
+
+TEST(GeneratedCode, MapWithRequiredFields) {
+  upb::Arena arena;
+  upb_test_ModelWithMaps* msg = upb_test_ModelWithMaps_new(arena.ptr());
+
+  auto im_required =
+      _upb_test_ModelWithMaps_map_im_required_mutable_upb_map(msg, arena.ptr());
+
+  ASSERT_NE(im_required, nullptr);
+
+  upb_MessageValue key, val;
+  key.int32_val = 0;
+  val.msg_val =
+      (const upb_Message*)upb_test_ModelWithRequiredFields_new(arena.ptr());
+
+  upb_Map_Set(im_required, key, val, arena.ptr());
+
+  // Serializing fails if we are checking required fields, but succeeds if we
+  // don't.
+  size_t size;
+  char* serialized = upb_test_ModelWithMaps_serialize_ex(
+      msg, kUpb_EncodeOption_CheckRequired, arena.ptr(), &size);
+  ASSERT_EQ(serialized, nullptr);
+
+  serialized = upb_test_ModelWithMaps_serialize_ex(msg, 0, arena.ptr(), &size);
+  ASSERT_NE(serialized, nullptr);
+
+  // Likewise, parsing fails if we are checking required fields, but succeeds
+  // if we don't.
+  upb_test_ModelWithMaps* msg2 = upb_test_ModelWithMaps_parse_ex(
+      serialized, size, nullptr, kUpb_DecodeOption_CheckRequired, arena.ptr());
+  ASSERT_EQ(msg2, nullptr);
+
+  msg2 = upb_test_ModelWithMaps_parse_ex(serialized, size, nullptr, 0,
+                                         arena.ptr());
+  ASSERT_NE(msg2, nullptr);
+}

upb/wire/decode.c

@@ -538,17 +538,32 @@ static const char* _upb_Decoder_DecodeToMap(upb_Decoder* d, const char* ptr,
   // Parse map entry.
   memset(&ent, 0, sizeof(ent));
 
-  if (entry->UPB_PRIVATE(fields)[1].UPB_PRIVATE(descriptortype) ==
+  bool value_is_message =
+      entry->UPB_PRIVATE(fields)[1].UPB_PRIVATE(descriptortype) ==
           kUpb_FieldType_Message ||
       entry->UPB_PRIVATE(fields)[1].UPB_PRIVATE(descriptortype) ==
-          kUpb_FieldType_Group) {
+          kUpb_FieldType_Group;
+  const upb_MiniTable* sub_table =
+      value_is_message
+          ? upb_MiniTable_GetSubMessageTable(&entry->UPB_PRIVATE(fields)[1])
+          : NULL;
+  upb_Message* sub_msg = NULL;
+
+  if (sub_table) {
     // Create proactively to handle the case where it doesn't appear.
-    upb_Message* msg;
-    _upb_Decoder_NewSubMessage(d, &entry->UPB_PRIVATE(fields)[1], &msg);
-    ent.v.val = upb_value_ptr(msg);
+    _upb_Decoder_NewSubMessage(d, &entry->UPB_PRIVATE(fields)[1], &sub_msg);
+    ent.v.val = upb_value_ptr(sub_msg);
   }
 
   ptr = _upb_Decoder_DecodeSubMessage(d, ptr, &ent.message, field, val->size);
+
+  if (sub_msg && sub_table->UPB_PRIVATE(required_count)) {
+    // If the map entry did not contain a value on the wire, `sub_msg` is an
+    // empty message; we must check if it is missing any required fields. If the
+    // value was present, this check is redundant but harmless.
+    _upb_Decoder_CheckRequired(d, ptr, sub_msg, sub_table);
+  }
+
   if (upb_Message_HasUnknown(&ent.message)) {
     _upb_Decoder_AddMapEntryUnknown(d, msg, field, &ent.message, entry);
   } else {