From 31999a3f95d8ec9f93b56b0966e2895c5205da53 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Wed, 22 Jun 2016 11:46:10 -0700 Subject: Add JsonParseOptions to ignore unknown fields - add JsonParseOptions for JsonToBinaryString allow unknown fields - rename current JsonOptions to JsonPrintOptions --- src/google/protobuf/util/internal/proto_writer.cc | 6 +- src/google/protobuf/util/internal/proto_writer.h | 10 +- .../util/internal/protostream_objectwriter.cc | 4 +- .../util/internal/protostream_objectwriter.h | 7 +- .../util/internal/protostream_objectwriter_test.cc | 117 +++++++++++++++++++++ src/google/protobuf/util/json_util.cc | 18 ++-- src/google/protobuf/util/json_util.h | 49 +++++++-- src/google/protobuf/util/json_util_test.cc | 35 +++--- 8 files changed, 214 insertions(+), 32 deletions(-) diff --git a/src/google/protobuf/util/internal/proto_writer.cc b/src/google/protobuf/util/internal/proto_writer.cc index 36b79410..052a0c96 100644 --- a/src/google/protobuf/util/internal/proto_writer.cc +++ b/src/google/protobuf/util/internal/proto_writer.cc @@ -71,6 +71,7 @@ ProtoWriter::ProtoWriter(TypeResolver* type_resolver, adapter_(&buffer_), stream_(new CodedOutputStream(&adapter_)), listener_(listener), + ignore_unknown_fields_(false), invalid_depth_(0), tracker_(new ObjectLocationTracker()) {} @@ -88,6 +89,7 @@ ProtoWriter::ProtoWriter(const TypeInfo* typeinfo, adapter_(&buffer_), stream_(new CodedOutputStream(&adapter_)), listener_(listener), + ignore_unknown_fields_(false), invalid_depth_(0), tracker_(new ObjectLocationTracker()) {} @@ -692,7 +694,9 @@ const google::protobuf::Field* ProtoWriter::Lookup( } const google::protobuf::Field* field = typeinfo_->FindField(&e->type(), unnormalized_name); - if (field == NULL) InvalidName(unnormalized_name, "Cannot find field."); + if (field == NULL && !ignore_unknown_fields_) { + InvalidName(unnormalized_name, "Cannot find field."); + } return field; } diff --git a/src/google/protobuf/util/internal/proto_writer.h b/src/google/protobuf/util/internal/proto_writer.h index 957565e7..6f276065 100644 --- a/src/google/protobuf/util/internal/proto_writer.h +++ b/src/google/protobuf/util/internal/proto_writer.h @@ -142,6 +142,10 @@ class LIBPROTOBUF_EXPORT ProtoWriter : public StructuredObjectWriter { const TypeInfo* typeinfo() { return typeinfo_; } + void set_ignore_unknown_fields(bool ignore_unknown_fields) { + ignore_unknown_fields_ = ignore_unknown_fields; + } + protected: class LIBPROTOBUF_EXPORT ProtoElement : public BaseElement, public LocationTrackerInterface { public: @@ -240,7 +244,8 @@ class LIBPROTOBUF_EXPORT ProtoWriter : public StructuredObjectWriter { // Lookup the field in the current element. Looks in the base descriptor // and in any extension. This will report an error if the field cannot be - // found or if multiple matching extensions are found. + // found when ignore_unknown_names_ is false or if multiple matching + // extensions are found. const google::protobuf::Field* Lookup(StringPiece name); // Lookup the field type in the type descriptor. Returns NULL if the type @@ -293,6 +298,9 @@ class LIBPROTOBUF_EXPORT ProtoWriter : public StructuredObjectWriter { // Indicates whether we finished writing root message completely. bool done_; + // If true, don't report unknown field names to the listener. + bool ignore_unknown_fields_; + // Variable for internal state processing: // element_ : the current element. // size_insert_: sizes of nested messages. diff --git a/src/google/protobuf/util/internal/protostream_objectwriter.cc b/src/google/protobuf/util/internal/protostream_objectwriter.cc index 97a7909a..a6de246d 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter.cc +++ b/src/google/protobuf/util/internal/protostream_objectwriter.cc @@ -63,7 +63,9 @@ ProtoStreamObjectWriter::ProtoStreamObjectWriter( : ProtoWriter(type_resolver, type, output, listener), master_type_(type), current_(NULL), - options_(options) {} + options_(options) { + set_ignore_unknown_fields(options_.ignore_unknown_fields); +} ProtoStreamObjectWriter::ProtoStreamObjectWriter( const TypeInfo* typeinfo, const google::protobuf::Type& type, diff --git a/src/google/protobuf/util/internal/protostream_objectwriter.h b/src/google/protobuf/util/internal/protostream_objectwriter.h index e1162d43..8043bb3b 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter.h +++ b/src/google/protobuf/util/internal/protostream_objectwriter.h @@ -83,7 +83,12 @@ class LIBPROTOBUF_EXPORT ProtoStreamObjectWriter : public ProtoWriter { // preserve integer precision. bool struct_integers_as_strings; - Options() : struct_integers_as_strings(false) {} + // Not treat unknown fields as an error. If there is an unknown fields, + // just ignore it and continue to process the rest. + bool ignore_unknown_fields; + + Options() + : struct_integers_as_strings(false), ignore_unknown_fields(false) {} // Default instance of Options with all options set to defaults. static const Options& Defaults() { diff --git a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc index 9a0dcde1..30e58e62 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc +++ b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc @@ -183,6 +183,10 @@ class ProtoStreamObjectWriterTest : public BaseProtoStreamObjectWriterTest { ProtoStreamObjectWriterTest() : BaseProtoStreamObjectWriterTest(Book::descriptor()) {} + void ResetProtoWriter() { + ResetTypeInfo(Book::descriptor()); + } + virtual ~ProtoStreamObjectWriterTest() {} }; @@ -709,6 +713,119 @@ TEST_P(ProtoStreamObjectWriterTest, UnknownListAtPublisher) { CheckOutput(expected); } +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownFieldAtRoot) { + Book empty; + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, _, _)).Times(0); + ow_->StartObject("")->RenderString("unknown", "Nope!")->EndObject(); + CheckOutput(empty, 0); +} + +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownFieldAtAuthorFriend) { + Book expected; + Author* paul = expected.mutable_author(); + paul->set_name("Paul"); + Author* mark = paul->add_friend_(); + mark->set_name("Mark"); + Author* john = paul->add_friend_(); + john->set_name("John"); + Author* luke = paul->add_friend_(); + luke->set_name("Luke"); + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, _, _)).Times(0); + ow_->StartObject("") + ->StartObject("author") + ->RenderString("name", "Paul") + ->StartList("friend") + ->StartObject("") + ->RenderString("name", "Mark") + ->EndObject() + ->StartObject("") + ->RenderString("name", "John") + ->RenderString("address", "Patmos") + ->EndObject() + ->StartObject("") + ->RenderString("name", "Luke") + ->EndObject() + ->EndList() + ->EndObject() + ->EndObject(); + CheckOutput(expected); +} + +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownObjectAtRoot) { + Book empty; + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, StringPiece("unknown"), + StringPiece("Cannot find field."))) + .Times(0); + ow_->StartObject("")->StartObject("unknown")->EndObject()->EndObject(); + CheckOutput(empty, 0); +} + +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownObjectAtAuthor) { + Book expected; + Author* author = expected.mutable_author(); + author->set_name("William"); + author->add_pseudonym("Bill"); + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, _, _)).Times(0); + ow_->StartObject("") + ->StartObject("author") + ->RenderString("name", "William") + ->StartObject("wife") + ->RenderString("name", "Hilary") + ->EndObject() + ->RenderString("pseudonym", "Bill") + ->EndObject() + ->EndObject(); + CheckOutput(expected); +} + +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownListAtRoot) { + Book empty; + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, _, _)).Times(0); + ow_->StartObject("")->StartList("unknown")->EndList()->EndObject(); + CheckOutput(empty, 0); +} + +TEST_P(ProtoStreamObjectWriterTest, IgnoreUnknownListAtPublisher) { + Book expected; + expected.set_title("Brainwashing"); + Publisher* publisher = expected.mutable_publisher(); + publisher->set_name("propaganda"); + + options_.ignore_unknown_fields = true; + ResetProtoWriter(); + + EXPECT_CALL(listener_, InvalidName(_, _, _)).Times(0); + ow_->StartObject("") + ->StartObject("publisher") + ->RenderString("name", "propaganda") + ->StartList("alliance") + ->EndList() + ->EndObject() + ->RenderString("title", "Brainwashing") + ->EndObject(); + CheckOutput(expected); +} + TEST_P(ProtoStreamObjectWriterTest, MissingRequiredField) { Book expected; expected.set_title("My Title"); diff --git a/src/google/protobuf/util/json_util.cc b/src/google/protobuf/util/json_util.cc index 2659320a..6d45a4f9 100644 --- a/src/google/protobuf/util/json_util.cc +++ b/src/google/protobuf/util/json_util.cc @@ -74,7 +74,7 @@ util::Status BinaryToJsonStream(TypeResolver* resolver, const string& type_url, io::ZeroCopyInputStream* binary_input, io::ZeroCopyOutputStream* json_output, - const JsonOptions& options) { + const JsonPrintOptions& options) { io::CodedInputStream in_stream(binary_input); google::protobuf::Type type; RETURN_IF_ERROR(resolver->ResolveMessageType(type_url, &type)); @@ -95,7 +95,7 @@ util::Status BinaryToJsonString(TypeResolver* resolver, const string& type_url, const string& binary_input, string* json_output, - const JsonOptions& options) { + const JsonPrintOptions& options) { io::ArrayInputStream input_stream(binary_input.data(), binary_input.size()); io::StringOutputStream output_stream(json_output); return BinaryToJsonStream(resolver, type_url, &input_stream, &output_stream, @@ -141,13 +141,17 @@ class StatusErrorListener : public converter::ErrorListener { util::Status JsonToBinaryStream(TypeResolver* resolver, const string& type_url, io::ZeroCopyInputStream* json_input, - io::ZeroCopyOutputStream* binary_output) { + io::ZeroCopyOutputStream* binary_output, + const JsonParseOptions& options) { google::protobuf::Type type; RETURN_IF_ERROR(resolver->ResolveMessageType(type_url, &type)); internal::ZeroCopyStreamByteSink sink(binary_output); StatusErrorListener listener; + converter::ProtoStreamObjectWriter::Options proto_writer_options; + proto_writer_options.ignore_unknown_fields = options.ignore_unknown_fields; converter::ProtoStreamObjectWriter proto_writer(resolver, type, &sink, - &listener); + &listener, + proto_writer_options); converter::JsonStreamParser parser(&proto_writer); const void* buffer; @@ -165,10 +169,12 @@ util::Status JsonToBinaryStream(TypeResolver* resolver, util::Status JsonToBinaryString(TypeResolver* resolver, const string& type_url, const string& json_input, - string* binary_output) { + string* binary_output, + const JsonParseOptions& options) { io::ArrayInputStream input_stream(json_input.data(), json_input.size()); io::StringOutputStream output_stream(binary_output); - return JsonToBinaryStream(resolver, type_url, &input_stream, &output_stream); + return JsonToBinaryStream( + resolver, type_url, &input_stream, &output_stream, options); } } // namespace util diff --git a/src/google/protobuf/util/json_util.h b/src/google/protobuf/util/json_util.h index 1718bfb5..b4c2579b 100644 --- a/src/google/protobuf/util/json_util.h +++ b/src/google/protobuf/util/json_util.h @@ -44,7 +44,14 @@ class ZeroCopyOutputStream; } // namespace io namespace util { -struct JsonOptions { +struct JsonParseOptions { + // Whether to ignore unknown JSON fields during parsing + bool ignore_unknown_fields; + + JsonParseOptions() : ignore_unknown_fields(false) {} +}; + +struct JsonPrintOptions { // Whether to add spaces, line breaks and indentation to make the JSON output // easy to read. bool add_whitespace; @@ -54,11 +61,14 @@ struct JsonOptions { // behavior and print primitive fields regardless of their values. bool always_print_primitive_fields; - JsonOptions() : add_whitespace(false), - always_print_primitive_fields(false) { + JsonPrintOptions() : add_whitespace(false), + always_print_primitive_fields(false) { } }; +// DEPRECATED. Use JsonPrintOptions instead. +typedef JsonPrintOptions JsonOptions; + // Converts protobuf binary data to JSON. // The conversion will fail if: // 1. TypeResolver fails to resolve a type. @@ -70,14 +80,14 @@ util::Status BinaryToJsonStream( const string& type_url, io::ZeroCopyInputStream* binary_input, io::ZeroCopyOutputStream* json_output, - const JsonOptions& options); + const JsonPrintOptions& options); inline util::Status BinaryToJsonStream( TypeResolver* resolver, const string& type_url, io::ZeroCopyInputStream* binary_input, io::ZeroCopyOutputStream* json_output) { return BinaryToJsonStream(resolver, type_url, binary_input, json_output, - JsonOptions()); + JsonPrintOptions()); } LIBPROTOBUF_EXPORT util::Status BinaryToJsonString( @@ -85,14 +95,14 @@ LIBPROTOBUF_EXPORT util::Status BinaryToJsonString( const string& type_url, const string& binary_input, string* json_output, - const JsonOptions& options); + const JsonPrintOptions& options); inline util::Status BinaryToJsonString(TypeResolver* resolver, const string& type_url, const string& binary_input, string* json_output) { return BinaryToJsonString(resolver, type_url, binary_input, json_output, - JsonOptions()); + JsonPrintOptions()); } // Converts JSON data to protobuf binary format. @@ -100,18 +110,37 @@ inline util::Status BinaryToJsonString(TypeResolver* resolver, // 1. TypeResolver fails to resolve a type. // 2. input is not valid JSON format, or conflicts with the type // information returned by TypeResolver. -// 3. input has unknown fields. util::Status JsonToBinaryStream( TypeResolver* resolver, const string& type_url, io::ZeroCopyInputStream* json_input, - io::ZeroCopyOutputStream* binary_output); + io::ZeroCopyOutputStream* binary_output, + const JsonParseOptions& options); + +inline util::Status JsonToBinaryStream( + TypeResolver* resolver, + const string& type_url, + io::ZeroCopyInputStream* json_input, + io::ZeroCopyOutputStream* binary_output) { + return JsonToBinaryStream(resolver, type_url, json_input, binary_output, + JsonParseOptions()); +} LIBPROTOBUF_EXPORT util::Status JsonToBinaryString( TypeResolver* resolver, const string& type_url, const string& json_input, - string* binary_output); + string* binary_output, + const JsonParseOptions& options); + +inline util::Status JsonToBinaryString( + TypeResolver* resolver, + const string& type_url, + const string& json_input, + string* binary_output) { + return JsonToBinaryString(resolver, type_url, json_input, binary_output, + JsonParseOptions()); +} namespace internal { // Internal helper class. Put in the header so we can write unit-tests for it. diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index a4d3cc98..c7d5c59e 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -67,7 +67,7 @@ class JsonUtilTest : public testing::Test { kTypeUrlPrefix, DescriptorPool::generated_pool())); } - string ToJson(const Message& message, const JsonOptions& options) { + string ToJson(const Message& message, const JsonPrintOptions& options) { string result; GOOGLE_CHECK_OK(BinaryToJsonString(resolver_.get(), GetTypeUrl(message.GetDescriptor()), @@ -75,10 +75,12 @@ class JsonUtilTest : public testing::Test { return result; } - bool FromJson(const string& json, Message* message) { + bool FromJson(const string& json, Message* message, + const JsonParseOptions& options) { string binary; if (!JsonToBinaryString(resolver_.get(), - GetTypeUrl(message->GetDescriptor()), json, &binary) + GetTypeUrl(message->GetDescriptor()), json, &binary, + options) .ok()) { return false; } @@ -92,7 +94,7 @@ TEST_F(JsonUtilTest, TestWhitespaces) { TestMessage m; m.mutable_message_value(); - JsonOptions options; + JsonPrintOptions options; EXPECT_EQ("{\"messageValue\":{}}", ToJson(m, options)); options.add_whitespace = true; EXPECT_EQ( @@ -104,7 +106,7 @@ TEST_F(JsonUtilTest, TestWhitespaces) { TEST_F(JsonUtilTest, TestDefaultValues) { TestMessage m; - JsonOptions options; + JsonPrintOptions options; EXPECT_EQ("{}", ToJson(m, options)); options.always_print_primitive_fields = true; EXPECT_EQ( @@ -147,8 +149,9 @@ TEST_F(JsonUtilTest, ParseMessage) { " {\"value\": 40}, {\"value\": 96}\n" " ]\n" "}\n"; + JsonParseOptions options; TestMessage m; - ASSERT_TRUE(FromJson(input, &m)); + ASSERT_TRUE(FromJson(input, &m, options)); EXPECT_EQ(1024, m.int32_value()); ASSERT_EQ(2, m.repeated_int32_value_size()); EXPECT_EQ(1, m.repeated_int32_value(0)); @@ -162,20 +165,28 @@ TEST_F(JsonUtilTest, ParseMessage) { TEST_F(JsonUtilTest, ParseMap) { TestMap message; (*message.mutable_string_map())["hello"] = 1234; - JsonOptions options; - EXPECT_EQ("{\"stringMap\":{\"hello\":1234}}", ToJson(message, options)); + JsonPrintOptions print_options; + JsonParseOptions parse_options; + EXPECT_EQ("{\"stringMap\":{\"hello\":1234}}", ToJson(message, print_options)); TestMap other; - ASSERT_TRUE(FromJson(ToJson(message, options), &other)); + ASSERT_TRUE(FromJson(ToJson(message, print_options), &other, parse_options)); EXPECT_EQ(message.DebugString(), other.DebugString()); } +TEST_F(JsonUtilTest, TestParseIgnoreUnknownFields) { + TestMessage m; + JsonParseOptions options; + options.ignore_unknown_fields = true; + EXPECT_TRUE(FromJson("{\"unknownName\":0}", &m, options)); +} + TEST_F(JsonUtilTest, TestParseErrors) { TestMessage m; - JsonOptions options; + JsonParseOptions options; // Parsing should fail if the field name can not be recognized. - EXPECT_FALSE(FromJson("{\"unknownName\":0}", &m)); + EXPECT_FALSE(FromJson("{\"unknownName\":0}", &m, options)); // Parsing should fail if the value is invalid. - EXPECT_FALSE(FromJson("{\"int32Value\":2147483648}", &m)); + EXPECT_FALSE(FromJson("{\"int32Value\":2147483648}", &m, options)); } typedef pair Segment; -- cgit v1.2.3