Skip to content

Commit bbeea7a

Browse files
sbenzaquencopybara-github
authored andcommitted
Fix error generation for very large inputs in TextFormat::Parser.
PiperOrigin-RevId: 952895478
1 parent 6a31b74 commit bbeea7a

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) {
@@ -1958,23 +1965,19 @@ TextFormat::Parser::Parser()
19581965
allow_singular_overwrites_(false),
19591966
recursion_limit_(kDefaultRecursionLimit) {}
19601967

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

1976-
} // namespace
1977-
19781981
bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
19791982
Message* output) {
19801983
output->Clear();
@@ -1994,14 +1997,14 @@ bool TextFormat::Parser::Parse(io::ZeroCopyInputStream* input,
19941997

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

20022005
bool TextFormat::Parser::ParseFromCord(const absl::Cord& input,
20032006
Message* output) {
2004-
DO(CheckParseInputSize(input, error_collector_));
2007+
DO(CheckParseInputSize(input, output));
20052008
io::CordInputStream input_stream(&input);
20062009
return Parse(&input_stream, output);
20072010
}
@@ -2019,7 +2022,7 @@ bool TextFormat::Parser::Merge(io::ZeroCopyInputStream* input,
20192022

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

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)