Add delimited field binary/json conformance tests.

These are already supported, but this will lock down that there's no issues like the ones we hit with text-format.

This also fixes an unrelated bug in our BinaryToJsonString algorithm related to unknown group handling.   We still can't actually test binary->JSON in conformance tests for extensions due to TypeResolver's lack of support though.

PiperOrigin-RevId: 622360970
pull/16432/head
Mike Kruskal 2024-04-05 21:03:38 -07:00 committed by Copybara-Service
parent a8543e8237
commit 69d1dacf74
8 changed files with 104 additions and 10 deletions

View File

@ -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",

View File

@ -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;

View File

@ -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<uint32_t>(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<TestAllTypesProto2Editions>(
this, /*run_proto3_tests=*/false);
RunDelimitedFieldTests();
}
}
void BinaryAndJsonConformanceSuite::RunDelimitedFieldTests() {
TestAllTypesEdition2023 prototype;
SetTypeUrl(GetTypeUrl(TestAllTypesEdition2023::GetDescriptor()));
RunValidProtobufTest<TestAllTypesEdition2023>(
absl::StrCat("ValidDelimitedField.GroupLike"), REQUIRED,
group(201, field(202, WireFormatLite::WIRETYPE_VARINT, varint(99))),
R"pb(groupliketype { group_int32: 99 })pb");
RunValidProtobufTest<TestAllTypesEdition2023>(
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<TestAllTypesEdition2023>(
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<TestAllTypesEdition2023>(
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 <typename MessageType>
void BinaryAndJsonConformanceSuiteImpl<MessageType>::
ExpectParseFailureForProtoWithProtoVersion(const std::string& proto,
@ -447,7 +488,7 @@ void BinaryAndJsonConformanceSuiteImpl<MessageType>::
}
template <typename MessageType>
void BinaryAndJsonConformanceSuiteImpl<MessageType>::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<MessageType>::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 <typename MessageType>
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 <typename MessageType>
void BinaryAndJsonConformanceSuiteImpl<MessageType>::RunValidProtobufTest(
const std::string& test_name, ConformanceLevel level,
const std::string& input_protobuf,
const std::string& equivalent_text_format) {
suite_.RunValidProtobufTest<MessageType>(test_name, level, input_protobuf,
equivalent_text_format);
}
template <typename MessageType>

View File

@ -38,6 +38,20 @@ class BinaryAndJsonConformanceSuite : public ConformanceTestSuite {
type_url_ = std::string(type_url);
}
template <typename MessageType>
void RunValidBinaryProtobufTest(const std::string& test_name,
ConformanceLevel level,
const std::string& input_protobuf,
const std::string& equivalent_text_format);
template <typename MessageType>
void RunValidProtobufTest(const std::string& test_name,
ConformanceLevel level,
const std::string& input_protobuf,
const std::string& equivalent_text_format);
void RunDelimitedFieldTests();
template <typename MessageType>
friend class BinaryAndJsonConformanceSuiteImpl;

View File

@ -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

View File

@ -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

View File

@ -203,8 +203,8 @@ PROTOBUF_NOINLINE static absl::Status MakeTooDeepError() {
absl::Status UntypedMessage::Decode(io::CodedInputStream& stream,
absl::optional<int32_t> current_group) {
std::vector<int32_t> group_stack;
while (true) {
std::vector<int32_t> 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);
}

View File

@ -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) {