Skip to content

Commit

Permalink
Compare explicit zeroes from prototext in partially.
Browse files Browse the repository at this point in the history 
Right now, zeros are not allowed in explicit prototext comparison, frequently leading to very awkward manual checking. This tracks such explicit zeros, and ensures they remain 0.

PiperOrigin-RevId: 580729313
  • Loading branch information
@protobuf-github-bot
protobuf-github-bot authored and Copybara-Service committed Nov 9, 2023
1 parent 5987c89 commit 8c24163
Show file tree
Hide file tree
Showing 8 changed files with 518 additions and 42 deletions.
2 changes: 2 additions & 0 deletions src/google/protobuf/message.h
View file
Expand Up @@ -135,6 +135,7 @@ class MapValueConstRef;
class MapValueRef;
class MapIterator;
class MapReflectionTester;
class TextFormat;

namespace internal {
struct FuzzPeer;
Expand Down Expand Up @@ -1010,6 +1011,7 @@ class PROTOBUF_EXPORT Reflection final {
return schema_.IsSplit(field);
}

friend class google::protobuf::TextFormat;
friend class FastReflectionBase;
friend class FastReflectionMessageMutator;
friend bool internal::IsDescendant(Message& root, const Message& message);
Expand Down
48 changes: 29 additions & 19 deletions src/google/protobuf/text_format.cc
View file
Expand Up @@ -295,6 +295,14 @@ const Descriptor* DefaultFinderFindAnyType(const Message& message,
}
} // namespace

const void* TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldAddress(
const Message& message, const Reflection& reflection,
const FieldDescriptor& fd) {
// reflection->GetRaw() is a simple cast for any non-repeated type, so for
// simplicity we just pass in char as the template argument.
return &reflection.GetRaw<char>(message, &fd);
}

// ===========================================================================
// Internal class for parsing an ASCII representation of a Protocol Message.
// This class makes use of the Protocol Message compiler's tokenizer found
Expand Down Expand Up @@ -331,7 +339,7 @@ class TextFormat::Parser::ParserImpl {
bool allow_unknown_extension, bool allow_unknown_enum,
bool allow_field_number, bool allow_relaxed_whitespace,
bool allow_partial, int recursion_limit,
bool error_on_no_op_fields)
UnsetFieldsMetadata* no_op_fields)
: error_collector_(error_collector),
finder_(finder),
parse_info_tree_(parse_info_tree),
Expand All @@ -349,7 +357,7 @@ class TextFormat::Parser::ParserImpl {
recursion_limit_(recursion_limit),
had_silent_marker_(false),
had_errors_(false),
error_on_no_op_fields_(error_on_no_op_fields) {
no_op_fields_(no_op_fields) {
// For backwards-compatibility with proto1, we need to allow the 'f' suffix
// for floats.
tokenizer_.set_allow_f_after_float(true);
Expand Down Expand Up @@ -834,19 +842,21 @@ class TextFormat::Parser::ParserImpl {
// When checking for no-op operations, We verify that both the existing value in
// the message and the new value are the default. If the existing field value is
// not the default, setting it to the default should not be treated as a no-op.
#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE) \
if (field->is_repeated()) { \
reflection->Add##CPPTYPE(message, field, VALUE); \
} else { \
if (error_on_no_op_fields_ && !field->has_presence() && \
field->default_value_##CPPTYPELCASE() == \
reflection->Get##CPPTYPE(*message, field) && \
field->default_value_##CPPTYPELCASE() == VALUE) { \
ReportError("Input field " + field->full_name() + \
" did not change resulting proto."); \
} else { \
reflection->Set##CPPTYPE(message, field, std::move(VALUE)); \
} \
// The pointer of this is kept in no_op_fields_ for bookkeeping.
#define SET_FIELD(CPPTYPE, CPPTYPELCASE, VALUE) \
if (field->is_repeated()) { \
reflection->Add##CPPTYPE(message, field, VALUE); \
} else { \
if (no_op_fields_ && !field->has_presence() && \
field->default_value_##CPPTYPELCASE() == \
reflection->Get##CPPTYPE(*message, field) && \
field->default_value_##CPPTYPELCASE() == VALUE) { \
no_op_fields_->addresses_.insert( \
UnsetFieldsMetadata::GetUnsetFieldAddress(*message, *reflection, \
*field)); \
} else { \
reflection->Set##CPPTYPE(message, field, std::move(VALUE)); \
} \
}

switch (field->cpp_type()) {
Expand Down Expand Up @@ -1429,7 +1439,7 @@ class TextFormat::Parser::ParserImpl {
int recursion_limit_;
bool had_silent_marker_;
bool had_errors_;
bool error_on_no_op_fields_;
UnsetFieldsMetadata* no_op_fields_{};

};

Expand Down Expand Up @@ -1727,7 +1737,7 @@ bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
allow_partial_, recursion_limit_, error_on_no_op_fields_);
allow_partial_, recursion_limit_, no_op_fields_);
return MergeUsingImpl(input, output, &parser);
}

Expand All @@ -1752,7 +1762,7 @@ bool TextFormat::Parser::Merge(io::ZeroCopyInputStream* input,
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
allow_partial_, recursion_limit_, error_on_no_op_fields_);
allow_partial_, recursion_limit_, no_op_fields_);
return MergeUsingImpl(input, output, &parser);
}

Expand Down Expand Up @@ -1788,7 +1798,7 @@ bool TextFormat::Parser::ParseFieldValueFromString(absl::string_view input,
allow_case_insensitive_field_, allow_unknown_field_,
allow_unknown_extension_, allow_unknown_enum_,
allow_field_number_, allow_relaxed_whitespace_,
allow_partial_, recursion_limit_, error_on_no_op_fields_);
allow_partial_, recursion_limit_, no_op_fields_);
return parser.ParseField(field, output);
}

