Fix off-by-one check in required field count check.

upb has an implementation-specific maximum of 63 required fields per message. We need to verify this limit when building a MiniTable.

PiperOrigin-RevId: 850455994

Author: sbenzaquen

upb/mini_descriptor/decode.c

@@ -623,12 +623,13 @@ static void upb_MtDecoder_AssignHasbits(upb_MtDecoder* d) {
       field->presence = 0;
     }
   }
-  if (last_hasbit > kUpb_Reserved_Hasbits + 63) {
+  if (last_hasbit >= kUpb_Reserved_Hasbits + 63) {
     upb_MdDecoder_ErrorJmp(&d->base, "Too many required fields");
   }
 
   d->table.UPB_PRIVATE(required_count) =
       last_hasbit - (kUpb_Reserved_Hasbits - 1);
+  UPB_ASSERT(d->table.UPB_PRIVATE(required_count) < 64);
 
   // Next assign non-required hasbit fields.
   for (int i = 0; i < n; i++) {

upb/reflection/BUILD

@@ -191,6 +191,8 @@ cc_test(
         "@abseil-cpp//absl/log:check",
         "@abseil-cpp//absl/status",
         "@abseil-cpp//absl/status:statusor",
+        "@abseil-cpp//absl/strings",
+        "@abseil-cpp//absl/strings:str_format",
         "@googletest//:gtest",
         "@googletest//:gtest_main",
     ],

upb/reflection/reflection_test.cc

@@ -4,10 +4,13 @@
 
 #include "google/protobuf/descriptor.pb.h"
 #include "google/protobuf/descriptor.upb.h"
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "absl/log/absl_check.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
+#include "absl/strings/str_format.h"
+#include "absl/strings/str_join.h"
 #include "upb/base/status.hpp"
 #include "upb/mem/arena.hpp"
 #include "upb/reflection/def.hpp"
@@ -16,6 +19,8 @@
 namespace upb_test {
 namespace {
 
+using testing::HasSubstr;
+
 google_protobuf_FileDescriptorProto* ToUpbDescriptorSet(
     const google::protobuf::FileDescriptorProto& proto, upb::Arena& arena) {
   std::string serialized;
@@ -128,5 +133,37 @@ TEST(ReflectionTest, ImplicitPresenceWithNonZeroDefaultEnum) {
             "with a non-zero default (FooEnum)");
 }
 
+TEST(ReflectionTest, TooManyRequiredFieldsFailGracefully) {
+  const auto make_desc = [](int n) {
+    std::vector<std::string> fields;
+    for (int i = 1; i <= n; ++i) {
+      fields.push_back(absl::StrFormat(R"pb(
+                                         field {
+                                           name: "f%d"
+                                           number: %d
+                                           type: TYPE_INT32
+                                           label: LABEL_REQUIRED
+                                         })pb",
+                                       i, i));
+    }
+    return absl::StrFormat(
+        R"pb(
+          syntax: "proto2"
+          name: "F"
+          message_type { name: "FooMessage" %s }
+        )pb",
+        absl::StrJoin(fields, "\n"));
+  };
+  // 63 required fields is ok.
+  upb::DefPool good = LoadDescriptorProto(make_desc(63)).value();
+  auto m = good.FindMessageByName("FooMessage");
+  auto f = m.FindFieldByNumber(63);
+  EXPECT_STREQ(f.full_name(), "FooMessage.f63");
+
+  // 64 is too much.
+  EXPECT_THAT(LoadDescriptorProto(make_desc(64)).status().message(),
+              HasSubstr("Too many required fields"));
+}
+
 }  // namespace
 }  // namespace upb_test