Skip to content

Commit 8080b02

Browse files
sbenzaquencopybara-github
authored andcommitted
Fix error generation for very large inputs in TextFormat::Parser.
PiperOrigin-RevId: 953364578
1 parent 732618d commit 8080b02

3 files changed

Lines changed: 90 additions & 25 deletions

File tree

src/google/protobuf/text_format.cc

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,24 @@ const Descriptor* DefaultFinderFindAnyType(const Message& message,
337337
const std::string& name) {
338338
return message.GetDescriptor()->file()->pool()->FindMessageTypeByName(name);
339339
}
340+
341+
void ReportErrorImpl(int line, int col, absl::string_view message,
342+
const Descriptor* root_message_type,
343+
io::ErrorCollector* error_collector) {
344+
if (error_collector == nullptr) {
345+
if (line >= 0) {
346+
ABSL_LOG(ERROR) << "Error parsing text-format "
347+
<< root_message_type->full_name() << ": " << (line + 1)
348+
<< ":" << (col + 1) << ": " << message;
349+
} else {
350+
ABSL_LOG(ERROR) << "Error parsing text-format "
351+
<< root_message_type->full_name() << ": " << message;
352+
}
353+
} else {
354+
error_collector->RecordError(line, col, message);
355+
}
356+
}
357+
340358
} // namespace
341359

