diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index 9a443c7c99..86a4adc7d1 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -1638,70 +1638,69 @@ class Parser { Parser(LineConsumer* line_consumer) : line_consumer_(line_consumer), line_(0) {} - // Parses a check of input, returning success/failure. - bool ParseChunk(StringPiece chunk); + // Feeds in some input, prase what it can, returning success/failure. Calling + // again after an error is undefined. + bool ParseChunk(StringPiece chunk, std::string* out_error); // Should be called to finish parsing (after all input has been provided via - // ParseChunk()). Returns success/failure. - bool Finish(); + // successful calls to ParseChunk(), calling after a ParseChunk() failure is + // undefined). Returns success/failure. + bool Finish(std::string* out_error); int last_line() const { return line_; } - std::string error_str() const { return error_str_; } private: - bool ParseLoop(); - LineConsumer* line_consumer_; int line_; - std::string error_str_; - StringPiece p_; std::string leftover_; }; -bool Parser::ParseChunk(StringPiece chunk) { +bool Parser::ParseChunk(StringPiece chunk, std::string* out_error) { + StringPiece full_chunk; if (!leftover_.empty()) { leftover_ += std::string(chunk); - p_ = StringPiece(leftover_); + full_chunk = StringPiece(leftover_); } else { - p_ = chunk; + full_chunk = chunk; + } + + StringPiece line; + while (ReadLine(&full_chunk, &line)) { + ++line_; + RemoveComment(&line); + TrimWhitespace(&line); + if (!line.empty() && !line_consumer_->ConsumeLine(line, out_error)) { + if (out_error->empty()) { + *out_error = "ConsumeLine failed without setting an error."; + } + leftover_.clear(); + return false; + } } - bool result = ParseLoop(); - if (p_.empty()) { + + if (full_chunk.empty()) { leftover_.clear(); } else { - leftover_ = std::string(p_); + leftover_ = std::string(full_chunk); } - return result; + return true; } -bool Parser::Finish() { +bool Parser::Finish(std::string* out_error) { // If there is still something to go, flush it with a newline. - if (!leftover_.empty() && !ParseChunk("\n")) { + if (!leftover_.empty() && !ParseChunk("\n", out_error)) { return false; } // This really should never fail if ParseChunk succeeded, but check to be sure. - return leftover_.empty(); -} - -bool Parser::ParseLoop() { - StringPiece line; - while (ReadLine(&p_, &line)) { - ++line_; - RemoveComment(&line); - TrimWhitespace(&line); - if (line.empty()) { - continue; // Blank line. - } - if (!line_consumer_->ConsumeLine(line, &error_str_)) { - return false; - } + if (!leftover_.empty()) { + *out_error = "ParseSimple Internal error: finished with pending data."; + return false; } return true; } -std::string ParserErrorString(const Parser& parser, const std::string& name) { - return std::string("error: ") + name + " Line " + - StrCat(parser.last_line()) + ", " + parser.error_str(); +std::string FullErrorString(const std::string& name, int line_num, const std::string& msg) { + return std::string("error: ") + name + " Line " + StrCat(line_num) + ", " + msg; } } // namespace @@ -1731,6 +1730,7 @@ bool ParseSimpleStream(io::ZeroCopyInputStream& input_stream, const std::string& stream_name, LineConsumer* line_consumer, std::string* out_error) { + std::string local_error; Parser parser(line_consumer); const void* buf; int buf_len; @@ -1739,13 +1739,14 @@ bool ParseSimpleStream(io::ZeroCopyInputStream& input_stream, continue; } - if (!parser.ParseChunk(StringPiece(static_cast(buf), buf_len))) { - *out_error = ParserErrorString(parser, stream_name); + if (!parser.ParseChunk(StringPiece(static_cast(buf), buf_len), + &local_error)) { + *out_error = FullErrorString(stream_name, parser.last_line(), local_error); return false; } } - if (!parser.Finish()) { - *out_error = ParserErrorString(parser, stream_name); + if (!parser.Finish(&local_error)) { + *out_error = FullErrorString(stream_name, parser.last_line(), local_error); return false; } return true; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc index bfbdf5a8e6..6d4225d5d4 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers_unittest.cc @@ -246,13 +246,16 @@ TEST(ObjCHelperDeathTest, TextFormatDecodeData_Failures) { class TestLineCollector : public LineConsumer { public: TestLineCollector(std::vector* inout_lines, - const std::string* reject_line = nullptr) - : lines_(inout_lines), reject_(reject_line) {} + const std::string* reject_line = nullptr, + bool skip_msg = false) + : lines_(inout_lines), reject_(reject_line), skip_msg_(skip_msg) {} bool ConsumeLine(const StringPiece& line, std::string* out_error) override { if (reject_) { if (*reject_ == line) { - *out_error = std::string("Rejected '") + *reject_ + "'"; + if (!skip_msg_) { + *out_error = std::string("Rejected '") + *reject_ + "'"; + } return false; } } @@ -265,6 +268,7 @@ class TestLineCollector : public LineConsumer { private: std::vector* lines_; const std::string* reject_; + bool skip_msg_; }; const int kBlockSizes[] = {-1, 1, 2, 5, 64}; @@ -325,10 +329,10 @@ TEST(ObjCHelper, ParseSimple_DropsComments) { TEST(ObjCHelper, ParseSimple_RejectLines) { const std::vector> tests = { - {"a\nb\nc", "a", 1}, - {"a\nb\nc", "b", 2}, - {"a\nb\nc", "c", 3}, - {"a\nb\nc\n", "c", 3}, + std::make_tuple("a\nb\nc", "a", 1), + std::make_tuple("a\nb\nc", "b", 2), + std::make_tuple("a\nb\nc", "c", 3), + std::make_tuple("a\nb\nc\n", "c", 3), }; for (const auto& test : tests) { @@ -338,7 +342,31 @@ TEST(ObjCHelper, ParseSimple_RejectLines) { std::string err_str; TestLineCollector collector(nullptr, &std::get<1>(test)); EXPECT_FALSE(ParseSimpleStream(input, "dummy", &collector, &err_str)); - std::string expected_err = StrCat("error: dummy Line ", std::get<2>(test), ", Rejected '", std::get<1>(test), "'"); + std::string expected_err = + StrCat("error: dummy Line ", std::get<2>(test), ", Rejected '", std::get<1>(test), "'"); + EXPECT_EQ(err_str, expected_err); + } + } +} + +TEST(ObjCHelper, ParseSimple_RejectLinesNoMessage) { + const std::vector> tests = { + std::make_tuple("a\nb\nc", "a", 1), + std::make_tuple("a\nb\nc", "b", 2), + std::make_tuple("a\nb\nc", "c", 3), + std::make_tuple("a\nb\nc\n", "c", 3), + }; + + for (const auto& test : tests) { + for (int i = 0; i < kBlockSizeCount; i++) { + io::ArrayInputStream input(std::get<0>(test).data(), std::get<0>(test).size(), + kBlockSizes[i]); + std::string err_str; + TestLineCollector collector(nullptr, &std::get<1>(test), true /* skip msg */); + EXPECT_FALSE(ParseSimpleStream(input, "dummy", &collector, &err_str)); + std::string expected_err = + StrCat("error: dummy Line ", std::get<2>(test), + ", ConsumeLine failed without setting an error."); EXPECT_EQ(err_str, expected_err); } }