Merge pull request #8502 from haberman/sync-stage

Integrate from Piper for C++, Java, and Python

CHANGES.txt

@@ -9,6 +9,14 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript)
   Kotlin
   * Restrict extension setter and getter operators to non-nullable T.
 
+  Python
+  * Make JSON parsing match C++ and Java when multiple fields from the same
+    oneof are present and all but one is null.
+
+  Conformance Tests
+  * Added a conformance test for the case of multiple fields from the same
+    oneof.
+
 3.16.0 (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript)
 
   C++

conformance/binary_json_conformance_suite.cc

@@ -2329,6 +2329,12 @@ void BinaryAndJsonConformanceSuite::RunJsonTestsForNonRepeatedTypes() {
   ExpectParseFailureForJson(
       "OneofFieldDuplicate", REQUIRED,
       R"({"oneofUint32": 1, "oneofString": "test"})");
+  RunValidJsonTest("OneofFieldNullFirst", REQUIRED,
+                   R"({"oneofUint32": null, "oneofString": "test"})",
+                   "oneof_string: \"test\"");
+  RunValidJsonTest("OneofFieldNullSecond", REQUIRED,
+                   R"({"oneofString": "test", "oneofUint32": null})",
+                   "oneof_string: \"test\"");
   // Ensure zero values for oneof make it out/backs.
   TestAllTypesProto3 messageProto3;
   TestAllTypesProto2 messageProto2;

conformance/failure_list_csharp.txt

@@ -1,3 +1,7 @@
+Recommended.Proto2.JsonInput.FieldNameExtension.Validator
 Recommended.Proto3.JsonInput.BytesFieldBase64Url.JsonOutput
 Recommended.Proto3.JsonInput.BytesFieldBase64Url.ProtobufOutput
-Recommended.Proto2.JsonInput.FieldNameExtension.Validator
+Required.Proto3.JsonInput.OneofFieldNullFirst.JsonOutput
+Required.Proto3.JsonInput.OneofFieldNullFirst.ProtobufOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput

conformance/failure_list_php.txt

@@ -13,6 +13,8 @@ Required.Proto3.JsonInput.FloatFieldTooSmall
 Required.Proto3.JsonInput.Int32FieldNotInteger
 Required.Proto3.JsonInput.Int64FieldNotInteger
 Required.Proto3.JsonInput.OneofFieldDuplicate
+Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
 Required.Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt
 Required.Proto3.JsonInput.RepeatedListValue.JsonOutput
 Required.Proto3.JsonInput.RepeatedListValue.ProtobufOutput

conformance/failure_list_php_c.txt

@@ -1,2 +1,4 @@
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
 Required.Proto2.JsonInput.StoresDefaultPrimitive.Validator
+Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput

conformance/failure_list_ruby.txt

@@ -56,3 +56,5 @@ Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOu
 Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.UnpackedOutput.ProtobufOutput
 Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput
 Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.UnpackedOutput.ProtobufOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
+Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput

python/google/protobuf/json_format.py

@@ -531,8 +531,9 @@ class _Parser(object):
                            '"{1}" fields.'.format(
                                message.DESCRIPTOR.full_name, name))
         names.append(name)
+        value = js[name]
         # Check no other oneof field is parsed.
-        if field.containing_oneof is not None:
+        if field.containing_oneof is not None and value is not None:
           oneof_name = field.containing_oneof.name
           if oneof_name in names:
             raise ParseError('Message type "{0}" should not have multiple '
@@ -540,7 +541,6 @@ class _Parser(object):
                                  message.DESCRIPTOR.full_name, oneof_name))
           names.append(oneof_name)
 
-        value = js[name]
         if value is None:
           if (field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE
               and field.message_type.full_name == 'google.protobuf.Value'):

src/google/protobuf/util/internal/protostream_objectwriter_test.cc

@@ -699,6 +699,61 @@ TEST_P(ProtoStreamObjectWriterTest, ImplicitMessageList) {
   CheckOutput(expected);
 }
 
+TEST_P(ProtoStreamObjectWriterTest, DisableImplicitMessageList) {
+  options_.disable_implicit_message_list = true;
+  options_.suppress_implicit_message_list_error = true;
+  ResetProtoWriter();
+
+  Book expected;
+  // The repeated friend field of the author is empty.
+  expected.mutable_author();
+
+  EXPECT_CALL(listener_, InvalidValue(_, _, _)).Times(0);
+
+  ow_->StartObject("")
+      ->StartObject("author")
+      ->StartObject("friend")
+      ->RenderString("name", "first")
+      ->EndObject()
+      ->StartObject("friend")
+      ->RenderString("name", "second")
+      ->EndObject()
+      ->EndObject()
+      ->EndObject();
+  CheckOutput(expected);
+}
+
+TEST_P(ProtoStreamObjectWriterTest,
+       DisableImplicitMessageListWithoutErrorSuppressed) {
+  options_.disable_implicit_message_list = true;
+  ResetProtoWriter();
+
+  Book expected;
+  // The repeated friend field of the author is empty.
+  expected.mutable_author();
+  EXPECT_CALL(
+      listener_,
+      InvalidValue(
+          _, StringPiece("friend"),
+          StringPiece(
+              "Starting an object in a repeated field but the parent object "
+              "is not a list")))
+      .With(Args<0>(HasObjectLocation("author")))
+      .Times(2);
+
+  ow_->StartObject("")
+      ->StartObject("author")
+      ->StartObject("friend")
+      ->RenderString("name", "first")
+      ->EndObject()
+      ->StartObject("friend")
+      ->RenderString("name", "second")
+      ->EndObject()
+      ->EndObject()
+      ->EndObject();
+  CheckOutput(expected);
+}
+
 TEST_P(ProtoStreamObjectWriterTest,
        LastWriteWinsOnNonRepeatedMessageFieldWithDuplicates) {
   Book expected;