Rewrite FMLexer to use nullptr for errors

This CL rewrites how FMLexer returns errors, instead of having a flag
that gets flipped and needs to be checked, it now returns nullptr for
NextToken() when an error occurs. The Lexer's behaviour has also been
changed to only return nullptr once an error has occurred, instead of
advancing the lexing on further calls.

FMParse now checks the returned value from the lexer instead of
testing the error flag on the parser object. For any operation that
might cause the error state of the parser to change, i.e. consuming a
token, an error check has been added. In the event this check fails
the related function returns nullptr. This will cause the parse to
short circuit and exit.

BUG=pdfium:814

Change-Id: I669012c4732c18d13009be7cd7bf1ae682950904
Reviewed-on: https://pdfium-review.googlesource.com/8950
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: (OOO Jul 28 - Aug 8) dsinclair <dsinclair@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

1 files changed, 14 insertions, 30 deletions

diff --git a/xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp b/xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp
index 92b8fa93b3..a6ab871b18 100644
--- a/xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmlexer_unittest.cpp
@@ -12,13 +12,13 @@
 
 TEST(CXFA_FMLexerTest, EmptyString) {
   CXFA_FMLexer lexer(L"");
-  CXFA_FMToken* token = lexer.NextToken();
+  std::unique_ptr<CXFA_FMToken> token = lexer.NextToken();
   EXPECT_EQ(TOKeof, token->m_type);
 }
 
 TEST(CXFA_FMLexerTest, Numbers) {
   auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"-12");
-  CXFA_FMToken* token = lexer->NextToken();
+  std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
   // TODO(dsinclair): Should this return -12 instead of two tokens?
   EXPECT_EQ(TOKminus, token->m_type);
   token = lexer->NextToken();
@@ -66,7 +66,7 @@ TEST(CXFA_FMLexerTest, Numbers) {
 TEST(CXFA_FMLexerTest, Strings) {
   auto lexer =
       pdfium::MakeUnique<CXFA_FMLexer>(L"\"The cat jumped over the fence.\"");
-  CXFA_FMToken* token = lexer->NextToken();
+  std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
   EXPECT_EQ(TOKstring, token->m_type);
   EXPECT_EQ(L"\"The cat jumped over the fence.\"", token->m_string);
 
@@ -161,14 +161,14 @@ TEST(CXFA_FMLexerTest, OperatorsAndKeywords) {
 
   for (size_t i = 0; i < FX_ArraySize(op); ++i) {
     auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(op[i].op);
-    CXFA_FMToken* token = lexer->NextToken();
+    std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
     EXPECT_EQ(op[i].token, token->m_type);
   }
 }
 
 TEST(CXFA_FMLexerTest, Comments) {
   auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"// Empty.");
-  CXFA_FMToken* token = lexer->NextToken();
+  std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
   EXPECT_EQ(TOKeof, token->m_type);
 
   lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"//");
@@ -213,7 +213,7 @@ TEST(CXFA_FMLexerTest, ValidIdentifiers) {
       L"a", L"an_identifier", L"_ident", L"$ident", L"!ident", L"GetAddr"};
   for (const auto* ident : identifiers) {
     auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(ident);
-    CXFA_FMToken* token = lexer->NextToken();
+    std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
     EXPECT_EQ(TOKidentifier, token->m_type);
     EXPECT_EQ(ident, token->m_string);
   }
@@ -221,40 +221,24 @@ TEST(CXFA_FMLexerTest, ValidIdentifiers) {
 
 TEST(CXFA_FMLexerTest, InvalidIdentifiers) {
   auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"#a");
-  lexer->NextToken();
-  // TODO(rharrison): Add an expects for the return being nullptr here.
-  // See https://crbug.com/pdfium/814
-  EXPECT_TRUE(lexer->HasError());
+  EXPECT_EQ(nullptr, lexer->NextToken());
 
   lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"1a");
-  lexer->NextToken();
-  // TODO(rharrison): Add an expects for the return being nullptr here.
-  // See https://crbug.com/pdfium/814
-  EXPECT_TRUE(lexer->HasError());
+  EXPECT_EQ(nullptr, lexer->NextToken());
 
   lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"an@identifier");
-  lexer->NextToken();
-  EXPECT_FALSE(lexer->HasError());
-  lexer->NextToken();
-  // TODO(rharrison): Add an expects for the return being nullptr here.
-  // See https://crbug.com/pdfium/814
-  EXPECT_TRUE(lexer->HasError());
-  // TODO(rharrison): Add a test for if an another call to NextToken occurs,
-  // the error state will be retained, instead of continuing the parse.
-  // See https://crbug.com/pdfium/814
+  EXPECT_NE(nullptr, lexer->NextToken());
+  EXPECT_EQ(nullptr, lexer->NextToken());
+  EXPECT_EQ(nullptr, lexer->NextToken());
 
   lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"_ident@");
-  lexer->NextToken();
-  EXPECT_FALSE(lexer->HasError());
-  lexer->NextToken();
-  // TODO(rharrison): Add an expects for the return being nullptr here.
-  // See https://crbug.com/pdfium/814
-  EXPECT_TRUE(lexer->HasError());
+  EXPECT_NE(nullptr, lexer->NextToken());
+  EXPECT_EQ(nullptr, lexer->NextToken());
 }
 
 TEST(CXFA_FMLexerTest, Whitespace) {
   auto lexer = pdfium::MakeUnique<CXFA_FMLexer>(L" \t\xc\x9\xb");
-  CXFA_FMToken* token = lexer->NextToken();
+  std::unique_ptr<CXFA_FMToken> token = lexer->NextToken();
   EXPECT_EQ(TOKeof, token->m_type);
 
   lexer = pdfium::MakeUnique<CXFA_FMLexer>(L"123 \t\xc\x9\xb 456");