diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 50a95b419974..33185a00fc5e 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -88,6 +88,21 @@ ProtoValidationException::ProtoValidationException(const std::string& validation ENVOY_LOG_MISC(debug, "Proto validation error; throwing {}", what()); } +void MessageUtil::checkUnknownFields(const Protobuf::Message& message, + ProtobufMessage::ValidationVisitor& validation_visitor) { + const auto& unknown_fields = message.GetReflection()->GetUnknownFields(message); + // If there are no unknown fields, we're done here. + if (unknown_fields.empty()) { + return; + } + std::string error_msg; + for (int n = 0; n < unknown_fields.field_count(); ++n) { + error_msg += absl::StrCat(n > 0 ? ", " : "", unknown_fields.field(n).number()); + } + validation_visitor.onUnknownField("type " + message.GetTypeName() + " with unknown field set {" + + error_msg + "}"); +} + void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message, ProtobufMessage::ValidationVisitor& validation_visitor) { Protobuf::util::JsonParseOptions options; diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index a05587338e26..2f2aff2a3f6e 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -207,11 +207,7 @@ class MessageUtil { } static void checkUnknownFields(const Protobuf::Message& message, - ProtobufMessage::ValidationVisitor& validation_visitor) { - if (!message.GetReflection()->GetUnknownFields(message).empty()) { - validation_visitor.onUnknownField("type " + message.GetTypeName()); - } - } + ProtobufMessage::ValidationVisitor& validation_visitor); static void loadFromJson(const std::string& json, Protobuf::Message& message, ProtobufMessage::ValidationVisitor& validation_visitor); diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 04fb200fc075..9d437982a0e0 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -147,15 +147,31 @@ TEST_F(ProtobufUtilityTest, LoadBinaryProtoFromFile) { EXPECT_TRUE(TestUtility::protoEqual(bootstrap, proto_from_file)); } +// An unknown field (or with wrong type) in a message is rejected. TEST_F(ProtobufUtilityTest, LoadBinaryProtoUnknownFieldFromFile) { ProtobufWkt::Duration source_duration; source_duration.set_seconds(42); const std::string filename = TestEnvironment::writeStringToFileForTest("proto.pb", source_duration.SerializeAsString()); envoy::config::bootstrap::v2::Bootstrap proto_from_file; - EXPECT_THROW_WITH_MESSAGE( - TestUtility::loadFromFile(filename, proto_from_file, *api_), EnvoyException, - "Protobuf message (type envoy.config.bootstrap.v2.Bootstrap) has unknown fields"); + EXPECT_THROW_WITH_MESSAGE(TestUtility::loadFromFile(filename, proto_from_file, *api_), + EnvoyException, + "Protobuf message (type envoy.config.bootstrap.v2.Bootstrap with " + "unknown field set {1}) has unknown fields"); +} + +// Multiple unknown fields (or with wrong type) in a message are rejected. +TEST_F(ProtobufUtilityTest, LoadBinaryProtoUnknownMultipleFieldsFromFile) { + ProtobufWkt::Duration source_duration; + source_duration.set_seconds(42); + source_duration.set_nanos(42); + const std::string filename = + TestEnvironment::writeStringToFileForTest("proto.pb", source_duration.SerializeAsString()); + envoy::config::bootstrap::v2::Bootstrap proto_from_file; + EXPECT_THROW_WITH_MESSAGE(TestUtility::loadFromFile(filename, proto_from_file, *api_), + EnvoyException, + "Protobuf message (type envoy.config.bootstrap.v2.Bootstrap with " + "unknown field set {1, 2}) has unknown fields"); } TEST_F(ProtobufUtilityTest, LoadTextProtoFromFile) { @@ -333,7 +349,8 @@ TEST_F(ProtobufUtilityTest, AnyConvertWrongFields) { source_any.set_type_url("type.google.com/google.protobuf.Timestamp"); EXPECT_THROW_WITH_MESSAGE(TestUtility::anyConvert(source_any), EnvoyException, - "Protobuf message (type google.protobuf.Timestamp) has unknown fields"); + "Protobuf message (type google.protobuf.Timestamp with unknown " + "field set {1}) has unknown fields"); } TEST_F(ProtobufUtilityTest, JsonConvertSuccess) { diff --git a/test/server/server_test.cc b/test/server/server_test.cc index ca2a755d26ac..1b9b0deeee19 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -696,7 +696,7 @@ TEST_P(ServerInstanceImplTest, EmptyBootstrap) { } // Custom header bootstrap succeeds. -TEST_P(ServerInstanceImplTest, CusomHeaderBoostrap) { +TEST_P(ServerInstanceImplTest, CustomHeaderBootstrap) { options_.config_path_ = TestEnvironment::writeStringToFileForTest( "custom.yaml", "header_prefix: \"x-envoy\"\nstatic_resources:\n"); options_.service_cluster_name_ = "some_cluster_name";