diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index 70de9eeb7c..61a6bd43d1 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -166,6 +166,7 @@ cc_library( ":conformance_test", ":test_messages_proto2_proto_cc", ":test_messages_proto3_proto_cc", + "//conformance/test_protos:test_messages_edition2023_cc_proto", "//src/google/protobuf", "//src/google/protobuf:protobuf_lite", "//src/google/protobuf/editions:test_messages_proto2_editions_cc_proto", diff --git a/conformance/ConformanceJavaLite.java b/conformance/ConformanceJavaLite.java index abfaabb82b..d8c286d989 100644 --- a/conformance/ConformanceJavaLite.java +++ b/conformance/ConformanceJavaLite.java @@ -214,7 +214,7 @@ class ConformanceJavaLite { return TestAllTypesProto3.class; case "protobuf_test_messages.proto2.TestAllTypesProto2": return TestAllTypesProto2.class; - case "protobuf_test_messages.edition2023.TestAllTypesEdition2023": + case "protobuf_test_messages.editions.TestAllTypesEdition2023": return TestAllTypesEdition2023.class; case "protobuf_test_messages.editions.proto3.TestAllTypesProto3": return TestMessagesProto3Editions.TestAllTypesProto3.class; @@ -232,7 +232,7 @@ class ConformanceJavaLite { return TestMessagesProto3.class; case "protobuf_test_messages.proto2.TestAllTypesProto2": return TestMessagesProto2.class; - case "protobuf_test_messages.edition2023.TestAllTypesEdition2023": + case "protobuf_test_messages.editions.TestAllTypesEdition2023": return TestMessagesEdition2023.class; case "protobuf_test_messages.editions.proto3.TestAllTypesProto3": return TestMessagesProto3Editions.class; diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 40494b74bc..d473ffb822 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -26,6 +26,7 @@ #include "absl/strings/substitute.h" #include "conformance/conformance.pb.h" #include "conformance_test.h" +#include "conformance/test_protos/test_messages_edition2023.pb.h" #include "google/protobuf/editions/golden/test_messages_proto2_editions.pb.h" #include "google/protobuf/editions/golden/test_messages_proto3_editions.pb.h" #include "google/protobuf/endian.h" @@ -47,6 +48,7 @@ using google::protobuf::FieldDescriptor; using google::protobuf::internal::WireFormatLite; using google::protobuf::internal::little_endian::FromHost; using google::protobuf::util::NewTypeResolverForDescriptorPool; +using protobuf_test_messages::editions::TestAllTypesEdition2023; using protobuf_test_messages::proto2::TestAllTypesProto2; using protobuf_test_messages::proto3::TestAllTypesProto3; using TestAllTypesProto2Editions = @@ -143,6 +145,16 @@ std::string tag(int fieldnum, char wire_type) { return tag(static_cast(fieldnum), wire_type); } +std::string field(uint32_t fieldnum, char wire_type, std::string content) { + return absl::StrCat(tag(fieldnum, wire_type), content); +} + +std::string group(uint32_t fieldnum, std::string content) { + return absl::StrCat(tag(fieldnum, WireFormatLite::WIRETYPE_START_GROUP), + content, + tag(fieldnum, WireFormatLite::WIRETYPE_END_GROUP)); +} + std::string GetDefaultValue(FieldDescriptor::Type type) { switch (type) { case FieldDescriptor::TYPE_INT32: @@ -263,6 +275,7 @@ bool BinaryAndJsonConformanceSuite::ParseJsonResponse( response.json_payload(), &binary_protobuf); if (!status.ok()) { + ABSL_LOG(ERROR) << status; return false; } @@ -342,9 +355,37 @@ void BinaryAndJsonConformanceSuite::RunSuiteImpl() { this, /*run_proto3_tests=*/true); BinaryAndJsonConformanceSuiteImpl( this, /*run_proto3_tests=*/false); + RunDelimitedFieldTests(); } } +void BinaryAndJsonConformanceSuite::RunDelimitedFieldTests() { + TestAllTypesEdition2023 prototype; + SetTypeUrl(GetTypeUrl(TestAllTypesEdition2023::GetDescriptor())); + + RunValidProtobufTest( + absl::StrCat("ValidDelimitedField.GroupLike"), REQUIRED, + group(201, field(202, WireFormatLite::WIRETYPE_VARINT, varint(99))), + R"pb(groupliketype { group_int32: 99 })pb"); + + RunValidProtobufTest( + absl::StrCat("ValidDelimitedField.NotGroupLike"), REQUIRED, + group(202, field(202, WireFormatLite::WIRETYPE_VARINT, varint(99))), + R"pb(delimited_field { group_int32: 99 })pb"); + + // Note: extensions don't work with TypeResolver, which is used by + // binary->JSON tests. + RunValidBinaryProtobufTest( + absl::StrCat("ValidDelimitedExtension.GroupLike"), REQUIRED, + group(121, field(1, WireFormatLite::WIRETYPE_VARINT, varint(99))), + R"pb([protobuf_test_messages.editions.groupliketype] { c: 99 })pb"); + + RunValidBinaryProtobufTest( + absl::StrCat("ValidDelimitedExtension.NotGroupLike"), REQUIRED, + group(122, field(1, WireFormatLite::WIRETYPE_VARINT, varint(99))), + R"pb([protobuf_test_messages.editions.delimited_ext] { c: 99 })pb"); +} + template void BinaryAndJsonConformanceSuiteImpl:: ExpectParseFailureForProtoWithProtoVersion(const std::string& proto, @@ -447,7 +488,7 @@ void BinaryAndJsonConformanceSuiteImpl:: } template -void BinaryAndJsonConformanceSuiteImpl::RunValidProtobufTest( +void BinaryAndJsonConformanceSuite::RunValidBinaryProtobufTest( const std::string& test_name, ConformanceLevel level, const std::string& input_protobuf, const std::string& equivalent_text_format) { @@ -456,12 +497,34 @@ void BinaryAndJsonConformanceSuiteImpl::RunValidProtobufTest( ConformanceRequestSetting binary_to_binary( level, conformance::PROTOBUF, conformance::PROTOBUF, conformance::BINARY_TEST, prototype, test_name, input_protobuf); - suite_.RunValidInputTest(binary_to_binary, equivalent_text_format); + RunValidInputTest(binary_to_binary, equivalent_text_format); +} + +template +void BinaryAndJsonConformanceSuite::RunValidProtobufTest( + const std::string& test_name, ConformanceLevel level, + const std::string& input_protobuf, + const std::string& equivalent_text_format) { + MessageType prototype; + + ConformanceRequestSetting binary_to_binary( + level, conformance::PROTOBUF, conformance::PROTOBUF, + conformance::BINARY_TEST, prototype, test_name, input_protobuf); + RunValidInputTest(binary_to_binary, equivalent_text_format); ConformanceRequestSetting binary_to_json( level, conformance::PROTOBUF, conformance::JSON, conformance::BINARY_TEST, prototype, test_name, input_protobuf); - suite_.RunValidInputTest(binary_to_json, equivalent_text_format); + RunValidInputTest(binary_to_json, equivalent_text_format); +} + +template +void BinaryAndJsonConformanceSuiteImpl::RunValidProtobufTest( + const std::string& test_name, ConformanceLevel level, + const std::string& input_protobuf, + const std::string& equivalent_text_format) { + suite_.RunValidProtobufTest(test_name, level, input_protobuf, + equivalent_text_format); } template diff --git a/conformance/binary_json_conformance_suite.h b/conformance/binary_json_conformance_suite.h index 1bce458e2c..a510e9a6b8 100644 --- a/conformance/binary_json_conformance_suite.h +++ b/conformance/binary_json_conformance_suite.h @@ -38,6 +38,20 @@ class BinaryAndJsonConformanceSuite : public ConformanceTestSuite { type_url_ = std::string(type_url); } + template + void RunValidBinaryProtobufTest(const std::string& test_name, + ConformanceLevel level, + const std::string& input_protobuf, + const std::string& equivalent_text_format); + + template + void RunValidProtobufTest(const std::string& test_name, + ConformanceLevel level, + const std::string& input_protobuf, + const std::string& equivalent_text_format); + + void RunDelimitedFieldTests(); + template friend class BinaryAndJsonConformanceSuiteImpl; diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt index e16578c329..f5ddea6f0f 100644 --- a/conformance/failure_list_csharp.txt +++ b/conformance/failure_list_csharp.txt @@ -33,3 +33,8 @@ Recommended.Editions_Proto3.ValueRejectInfNumberValue.JsonOutput Recommended.Editions_Proto3.ValueRejectNanNumberValue.JsonOutput Required.Editions_Proto2.ProtobufInput.UnknownOrdering.ProtobufOutput Required.Editions_Proto3.ProtobufInput.UnknownOrdering.ProtobufOutput + +# TODO C# doesn't handle delimited parsing well. +Required.Editions.ProtobufInput.ValidDelimitedExtension.NotGroupLike.ProtobufOutput +Required.Editions.ProtobufInput.ValidDelimitedField.NotGroupLike.JsonOutput +Required.Editions.ProtobufInput.ValidDelimitedField.NotGroupLike.ProtobufOutput diff --git a/csharp/src/Google.Protobuf.Conformance/Program.cs b/csharp/src/Google.Protobuf.Conformance/Program.cs index ad78bdb479..e22a9618e9 100644 --- a/csharp/src/Google.Protobuf.Conformance/Program.cs +++ b/csharp/src/Google.Protobuf.Conformance/Program.cs @@ -78,10 +78,10 @@ namespace Google.Protobuf.Conformance ProtobufTestMessages.Editions.Proto2.TestAllTypesProto2.Types .MessageSetCorrectExtension2.Extensions.MessageSetExtension }; - ExtensionRegistry edition2023ExtensionRegistry = new ExtensionRegistry - { + ExtensionRegistry edition2023ExtensionRegistry = new ExtensionRegistry { ProtobufTestMessages.Editions.TestMessagesEdition2023Extensions.ExtensionInt32, - ProtobufTestMessages.Editions.TestMessagesEdition2023Extensions.DelimitedExt + ProtobufTestMessages.Editions.TestMessagesEdition2023Extensions.DelimitedExt, + ProtobufTestMessages.Editions.TestMessagesEdition2023Extensions.GroupLikeType }; IMessage message; try diff --git a/src/google/protobuf/json/internal/untyped_message.cc b/src/google/protobuf/json/internal/untyped_message.cc index 5e8e5944ee..f138f931f6 100644 --- a/src/google/protobuf/json/internal/untyped_message.cc +++ b/src/google/protobuf/json/internal/untyped_message.cc @@ -203,8 +203,8 @@ PROTOBUF_NOINLINE static absl::Status MakeTooDeepError() { absl::Status UntypedMessage::Decode(io::CodedInputStream& stream, absl::optional current_group) { + std::vector group_stack; while (true) { - std::vector group_stack; uint32_t tag = stream.ReadTag(); if (tag == 0) { return absl::OkStatus(); @@ -216,7 +216,8 @@ absl::Status UntypedMessage::Decode(io::CodedInputStream& stream, // EGROUP markers can show up as "unknown fields", so we need to handle them // before we even do field lookup. Being inside of a group behaves as if a // special field has been added to the message. - if (wire_type == WireFormatLite::WIRETYPE_END_GROUP) { + if (wire_type == WireFormatLite::WIRETYPE_END_GROUP && + group_stack.empty()) { if (!current_group.has_value()) { return MakeEndGroupWithoutGroupError(field_number); } diff --git a/src/google/protobuf/json/json_test.cc b/src/google/protobuf/json/json_test.cc index b656ec00ce..e628d2afa5 100644 --- a/src/google/protobuf/json/json_test.cc +++ b/src/google/protobuf/json/json_test.cc @@ -1274,6 +1274,16 @@ TEST_P(JsonTest, FieldOrder) { out, R"({"boolValue":true,"int64Value":"3","repeatedInt32Value":[2,2]})"); } +TEST_P(JsonTest, UnknownGroupField) { + // $ protoscope -s <<< "999: !{1: 99}" + std::string out; + absl::Status s = BinaryToJsonString(resolver_.get(), + "type.googleapis.com/proto3.TestMessage", + "\273>\010c\274>", &out); + ASSERT_OK(s); + EXPECT_EQ(out, "{}"); +} + // JSON values get special treatment when it comes to pre-existing values in // their repeated fields, when parsing through their dedicated syntax. TEST_P(JsonTest, ClearPreExistingRepeatedInJsonValues) {