Correctly discard unknown enum name strings in an array when parsing a repeated field.

This was a regression in the new parser.

PiperOrigin-RevId: 489292231

conformance/failure_list_cpp.txt

@@ -18,7 +18,6 @@ Recommended.Proto3.JsonInput.FieldNameDuplicateDifferentCasing1
 Recommended.Proto3.JsonInput.FieldNameDuplicateDifferentCasing2
 Recommended.Proto3.JsonInput.FieldNameNotQuoted
 Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
 Recommended.Proto3.JsonInput.MapFieldValueIsNull
 Recommended.Proto3.JsonInput.RepeatedFieldMessageElementIsNull
 Recommended.Proto3.JsonInput.RepeatedFieldPrimitiveElementIsNull

src/google/protobuf/json/internal/descriptor_traits.h

@@ -267,6 +267,10 @@ struct Proto2Descriptor {
 
   static bool IsOptional(Field f) { return f->has_presence(); }
 
+  static bool IsImplicitPresence(Field f) {
+    return !f->is_repeated() && !f->has_presence();
+  }
+
   static bool IsExtension(Field f) { return f->is_extension(); }
 
   static bool IsOneof(Field f) { return f->containing_oneof() != nullptr; }
@@ -449,8 +453,20 @@ struct Proto3Type {
   }
 
   static bool IsOptional(Field f) {
+    // Implicit presence requires this weird check: in proto3, everything is
+    // implicit presence, except for things that are members of oneofs,
+    // which is how proto3 optional is represented.
+    if (f->parent().proto().syntax() == google::protobuf::SYNTAX_PROTO3) {
+      return f->proto().oneof_index() != 0;
+    }
+
     return f->proto().cardinality() ==
-           google::protobuf::Field::CARDINALITY_OPTIONAL;
+               google::protobuf::Field::CARDINALITY_OPTIONAL ||
+           google::protobuf::Field::CARDINALITY_REQUIRED;
+  }
+
+  static bool IsImplicitPresence(Field f) {
+    return !IsRepeated(f) && !IsOptional(f);
   }
 
   static bool IsExtension(Field f) { return false; }

src/google/protobuf/json/internal/parser.cc

@@ -525,7 +525,7 @@ absl::Status ParseSingular(JsonLexer& lex, Field<Traits> field,
       absl::StatusOr<absl::optional<int32_t>> x = ParseEnum<Traits>(lex, field);
       RETURN_IF_ERROR(x.status());
 
-      if (x->has_value() || !Traits::IsOptional(field)) {
+      if (x->has_value() || Traits::IsImplicitPresence(field)) {
         Traits::SetEnum(field, msg, x->value_or(0));
       }
       break;

src/google/protobuf/json/json_test.cc

@@ -273,20 +273,20 @@ TEST_P(JsonTest, TestDefaultValues) {
           R"("defaultForeignEnum":"FOREIGN_BAR","defaultImportEnum":"IMPORT_BAR",)"
           R"("defaultStringPiece":"abc","defaultCord":"123"})"));
 
-    EXPECT_THAT(
-        ToJson(protobuf_unittest::TestExtremeDefaultValues(), options),
-        IsOkAndHolds(
-            R"({"escapedBytes":"XDAwMFwwMDFcMDA3XDAxMFwwMTRcblxyXHRcMDEzXFxcJ1wiXDM3Ng==")"
-            R"(,"largeUint32":4294967295,"largeUint64":"18446744073709551615",)"
-            R"("smallInt32":-2147483647,"smallInt64":"-9223372036854775807",)"
-            R"("utf8String":"ሴ","zeroFloat":0,"oneFloat":1,"smallFloat":1.5,)"
-            R"("negativeOneFloat":-1,"negativeFloat":-1.5,"largeFloat":2e+08,)"
-            R"("smallNegativeFloat":-8e-28,"infDouble":0,"negInfDouble":0,)"
-            R"("nanDouble":0,"infFloat":0,"negInfFloat":0,"nanFloat":0,)"
-            R"("cppTrigraph":"? ? ?? ?? ??? ??/ ??-","reallySmallInt32":-2147483648)"
-            R"(,"reallySmallInt64":"-9223372036854775808","stringWithZero":"hel\u0000lo")"
-            R"(,"bytesWithZero":"d29yXDAwMGxk","stringPieceWithZero":"ab\u0000c")"
-            R"(,"cordWithZero":"12\u00003","replacementString":"${unknown}"})"));
+  EXPECT_THAT(
+      ToJson(protobuf_unittest::TestExtremeDefaultValues(), options),
+      IsOkAndHolds(
+          R"({"escapedBytes":"XDAwMFwwMDFcMDA3XDAxMFwwMTRcblxyXHRcMDEzXFxcJ1wiXDM3Ng==")"
+          R"(,"largeUint32":4294967295,"largeUint64":"18446744073709551615",)"
+          R"("smallInt32":-2147483647,"smallInt64":"-9223372036854775807",)"
+          R"("utf8String":"ሴ","zeroFloat":0,"oneFloat":1,"smallFloat":1.5,)"
+          R"("negativeOneFloat":-1,"negativeFloat":-1.5,"largeFloat":2e+08,)"
+          R"("smallNegativeFloat":-8e-28,"infDouble":0,"negInfDouble":0,)"
+          R"("nanDouble":0,"infFloat":0,"negInfFloat":0,"nanFloat":0,)"
+          R"("cppTrigraph":"? ? ?? ?? ??? ??/ ??-","reallySmallInt32":-2147483648)"
+          R"(,"reallySmallInt64":"-9223372036854775808","stringWithZero":"hel\u0000lo")"
+          R"(,"bytesWithZero":"d29yXDAwMGxk","stringPieceWithZero":"ab\u0000c")"
+          R"(,"cordWithZero":"12\u00003","replacementString":"${unknown}"})"));
 }
 
 TEST_P(JsonTest, TestPreserveProtoFieldNames) {
@@ -972,6 +972,21 @@ TEST_P(JsonTest, TestParsingUnknownEnumsProto3FromArray) {
               StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
+TEST_P(JsonTest, TestParsingRepeatedUnknownEnums) {
+  absl::string_view input = R"json({
+    "repeated_enum_value": ["FOO", "BAZ", "BAR"]
+  })json";
+
+  EXPECT_THAT(ToProto<TestMessage>(input),
+              StatusIs(absl::StatusCode::kInvalidArgument));
+
+  ParseOptions options;
+  options.ignore_unknown_fields = true;
+  auto m = ToProto<TestMessage>(input, options);
+  ASSERT_OK(m);
+  EXPECT_THAT(m->repeated_enum_value(), ElementsAre(proto3::FOO, proto3::BAR));
+}
+
 TEST_P(JsonTest, TestParsingEnumCaseSensitive) {
   TestMessage m;
   m.set_enum_value(proto3::FOO);
@@ -1278,9 +1293,8 @@ TEST_P(JsonTest, FieldOrder) {
       resolver_.get(), "type.googleapis.com/proto3.TestMessage",
       "\x18\x03\xb0\x01\x02\x08\x01\xb0\x01\x02", &out);
   ASSERT_OK(s);
-    EXPECT_EQ(
-        out,
-        R"({"boolValue":true,"int64Value":"3","repeatedInt32Value":[2,2]})");
+  EXPECT_EQ(
+      out, R"({"boolValue":true,"int64Value":"3","repeatedInt32Value":[2,2]})");
 }
 
 // JSON values get special treatment when it comes to pre-existing values in