diff --git a/CHANGES.txt b/CHANGES.txt index 9e6eb44885..90378673a7 100644 --- a/CHANGES.txt +++ b/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++ diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 48bfa9660f..0275e2e30b 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/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; diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt index 31bdf25ebf..bba295e7e2 100644 --- a/conformance/failure_list_csharp.txt +++ b/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 diff --git a/conformance/failure_list_php.txt b/conformance/failure_list_php.txt index d51a75dc36..667f80ca37 100644 --- a/conformance/failure_list_php.txt +++ b/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 diff --git a/conformance/failure_list_php_c.txt b/conformance/failure_list_php_c.txt index 63c7e8a024..1982029112 100644 --- a/conformance/failure_list_php_c.txt +++ b/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 diff --git a/conformance/failure_list_ruby.txt b/conformance/failure_list_ruby.txt index 4938202ad7..ea5de36609 100644 --- a/conformance/failure_list_ruby.txt +++ b/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 diff --git a/python/google/protobuf/json_format.py b/python/google/protobuf/json_format.py index c8f5602f36..965614d803 100644 --- a/python/google/protobuf/json_format.py +++ b/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'): diff --git a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc index 211b7bcbf8..2232bec568 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc +++ b/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;