From 9732035b83891cd9eb001211504b86c96abc0263 Mon Sep 17 00:00:00 2001 From: Teju Nareddy Date: Fri, 17 Apr 2020 10:29:58 -0700 Subject: [PATCH 1/3] Allow unknown fields in corpus parser As discussed in envoyproxy/envoy#10796, this will allow breaking wire-compatibility changes in the input proto. The pre-existing corpus will still function, but old fields will be ignored. Risk: Typos in the text proto will cause the fuzzer to run on an incomplete proto. Previously, this would log an error message and skip fuzzing with that test case. --- src/text_format.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/text_format.cc b/src/text_format.cc index 94e6a79..4479229 100644 --- a/src/text_format.cc +++ b/src/text_format.cc @@ -30,6 +30,7 @@ bool ParseTextMessage(const std::string& data, protobuf::Message* output) { TextFormat::Parser parser; parser.SetRecursionLimit(100); parser.AllowPartialMessage(true); + parser.AllowUnknownField(true); if (!parser.ParseFromString(data, output)) { output->Clear(); return false; From 40ccfdcbc064c2fd4bed09cc80a08ee9e8a6faad Mon Sep 17 00:00:00 2001 From: Teju Nareddy Date: Fri, 17 Apr 2020 16:20:08 -0700 Subject: [PATCH 2/3] Add test --- src/mutator_test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/mutator_test.cc b/src/mutator_test.cc index fe93181..30b5b2b 100644 --- a/src/mutator_test.cc +++ b/src/mutator_test.cc @@ -293,6 +293,11 @@ const char kOptionalInDeepAnyFields[] = R"( } )"; +const char kUnknownField[] = R"( + optional_bool: true + unknown_field: "test unknown field" +)"; + class TestMutator : public Mutator { public: explicit TestMutator(bool keep_initialized, @@ -668,6 +673,12 @@ TYPED_TEST(MutatorTypedTest, Serialization) { } } +TYPED_TEST(MutatorTypedTest, UnknownSerialization) { + typename TestFixture::Message parsed; + EXPECT_TRUE(ParseTextMessage(kUnknownField, &parsed)); + EXPECT_NE(SaveMessageAsText(parsed), kUnknownField); +} + TYPED_TEST(MutatorTypedTest, DeepRecursion) { typename TestFixture::Message message; typename TestFixture::Message* last = &message; From 74388093f8019571d2f10cea3cd5954a989912f0 Mon Sep 17 00:00:00 2001 From: Teju Nareddy Date: Mon, 20 Apr 2020 09:52:45 -0700 Subject: [PATCH 3/3] Review comments Signed-off-by: Teju Nareddy --- src/mutator_test.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/mutator_test.cc b/src/mutator_test.cc index 30b5b2b..312b2d9 100644 --- a/src/mutator_test.cc +++ b/src/mutator_test.cc @@ -293,11 +293,14 @@ const char kOptionalInDeepAnyFields[] = R"( } )"; -const char kUnknownField[] = R"( +const char kUnknownFieldInput[] = R"( optional_bool: true unknown_field: "test unknown field" )"; +const char kUnknownFieldExpected[] = R"(optional_bool: true +)"; + class TestMutator : public Mutator { public: explicit TestMutator(bool keep_initialized, @@ -673,10 +676,10 @@ TYPED_TEST(MutatorTypedTest, Serialization) { } } -TYPED_TEST(MutatorTypedTest, UnknownSerialization) { +TYPED_TEST(MutatorTypedTest, UnknownFieldTextFormat) { typename TestFixture::Message parsed; - EXPECT_TRUE(ParseTextMessage(kUnknownField, &parsed)); - EXPECT_NE(SaveMessageAsText(parsed), kUnknownField); + EXPECT_TRUE(ParseTextMessage(kUnknownFieldInput, &parsed)); + EXPECT_EQ(SaveMessageAsText(parsed), kUnknownFieldExpected); } TYPED_TEST(MutatorTypedTest, DeepRecursion) {