342360
auto TextFormat::Parser::UnsetFieldsMetadata::GetUnsetFieldId(
@@ -454,18 +472,7 @@ class TextFormat::Parser::ParserImpl {
454472

455473
void ReportError(int line, int col, absl::string_view message) {
456474
had_errors_ = true;
457-
if (error_collector_ == nullptr) {
458-
if (line >= 0) {
459-
ABSL_LOG(ERROR) << "Error parsing text-format "
460-
<< root_message_type_->full_name() << ": " << (line + 1)
461-
<< ":" << (col + 1) << ": " << message;
462-
} else {
463-
ABSL_LOG(ERROR) << "Error parsing text-format "
464-
<< root_message_type_->full_name() << ": " << message;
465-
}
466-
} else {
467-
error_collector_->RecordError(line, col, message);
468-
}
475+
ReportErrorImpl(line, col, message, root_message_type_, error_collector_);
469476
}
470477

471478
void ReportWarning(int line, int col, const absl::string_view message) {
@@ -1959,23 +1966,19 @@ TextFormat::Parser::Parser()
19591966
allow_singular_overwrites_(false),
19601967
recursion_limit_(kDefaultRecursionLimit) {}
19611968

1962-
namespace {
1963-
19641969
template <typename T>
1965-
bool CheckParseInputSize(T& input, io::ErrorCollector* error_collector) {
1970+
bool TextFormat::Parser::CheckParseInputSize(T& input, Message* output) const {
19661971
if (input.size() > INT_MAX) {
1967-
error_collector->RecordError(
1968-
-1, 0,
1969-
absl::StrCat(
1970-
"Input size too large: ", static_cast<int64_t>(input.size()),
1971-
" bytes", " > ", INT_MAX, " bytes."));
1972+
ReportErrorImpl(-1, 0,
1973+
absl::StrCat("Input size too large: ",
1974+
static_cast<int64_t>(input.size()), " bytes",
1975+
" > ", INT_MAX, " bytes."),
1976+
output->GetDescriptor(), error_collector_);
19721977
return false;
19731978
}
19741979
return true;
19751980
}
19761981

1977-
} // namespace
1978-
19791982
bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
19801983
Message* output) {
19811984
output->Clear();
@@ -1995,14 +1998,14 @@ bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
19951998

19961999
bool TextFormat::Parser::ParseFromString(absl::string_view input,
19972000
Message* output) {
1998-
DO(CheckParseInputSize(input, error_collector_));
2001+
DO(CheckParseInputSize(input, output));
19992002
io::ArrayInputStream input_stream(input.data(), input.size());
20002003
return Parse(&input_stream, output);
20012004
}
20022005

20032006
bool TextFormat::Parser::ParseFromCord(const absl::Cord& input,
20042007
Message* output) {
2005-
DO(CheckParseInputSize(input, error_collector_));
2008+
DO(CheckParseInputSize(input, output));
20062009
io::CordInputStream input_stream(&input);
20072010
return Parse(&input_stream, output);
20082011
}
@@ -2020,7 +2023,7 @@ bool TextFormat::Parser::Merge(io::ZeroCopyInputStream* input,
20202023

20212024
bool TextFormat::Parser::MergeFromString(absl::string_view input,
20222025
Message* output) {
2023-
DO(CheckParseInputSize(input, error_collector_));
2026+
DO(CheckParseInputSize(input, output));
20242027
io::ArrayInputStream input_stream(input.data(), input.size());
20252028
return Merge(&input_stream, output);
20262029
}

src/google/protobuf/text_format.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,9 @@ class PROTOBUF_EXPORT TextFormat {
863863
// representations (see text_format.cc for implementation).
864864
class ParserImpl;
865865

866+
template <typename T>
867+
bool CheckParseInputSize(T& input, Message* output) const;
868+
866869
// Like TextFormat::Merge(). The provided implementation is used
867870
// to do the parsing.
868871
bool MergeUsingImpl(io::ZeroCopyInputStream* input, Message* output,

src/google/protobuf/text_format_unittest.cc

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ namespace text_format_unittest {
8282
using ::absl_testing::IsOk;
8383
using ::absl_testing::StatusIs;
8484
using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
85+
using ::testing::_;
8586
using ::testing::AllOf;
8687
using ::testing::HasSubstr;
8788
using ::testing::UnorderedElementsAre;
@@ -295,6 +296,64 @@ TEST_F(TextFormatTest, ShortFormat) {
295296
value_replacement, kTextMarkerRegex)));
296297
}
297298

299+
TEST_F(TextFormatTest, InputTooLarge) {
300+
if (sizeof(size_t) <= sizeof(int)) {
301+
GTEST_SKIP() << "Not supported in platform.";
302+
}
303+
unittest::TestAllTypes msg;
304+
305+
constexpr absl::string_view kError = "Input size too large";
306+
307+
const auto expect_error = [&](auto f) {
308+
{
309+
// First without a collector.
310+
TextFormat::Parser parser;
311+
absl::ScopedMockLog log(absl::MockLogDefault::kDisallowUnexpected);
312+
EXPECT_CALL(log, Log(absl::LogSeverity::kError, _, HasSubstr(kError)))
313+
.Times(1);
314+
log.StartCapturingLogs();
315+
EXPECT_FALSE(f(parser));
316+
}
317+
318+
// Then with a collector.
319+
TextFormat::Parser parser;
320+
class MockErrorCollector : public io::ErrorCollector {
321+
public:
322+
MockErrorCollector() = default;
323+
~MockErrorCollector() override = default;
324+
325+
std::string text_;
326+
327+
// implements ErrorCollector -------------------------------------
328+
void RecordError(int line, int column,
329+
absl::string_view message) override {
330+
text_ = absl::StrCat(message);
331+
}
332+
333+
void RecordWarning(int line, int column,
334+
absl::string_view message) override {}
335+
};
336+
337+
MockErrorCollector error_collector;
338+
parser.RecordErrorsTo(&error_collector);
339+
EXPECT_FALSE(f(parser));
340+
EXPECT_THAT(error_collector.text_, HasSubstr(kError));
341+
};
342+
343+
// Use a fake string_view with very large size.
344+
// The contents don't matter because it should fail just by the size.
345+
const absl::string_view too_large_sv(
346+
"asdf", size_t{std::numeric_limits<int>::max()} + 1);
347+
expect_error([&](auto& p) { return p.ParseFromString(too_large_sv, &msg); });
348+
expect_error([&](auto& p) { return p.MergeFromString(too_large_sv, &msg); });
349+
350+
absl::Cord too_large_cord("sdaf");
351+
while (too_large_cord.size() < std::numeric_limits<int>::max()) {
352+
too_large_cord.Append(too_large_cord);
353+
}
354+
expect_error([&](auto& p) { return p.ParseFromCord(too_large_cord, &msg); });
355+
}
356+
298357
TEST_F(TextFormatTest, Utf8Format) {
299358
unittest::RedactedFields proto;
300359
unittest::TestNestedMessageRedaction redacted_nested_proto;

0 commit comments

Comments
 (0)