From 4008d229aa7505775486d6a86705efd2800599b3 Mon Sep 17 00:00:00 2001 From: Derek Perez Date: Thu, 1 Apr 2021 18:49:52 +0000 Subject: [PATCH] Sync from Piper @366297034 PROTOBUF_SYNC_PIPER --- .github/mergeable.yml | 2 +- .github/workflows/codespell.yml | 2 +- .gitignore | 1 + CHANGES.txt | 1 + csharp/src/Google.Protobuf.Test/testprotos.pb | Bin 340187 -> 340240 bytes .../Google.Protobuf/Reflection/Descriptor.cs | 16 ++--- .../com/google/protobuf/CodedInputStream.java | 3 + .../google/protobuf/CodedInputStreamTest.java | 29 ++++++++ .../Google/Protobuf/Internal/FileOptions.php | 64 ++++++++--------- protoc-artifacts/README.md | 2 +- src/google/protobuf/descriptor.proto | 16 ++--- src/google/protobuf/port_undef.inc | 4 ++ src/google/protobuf/util/field_comparator.h | 17 ++--- .../protobuf/util/message_differencer.cc | 65 +++++++++++------- .../protobuf/util/message_differencer.h | 15 +++- 15 files changed, 151 insertions(+), 86 deletions(-) diff --git a/.github/mergeable.yml b/.github/mergeable.yml index 4027cf5a9f..ade6c679a4 100644 --- a/.github/mergeable.yml +++ b/.github/mergeable.yml @@ -11,7 +11,7 @@ mergeable: regex: 'release notes: yes' message: 'Please include release notes: yes' - must_include: - regex: '^(autotools|bazel|c#|c\+\+|cleanup|cmake|conformance tests|integration|go|java|javascript|objective-c|php|protoc|python|ruby)' + regex: '^(autotools|bazel|c#|c\+\+|cleanup|cmake|conformance tests|integration|go|java|javascript|objective-c|php|protoc|python|ruby|kotlin)' message: 'Please include at least a language label (e.g., c++, java, python). Or apply one of the following labels: autotools, bazel, cmake, cleanup, conformance tests, integration, protoc.' - must_include: regex: 'release notes: no' diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index a138098c94..5d252faa06 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -13,4 +13,4 @@ jobs: with: check_filenames: true skip: ./.git,./conformance/third_party,*.snk,*.pb,*.pb.cc,*.pb.h,./src/google/protobuf/testdata,./objectivec/Tests,./python/compatibility_tests/v2.5.0/tests/google/protobuf/internal - ignore_words_list: "alow,alse,ba,cleare,copyable,cloneable,dedup,dur,errorprone,files',fo,fundementals,hel,importd,inout,leapyear,nd,nin,ois,ons,parseable,process',te,testof,ue,unparseable,wasn,wee,gae,keyserver,objext,od" + ignore_words_list: "alow,alse,ba,cleare,copyable,cloneable,dedup,dur,errorprone,files',fo,fundementals,hel,importd,inout,leapyear,nd,nin,ois,ons,parseable,process',te,testof,ue,unparseable,wasn,wee,gae,keyserver,objext,od,OptIn" diff --git a/.gitignore b/.gitignore index 44ab2d49ef..4880c495de 100644 --- a/.gitignore +++ b/.gitignore @@ -83,6 +83,7 @@ src/**/*.trs # JavaBuild output. java/core/target +java/lite/target java/util/target javanano/target java/.idea diff --git a/CHANGES.txt b/CHANGES.txt index 99d9ad712d..05a15edbf5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) C++ + * The ::pb namespace is no longer exposed due to conflicts. * Allow MessageDifferencer::TreatAsSet() (and friends) to override previous calls instead of crashing. * Reduce the size of generated proto headers for protos with `string` or diff --git a/csharp/src/Google.Protobuf.Test/testprotos.pb b/csharp/src/Google.Protobuf.Test/testprotos.pb index 06a17c1038c06c8a5879a2950630314fab7d1126..42ecd3adf8e39e1f51a98d1b7fc37c7d63b3fdac 100644 GIT binary patch delta 428 zcmYk2ze@u#9K}g`T~4P`3KoU0^ol>}sDp@uE;>63CH8V{B$pgX>d{FMLZNSseriadb0xRp{{GoABQEeUg{g^3AyXFdTVL7X{ybRf>!KkI~%%K7a0C zzu#`3ePiGAe2hxo8^&wZF@{|!jFMb~=}-_PrXZtyP8CTKs^EZ}5@>OvHMFUqiWnL~ zBvo!|BuW|y5#zMs!(oS*KWoZ32f|a*)6jAigb5eepfHtK$~Kxks#}UBMyll%h^gtw z5Qz36rP&4zMD#4wQQiKmqM0MtvLC^TV`|1-Zdk%;Gi%qtG=xl(V@|{Mj7_+DoSb8B_-J-x}Z z*XPN_*Y(cufw_+3GArYJGPa%@GuUrKK)M1b1Za<-DKH2qfVQu?=|!#QE3Kf-1p)d5 zk&nF|wSY@N2)OJBazTZf{J9n)uDw**;eC=Iu}d|8LyreAG*ZEhyD@Ai^Xwq;-f*Er z$06^dAgPmG0-{6<)~Zz{seXigtS10#pn94sSq7A?xp)&wkw?B^Kv - /// If set, all the classes from the .proto file are wrapped in a single - /// outer class with the given name. This applies to both Proto1 - /// (equivalent to the old "--one_java_file" option) and Proto2 (where - /// a .proto always translates to a single class, but you may want to - /// explicitly choose the class name). + /// Controls the name of the wrapper Java class generated for the .proto file. + /// That class will always contain the .proto file's getDescriptor() method as + /// well as any top-level extensions defined in the .proto file. + /// If java_multiple_files is disabled, then all the other classes from the + /// .proto file will be nested inside the single wrapper outer class. /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] public string JavaOuterClassname { @@ -4826,10 +4826,10 @@ namespace Google.Protobuf.Reflection { private bool javaMultipleFiles_; /// - /// If set true, then the Java code generator will generate a separate .java + /// If enabled, then the Java code generator will generate a separate .java /// file for each top-level message, enum, and service defined in the .proto - /// file. Thus, these types will *not* be nested inside the outer class - /// named by java_outer_classname. However, the outer class will still be + /// file. Thus, these types will *not* be nested inside the wrapper class + /// named by java_outer_classname. However, the wrapper class will still be /// generated to contain the file's getDescriptor() method as well as any /// top-level extensions defined in the file. /// diff --git a/java/core/src/main/java/com/google/protobuf/CodedInputStream.java b/java/core/src/main/java/com/google/protobuf/CodedInputStream.java index 6098a9ad8e..37b986d7a5 100644 --- a/java/core/src/main/java/com/google/protobuf/CodedInputStream.java +++ b/java/core/src/main/java/com/google/protobuf/CodedInputStream.java @@ -1185,6 +1185,9 @@ public abstract class CodedInputStream { throw InvalidProtocolBufferException.negativeSize(); } byteLimit += getTotalBytesRead(); + if (byteLimit < 0) { + throw InvalidProtocolBufferException.parseFailure(); + } final int oldLimit = currentLimit; if (byteLimit > oldLimit) { throw InvalidProtocolBufferException.truncatedMessage(); diff --git a/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java index 4de5f5b12d..10d156ead2 100644 --- a/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java +++ b/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java @@ -1284,4 +1284,33 @@ public class CodedInputStreamTest extends TestCase { maliciousCapture.get(1)[0] = 0x9; assertEquals(0x9, byteArray[0]); // MODIFICATION! Should we fix? } + + public void testInvalidInputYieldsInvalidProtocolBufferException_readTag() throws Exception { + byte[] input = new byte[] {0x0a, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x77}; + CodedInputStream inputStream = CodedInputStream.newInstance(input); + try { + inputStream.readTag(); + int size = inputStream.readRawVarint32(); + inputStream.pushLimit(size); + inputStream.readTag(); + fail(); + } catch (InvalidProtocolBufferException ex) { + // Expected. + } + } + + public void testInvalidInputYieldsInvalidProtocolBufferException_readBytes() throws Exception { + byte[] input = + new byte[] {0x0a, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x67, 0x1a, 0x1a}; + CodedInputStream inputStream = CodedInputStream.newInstance(input); + try { + inputStream.readTag(); + int size = inputStream.readRawVarint32(); + inputStream.pushLimit(size); + inputStream.readBytes(); + fail(); + } catch (InvalidProtocolBufferException ex) { + // Expected. + } + } } diff --git a/php/src/Google/Protobuf/Internal/FileOptions.php b/php/src/Google/Protobuf/Internal/FileOptions.php index 6283b2ad7a..3f2c3a4d4c 100644 --- a/php/src/Google/Protobuf/Internal/FileOptions.php +++ b/php/src/Google/Protobuf/Internal/FileOptions.php @@ -25,20 +25,20 @@ class FileOptions extends \Google\Protobuf\Internal\Message */ protected $java_package = null; /** - * If set, all the classes from the .proto file are wrapped in a single - * outer class with the given name. This applies to both Proto1 - * (equivalent to the old "--one_java_file" option) and Proto2 (where - * a .proto always translates to a single class, but you may want to - * explicitly choose the class name). + * Controls the name of the wrapper Java class generated for the .proto file. + * That class will always contain the .proto file's getDescriptor() method as + * well as any top-level extensions defined in the .proto file. + * If java_multiple_files is disabled, then all the other classes from the + * .proto file will be nested inside the single wrapper outer class. * * Generated from protobuf field optional string java_outer_classname = 8; */ protected $java_outer_classname = null; /** - * If set true, then the Java code generator will generate a separate .java + * If enabled, then the Java code generator will generate a separate .java * file for each top-level message, enum, and service defined in the .proto - * file. Thus, these types will *not* be nested inside the outer class - * named by java_outer_classname. However, the outer class will still be + * file. Thus, these types will *not* be nested inside the wrapper class + * named by java_outer_classname. However, the wrapper class will still be * generated to contain the file's getDescriptor() method as well as any * top-level extensions defined in the file. * @@ -192,16 +192,16 @@ class FileOptions extends \Google\Protobuf\Internal\Message * inappropriate because proto packages do not normally start with backwards * domain names. * @type string $java_outer_classname - * If set, all the classes from the .proto file are wrapped in a single - * outer class with the given name. This applies to both Proto1 - * (equivalent to the old "--one_java_file" option) and Proto2 (where - * a .proto always translates to a single class, but you may want to - * explicitly choose the class name). + * Controls the name of the wrapper Java class generated for the .proto file. + * That class will always contain the .proto file's getDescriptor() method as + * well as any top-level extensions defined in the .proto file. + * If java_multiple_files is disabled, then all the other classes from the + * .proto file will be nested inside the single wrapper outer class. * @type bool $java_multiple_files - * If set true, then the Java code generator will generate a separate .java + * If enabled, then the Java code generator will generate a separate .java * file for each top-level message, enum, and service defined in the .proto - * file. Thus, these types will *not* be nested inside the outer class - * named by java_outer_classname. However, the outer class will still be + * file. Thus, these types will *not* be nested inside the wrapper class + * named by java_outer_classname. However, the wrapper class will still be * generated to contain the file's getDescriptor() method as well as any * top-level extensions defined in the file. * @type bool $java_generate_equals_and_hash @@ -319,11 +319,11 @@ class FileOptions extends \Google\Protobuf\Internal\Message } /** - * If set, all the classes from the .proto file are wrapped in a single - * outer class with the given name. This applies to both Proto1 - * (equivalent to the old "--one_java_file" option) and Proto2 (where - * a .proto always translates to a single class, but you may want to - * explicitly choose the class name). + * Controls the name of the wrapper Java class generated for the .proto file. + * That class will always contain the .proto file's getDescriptor() method as + * well as any top-level extensions defined in the .proto file. + * If java_multiple_files is disabled, then all the other classes from the + * .proto file will be nested inside the single wrapper outer class. * * Generated from protobuf field optional string java_outer_classname = 8; * @return string @@ -344,11 +344,11 @@ class FileOptions extends \Google\Protobuf\Internal\Message } /** - * If set, all the classes from the .proto file are wrapped in a single - * outer class with the given name. This applies to both Proto1 - * (equivalent to the old "--one_java_file" option) and Proto2 (where - * a .proto always translates to a single class, but you may want to - * explicitly choose the class name). + * Controls the name of the wrapper Java class generated for the .proto file. + * That class will always contain the .proto file's getDescriptor() method as + * well as any top-level extensions defined in the .proto file. + * If java_multiple_files is disabled, then all the other classes from the + * .proto file will be nested inside the single wrapper outer class. * * Generated from protobuf field optional string java_outer_classname = 8; * @param string $var @@ -363,10 +363,10 @@ class FileOptions extends \Google\Protobuf\Internal\Message } /** - * If set true, then the Java code generator will generate a separate .java + * If enabled, then the Java code generator will generate a separate .java * file for each top-level message, enum, and service defined in the .proto - * file. Thus, these types will *not* be nested inside the outer class - * named by java_outer_classname. However, the outer class will still be + * file. Thus, these types will *not* be nested inside the wrapper class + * named by java_outer_classname. However, the wrapper class will still be * generated to contain the file's getDescriptor() method as well as any * top-level extensions defined in the file. * @@ -389,10 +389,10 @@ class FileOptions extends \Google\Protobuf\Internal\Message } /** - * If set true, then the Java code generator will generate a separate .java + * If enabled, then the Java code generator will generate a separate .java * file for each top-level message, enum, and service defined in the .proto - * file. Thus, these types will *not* be nested inside the outer class - * named by java_outer_classname. However, the outer class will still be + * file. Thus, these types will *not* be nested inside the wrapper class + * named by java_outer_classname. However, the wrapper class will still be * generated to contain the file's getDescriptor() method as well as any * top-level extensions defined in the file. * diff --git a/protoc-artifacts/README.md b/protoc-artifacts/README.md index 1706ddc0d8..8fc366935d 100644 --- a/protoc-artifacts/README.md +++ b/protoc-artifacts/README.md @@ -20,7 +20,7 @@ the following files: ## Maven Location The published protoc artifacts are available on Maven here: - http://central.maven.org/maven2/com/google/protobuf/protoc/ + https://repo.maven.apache.org/maven2/com/google/protobuf/protoc/ ## Versioning The version of the ``protoc`` artifact must be the same as the version of the diff --git a/src/google/protobuf/descriptor.proto b/src/google/protobuf/descriptor.proto index 69c75208ee..156e410ae1 100644 --- a/src/google/protobuf/descriptor.proto +++ b/src/google/protobuf/descriptor.proto @@ -348,17 +348,17 @@ message FileOptions { optional string java_package = 1; - // If set, all the classes from the .proto file are wrapped in a single - // outer class with the given name. This applies to both Proto1 - // (equivalent to the old "--one_java_file" option) and Proto2 (where - // a .proto always translates to a single class, but you may want to - // explicitly choose the class name). + // Controls the name of the wrapper Java class generated for the .proto file. + // That class will always contain the .proto file's getDescriptor() method as + // well as any top-level extensions defined in the .proto file. + // If java_multiple_files is disabled, then all the other classes from the + // .proto file will be nested inside the single wrapper outer class. optional string java_outer_classname = 8; - // If set true, then the Java code generator will generate a separate .java + // If enabled, then the Java code generator will generate a separate .java // file for each top-level message, enum, and service defined in the .proto - // file. Thus, these types will *not* be nested inside the outer class - // named by java_outer_classname. However, the outer class will still be + // file. Thus, these types will *not* be nested inside the wrapper class + // named by java_outer_classname. However, the wrapper class will still be // generated to contain the file's getDescriptor() method as well as any // top-level extensions defined in the file. optional bool java_multiple_files = 10 [default = false]; diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index daef09bc45..85917621d4 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -79,6 +79,10 @@ #undef PROTOBUF_ATTRIBUTE_INIT_PRIORITY #undef PROTOBUF_PRAGMA_INIT_SEG +#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +#undef PROTOBUF_FUTURE_BREAKING_CHANGES +#endif + // Restore macro that may have been #undef'd in port_def.inc. #ifdef _MSC_VER #pragma pop_macro("CREATE_NEW") diff --git a/src/google/protobuf/util/field_comparator.h b/src/google/protobuf/util/field_comparator.h index 4bd5395e56..dd1a486996 100644 --- a/src/google/protobuf/util/field_comparator.h +++ b/src/google/protobuf/util/field_comparator.h @@ -146,15 +146,6 @@ class PROTOBUF_EXPORT SimpleFieldComparator : public FieldComparator { void SetDefaultFractionAndMargin(double fraction, double margin); protected: - // NOTE: this will go away. - ComparisonResult Compare(const Message& message_1, const Message& message_2, - const FieldDescriptor* field, int index_1, - int index_2, - const util::FieldContext* field_context) override { - return SimpleCompare(message_1, message_2, field, index_1, index_2, - field_context); - } - // Returns the comparison result for the given field in two messages. // // This function is called directly by DefaultFieldComparator::Compare. @@ -268,7 +259,13 @@ class PROTOBUF_EXPORT SimpleFieldComparator : public FieldComparator { }; // Default field comparison: use the basic implementation of FieldComparator. -class PROTOBUF_EXPORT DefaultFieldComparator : public SimpleFieldComparator { +#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +class PROTOBUF_EXPORT DefaultFieldComparator final + : public SimpleFieldComparator +#else // PROTOBUF_FUTURE_BREAKING_CHANGES +class PROTOBUF_EXPORT DefaultFieldComparator : public SimpleFieldComparator +#endif // PROTOBUF_FUTURE_BREAKING_CHANGES +{ public: ComparisonResult Compare(const Message& message_1, const Message& message_2, const FieldDescriptor* field, int index_1, diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc index 10fdc021a2..4815178cd4 100644 --- a/src/google/protobuf/util/message_differencer.cc +++ b/src/google/protobuf/util/message_differencer.cc @@ -148,18 +148,16 @@ class MessageDifferencer::MultipleFieldsMapKeyComparator const FieldDescriptor* field = key_field_path[path_index]; std::vector current_parent_fields(parent_fields); if (path_index == static_cast(key_field_path.size() - 1)) { - if (field->is_repeated()) { - if (!message_differencer_->CompareRepeatedField( - message1, message2, field, ¤t_parent_fields)) { - return false; - } + if (field->is_map()) { + return message_differencer_->CompareMapField(message1, message2, field, + ¤t_parent_fields); + } else if (field->is_repeated()) { + return message_differencer_->CompareRepeatedField( + message1, message2, field, ¤t_parent_fields); } else { - if (!message_differencer_->CompareFieldValueUsingParentFields( - message1, message2, field, -1, -1, ¤t_parent_fields)) { - return false; - } + return message_differencer_->CompareFieldValueUsingParentFields( + message1, message2, field, -1, -1, ¤t_parent_fields); } - return true; } else { const Reflection* reflection1 = message1.GetReflection(); const Reflection* reflection2 = message2.GetReflection(); @@ -830,24 +828,17 @@ bool MessageDifferencer::CompareWithFieldsInternal( bool fieldDifferent = false; assert(field1 != NULL); - if (field1->is_repeated()) { + if (field1->is_map()) { + fieldDifferent = + !CompareMapField(message1, message2, field1, parent_fields); + } else if (field1->is_repeated()) { fieldDifferent = !CompareRepeatedField(message1, message2, field1, parent_fields); - if (fieldDifferent) { - if (reporter_ == NULL) return false; - isDifferent = true; - } } else { fieldDifferent = !CompareFieldValueUsingParentFields( message1, message2, field1, -1, -1, parent_fields); - // If we have found differences, either report them or terminate if - // no reporter is present. - if (fieldDifferent && reporter_ == NULL) { - return false; - } - - if (reporter_ != NULL) { + if (reporter_ != nullptr) { SpecificField specific_field; specific_field.field = field1; parent_fields->push_back(specific_field); @@ -860,6 +851,10 @@ bool MessageDifferencer::CompareWithFieldsInternal( parent_fields->pop_back(); } } + if (fieldDifferent) { + if (reporter_ == nullptr) return false; + isDifferent = true; + } // Increment the field indices. ++field_index1; ++field_index2; @@ -1002,17 +997,19 @@ bool MessageDifferencer::CompareMapFieldByMapReflection( return true; } -bool MessageDifferencer::CompareRepeatedField( +bool MessageDifferencer::CompareMapField( const Message& message1, const Message& message2, const FieldDescriptor* repeated_field, std::vector* parent_fields) { + GOOGLE_DCHECK(repeated_field->is_map()); + // the input FieldDescriptor is guaranteed to be repeated field. const Reflection* reflection1 = message1.GetReflection(); const Reflection* reflection2 = message2.GetReflection(); // When both map fields are on map, do not sync to repeated field. // TODO(jieluo): Add support for reporter - if (repeated_field->is_map() && reporter_ == nullptr && + if (reporter_ == nullptr && // Users didn't set custom map field key comparator map_field_key_comparator_.find(repeated_field) == map_field_key_comparator_.end() && @@ -1052,6 +1049,26 @@ bool MessageDifferencer::CompareRepeatedField( } } + return CompareRepeatedRep(message1, message2, repeated_field, parent_fields); +} + +bool MessageDifferencer::CompareRepeatedField( + const Message& message1, const Message& message2, + const FieldDescriptor* repeated_field, + std::vector* parent_fields) { + GOOGLE_DCHECK(!repeated_field->is_map()); + return CompareRepeatedRep(message1, message2, repeated_field, parent_fields); +} + +bool MessageDifferencer::CompareRepeatedRep( + const Message& message1, const Message& message2, + const FieldDescriptor* repeated_field, + std::vector* parent_fields) { + // the input FieldDescriptor is guaranteed to be repeated field. + GOOGLE_DCHECK(repeated_field->is_repeated()); + const Reflection* reflection1 = message1.GetReflection(); + const Reflection* reflection2 = message2.GetReflection(); + const int count1 = reflection1->FieldSize(message1, repeated_field); const int count2 = reflection2->FieldSize(message2, repeated_field); const bool treated_as_subset = IsTreatedAsSubset(repeated_field); diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index 57588b9e3d..943a0db2d3 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h @@ -779,7 +779,20 @@ class PROTOBUF_EXPORT MessageDifferencer { const FieldDescriptor* field, std::vector* parent_fields); - // Compare the map fields using map reflection instead of sync to repeated. + // Compares map fields, and report the error. + bool CompareMapField(const Message& message1, const Message& message2, + const FieldDescriptor* field, + std::vector* parent_fields); + + // Helper for CompareRepeatedField and CompareMapField: compares and reports + // differences element-wise. This is the implementation for non-map fields, + // and can also compare map fields by using the underlying representation. + bool CompareRepeatedRep(const Message& message1, const Message& message2, + const FieldDescriptor* field, + std::vector* parent_fields); + + // Helper for CompareMapField: compare the map fields using map reflection + // instead of sync to repeated. bool CompareMapFieldByMapReflection(const Message& message1, const Message& message2, const FieldDescriptor* field,