Merge pull request #8444 from perezd/sync-stage

Integrate from Piper for C++, Java, and Python
This commit is contained in:
Derek Perez 2021-04-02 11:22:40 -07:00 committed by GitHub
commit 97cb3a862f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 149 additions and 83 deletions

View File

@ -22,8 +22,11 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript)
on an error path.
* Avoid expensive inlined code space for encoding message length for messages
>= 128 bytes and instead do a procedure call to a shared out-of-line routine.
* util::DefaultFieldComparator will be final in a future version of protobuf.
Subclasses should inherit from SimpleFieldComparator instead.
Java:
* Detect invalid overflow of byteLimit and return InvalidProtocolBufferException as documented.
* Exceptions thrown while reading from an InputStream in parseFrom are now
included as causes.
* Support potentially more efficient proto parsing from RopeByteStrings.

View File

@ -4796,11 +4796,11 @@ namespace Google.Protobuf.Reflection {
private string javaOuterClassname_;
/// <summary>
/// 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.
/// </summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
public string JavaOuterClassname {
@ -4826,10 +4826,10 @@ namespace Google.Protobuf.Reflection {
private bool javaMultipleFiles_;
/// <summary>
/// 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.
/// </summary>

View File

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

View File

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

View File

@ -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 <code>optional string java_outer_classname = 8;</code>
*/
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 <code>optional string java_outer_classname = 8;</code>
* @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 <code>optional string java_outer_classname = 8;</code>
* @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.
*

View File

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

View File

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

View File

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

View File

@ -148,18 +148,16 @@ class MessageDifferencer::MultipleFieldsMapKeyComparator
const FieldDescriptor* field = key_field_path[path_index];
std::vector<SpecificField> current_parent_fields(parent_fields);
if (path_index == static_cast<int64_t>(key_field_path.size() - 1)) {
if (field->is_repeated()) {
if (!message_differencer_->CompareRepeatedField(
message1, message2, field, &current_parent_fields)) {
return false;
}
if (field->is_map()) {
return message_differencer_->CompareMapField(message1, message2, field,
&current_parent_fields);
} else if (field->is_repeated()) {
return message_differencer_->CompareRepeatedField(
message1, message2, field, &current_parent_fields);
} else {
if (!message_differencer_->CompareFieldValueUsingParentFields(
message1, message2, field, -1, -1, &current_parent_fields)) {
return false;
}
return message_differencer_->CompareFieldValueUsingParentFields(
message1, message2, field, -1, -1, &current_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<SpecificField>* 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<SpecificField>* 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<SpecificField>* 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);

View File

@ -779,7 +779,20 @@ class PROTOBUF_EXPORT MessageDifferencer {
const FieldDescriptor* field,
std::vector<SpecificField>* 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<SpecificField>* 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<SpecificField>* 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,