Expand Down
43 changes: 38 additions & 5 deletions src/google/protobuf/text_format.h
View file
Expand Up @@ -21,6 +21,7 @@
#include <vector>

#include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h"
#include "absl/strings/cord.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/descriptor.h"
Expand Down Expand Up @@ -81,6 +82,9 @@ PROTOBUF_EXPORT enum class Option;
// Converts a protobuf message to a string with redaction enabled.
PROTOBUF_EXPORT std::string StringifyMessage(const Message& message,
Option option);

class UnsetFieldsMetadataTextFormatTestUtil;
class UnsetFieldsMetadataMessageDifferencerTestUtil;
} // namespace internal

// This class implements protocol buffer text format, colloquially known as text
Expand Down Expand Up @@ -719,11 +723,40 @@ class PROTOBUF_EXPORT TextFormat {
// the maximum allowed nesting of proto messages.
void SetRecursionLimit(int limit) { recursion_limit_ = limit; }

// If called, the parser will report an error if a parsed field had no
// Uniquely addresses fields in a message that was explicitly unset in
// textproto. Example:
// "some_int_field: 0"
// where some_int_field is non-optional.
//
// This class should only be used to pass data between the text_format
// parser and the MessageDifferencer.
class UnsetFieldsMetadata {
public:
UnsetFieldsMetadata() = default;

private:
// Return a pointer to the unset field in the given message.
static const void* GetUnsetFieldAddress(const Message& message,
const Reflection& reflection,
const FieldDescriptor& fd);

// List of addresses of explicitly unset proto fields.
absl::flat_hash_set<const void*> addresses_;

friend class ::google::protobuf::internal::
UnsetFieldsMetadataMessageDifferencerTestUtil;
friend class ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
friend class ::google::protobuf::util::MessageDifferencer;
friend class ::google::protobuf::TextFormat::Parser;
};

// If called, the parser will report the parsed fields that had no
// effect on the resulting proto (for example, fields with no presence that
// were set to their default value).
void ErrorOnNoOpFields(bool return_error) {
error_on_no_op_fields_ = return_error;
// were set to their default value). These can be passed to the Partially()
// matcher as an indicator to explicitly check these fields are missing
// in the actual.
void OutputNoOpFields(UnsetFieldsMetadata* no_op_fields) {
no_op_fields_ = no_op_fields;
}

private:
Expand All @@ -748,7 +781,7 @@ class PROTOBUF_EXPORT TextFormat {
bool allow_relaxed_whitespace_;
bool allow_singular_overwrites_;
int recursion_limit_;
bool error_on_no_op_fields_ = false;
UnsetFieldsMetadata* no_op_fields_ = nullptr;
};


Expand Down

0 comments on commit 8c24163

Please sign in to comment.