Clean up CPDF_SyntaxParser a little bit

core/fpdfapi/parser/cpdf_syntax_parser.cpp

 // Copyright 2016 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdfapi/parser/cpdf_syntax_parser.h"
 
 #include <vector>
 
 #include "core/fpdfapi/cpdf_modulemgr.h"
 #include "core/fpdfapi/parser/cpdf_array.h"
 #include "core/fpdfapi/parser/cpdf_boolean.h"
 #include "core/fpdfapi/parser/cpdf_crypto_handler.h"
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_name.h"
 #include "core/fpdfapi/parser/cpdf_null.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
 #include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_string.h"
 #include "core/fpdfapi/parser/fpdf_parser_decode.h"
 #include "core/fpdfapi/parser/fpdf_parser_utility.h"
 #include "core/fxcrt/fx_ext.h"
 #include "third_party/base/numerics/safe_math.h"
 
 namespace {
 
 struct SearchTagRecord {

[Lei Zhang, 2016/11/03 08:11:27]

No longer needed?

[npm, 2016/11/03 18:58:56]

Done.

   CFX_ByteStringC m_bsTag;
   FX_STRSIZE m_Offset;
 };
 
+enum class ReadStatus { Normal, Backslash, Octal, FinishOctal, CarriageReturn };
+
 }  // namespace
 
 // static
 int CPDF_SyntaxParser::s_CurrentRecursionDepth = 0;
 
 CPDF_SyntaxParser::CPDF_SyntaxParser()
     : CPDF_SyntaxParser(CFX_WeakPtr<CFX_ByteStringPool>()) {}
 
 CPDF_SyntaxParser::CPDF_SyntaxParser(
     const CFX_WeakPtr<CFX_ByteStringPool>& pPool)
     : m_MetadataObjnum(0),
       m_pFileAccess(nullptr),
       m_pFileBuf(nullptr),
       m_BufSize(CPDF_ModuleMgr::kFileBufSize),
       m_pPool(pPool) {}
 
 CPDF_SyntaxParser::~CPDF_SyntaxParser() {
   FX_Free(m_pFileBuf);
 }
 
 bool CPDF_SyntaxParser::GetCharAt(FX_FILESIZE pos, uint8_t& ch) {
   CFX_AutoRestorer<FX_FILESIZE> save_pos(&m_Pos);
   m_Pos = pos;
   return GetNextChar(ch);
 }
 
+bool CPDF_SyntaxParser::ReadChar(FX_FILESIZE read_pos, uint32_t read_size) {
+  if (static_cast<FX_FILESIZE>(read_pos + read_size) > m_FileLen) {
+    if (m_FileLen < static_cast<FX_FILESIZE>(read_size)) {
+      read_pos = 0;
+      read_size = static_cast<uint32_t>(m_FileLen);
+    } else {
+      read_pos = m_FileLen - read_size;
+    }
+  }
+  if (!m_pFileAccess->ReadBlock(m_pFileBuf, read_pos, read_size))
+    return false;
+
+  m_BufOffset = read_pos;
+  return true;
+}
+
 bool CPDF_SyntaxParser::GetNextChar(uint8_t& ch) {
   FX_FILESIZE pos = m_Pos + m_HeaderOffset;
   if (pos >= m_FileLen)
     return false;
 
-  if (m_BufOffset >= pos || (FX_FILESIZE)(m_BufOffset + m_BufSize) <= pos) {
+  if (m_BufOffset >= pos ||

[Lei Zhang, 2016/11/03 08:11:27]

This same check is in 3 places.

[npm, 2016/11/03 18:58:56]

Replaced with inline method. Only found it in 2 places.

+      static_cast<FX_FILESIZE>(m_BufOffset + m_BufSize) <= pos) {
     FX_FILESIZE read_pos = pos;
     uint32_t read_size = m_BufSize;
-    if ((FX_FILESIZE)read_size > m_FileLen)
-      read_size = (uint32_t)m_FileLen;
-
-    if ((FX_FILESIZE)(read_pos + read_size) > m_FileLen) {
-      if (m_FileLen < (FX_FILESIZE)read_size) {
-        read_pos = 0;
-        read_size = (uint32_t)m_FileLen;
-      } else {
-        read_pos = m_FileLen - read_size;
-      }
-    }
-
-    if (!m_pFileAccess->ReadBlock(m_pFileBuf, read_pos, read_size))
+    if (static_cast<FX_FILESIZE>(read_size) > m_FileLen)

[Lei Zhang, 2016/11/03 08:11:27]

std::min()

[npm, 2016/11/03 18:58:56]

Done.

+      read_size = static_cast<uint32_t>(m_FileLen);
+    if (!ReadChar(read_pos, read_size))
       return false;
-
-    m_BufOffset = read_pos;
   }
   ch = m_pFileBuf[pos - m_BufOffset];
   m_Pos++;
   return true;
 }
 
 bool CPDF_SyntaxParser::GetCharAtBackward(FX_FILESIZE pos, uint8_t& ch) {
   pos += m_HeaderOffset;
   if (pos >= m_FileLen)
     return false;
 
-  if (m_BufOffset >= pos || (FX_FILESIZE)(m_BufOffset + m_BufSize) <= pos) {
+  if (m_BufOffset >= pos ||
+      static_cast<FX_FILESIZE>(m_BufOffset + m_BufSize) <= pos) {

[Lei Zhang, 2016/11/03 08:11:27]

For later, since it's pre-existing - I'm paranoid about integer overflows.

[npm, 2016/11/03 18:58:56]

Agreed. Might need to add more checks to operations done by parser.

     FX_FILESIZE read_pos;
-    if (pos < (FX_FILESIZE)m_BufSize)
+    if (pos < static_cast<FX_FILESIZE>(m_BufSize))
       read_pos = 0;
     else
       read_pos = pos - m_BufSize + 1;
-
     uint32_t read_size = m_BufSize;
-    if ((FX_FILESIZE)(read_pos + read_size) > m_FileLen) {
-      if (m_FileLen < (FX_FILESIZE)read_size) {
-        read_pos = 0;
-        read_size = (uint32_t)m_FileLen;
-      } else {
-        read_pos = m_FileLen - read_size;
-      }
-    }
-
-    if (!m_pFileAccess->ReadBlock(m_pFileBuf, read_pos, read_size))
+    if (!ReadChar(read_pos, read_size))
       return false;
-
-    m_BufOffset = read_pos;
   }
   ch = m_pFileBuf[pos - m_BufOffset];
   return true;
 }
 
 bool CPDF_SyntaxParser::ReadBlock(uint8_t* pBuf, uint32_t size) {
   if (!m_pFileAccess->ReadBlock(pBuf, m_Pos + m_HeaderOffset, size))
     return false;
   m_Pos += size;
   return true;
@@ skipping 82 matching lines @@
   }
 }
 
 CFX_ByteString CPDF_SyntaxParser::ReadString() {
   uint8_t ch;
   if (!GetNextChar(ch))
     return CFX_ByteString();
 
   CFX_ByteTextBuf buf;
   int32_t parlevel = 0;
-  int32_t status = 0;
+  ReadStatus status = ReadStatus::Normal;
   int32_t iEscCode = 0;
   while (1) {
     switch (status) {
-      case 0:
+      case ReadStatus::Normal:
         if (ch == ')') {
-          if (parlevel == 0) {
+          if (parlevel == 0)
             return buf.MakeString();
-          }
           parlevel--;
-          buf.AppendChar(')');
         } else if (ch == '(') {
           parlevel++;
-          buf.AppendChar('(');
-        } else if (ch == '\\') {
-          status = 1;
-        } else {
+        }
+        if (ch == '\\')
+          status = ReadStatus::Backslash;
+        else
           buf.AppendChar(ch);
-        }
         break;
-      case 1:
+      case ReadStatus::Backslash:
         if (ch >= '0' && ch <= '7') {
           iEscCode = FXSYS_toDecimalDigit(static_cast<FX_WCHAR>(ch));
-          status = 2;
+          status = ReadStatus::Octal;
           break;
         }
 
         if (ch == 'n') {
           buf.AppendChar('\n');
         } else if (ch == 'r') {
           buf.AppendChar('\r');
         } else if (ch == 't') {
           buf.AppendChar('\t');
         } else if (ch == 'b') {
           buf.AppendChar('\b');
         } else if (ch == 'f') {
           buf.AppendChar('\f');
         } else if (ch == '\r') {
-          status = 4;
+          status = ReadStatus::CarriageReturn;
           break;
         } else if (ch != '\n') {
           buf.AppendChar(ch);
         }
-        status = 0;
+        status = ReadStatus::Normal;
         break;
-      case 2:
+      case ReadStatus::Octal:
         if (ch >= '0' && ch <= '7') {
           iEscCode =
               iEscCode * 8 + FXSYS_toDecimalDigit(static_cast<FX_WCHAR>(ch));
-          status = 3;
+          status = ReadStatus::FinishOctal;
         } else {
           buf.AppendChar(iEscCode);
-          status = 0;
+          status = ReadStatus::Normal;
           continue;
         }
         break;
-      case 3:
+      case ReadStatus::FinishOctal:
+        status = ReadStatus::Normal;
         if (ch >= '0' && ch <= '7') {
           iEscCode =
               iEscCode * 8 + FXSYS_toDecimalDigit(static_cast<FX_WCHAR>(ch));
           buf.AppendChar(iEscCode);
-          status = 0;
         } else {
           buf.AppendChar(iEscCode);
-          status = 0;
           continue;
         }
         break;
-      case 4:
-        status = 0;
+      case ReadStatus::CarriageReturn:
+        status = ReadStatus::Normal;
         if (ch != '\n')
           continue;
         break;
     }
 
     if (!GetNextChar(ch))
       break;
   }
 
   GetNextChar(ch);
@@ skipping 343 matching lines @@
     len = pLenObj->GetInteger();
 
   // Locate the start of stream.
   ToNextLine();
   FX_FILESIZE streamStartPos = m_Pos;
 
   const CFX_ByteStringC kEndStreamStr("endstream");
   const CFX_ByteStringC kEndObjStr("endobj");
 
   CPDF_CryptoHandler* pCryptoHandler =
-      objnum == (uint32_t)m_MetadataObjnum ? nullptr : m_pCryptoHandler.get();
+      objnum == static_cast<uint32_t>(m_MetadataObjnum)

[Lei Zhang, 2016/11/03 08:11:27]

Can we just make |m_MetadataObjnum| unsigned?

[npm, 2016/11/03 18:58:56]

Done.

+          ? nullptr
+          : m_pCryptoHandler.get();
   if (!pCryptoHandler) {
     bool bSearchForKeyword = true;
     if (len >= 0) {
       pdfium::base::CheckedNumeric<FX_FILESIZE> pos = m_Pos;
       pos += len;
       if (pos.IsValid() && pos.ValueOrDie() < m_FileLen)
         m_Pos = pos.ValueOrDie();
 
       m_Pos += ReadEOLMarkers(m_Pos);
       FXSYS_memset(m_WordBuffer, 0, kEndStreamStr.GetLength() + 1);
@@ skipping 125 matching lines @@
   FX_Free(m_pFileBuf);
 
   m_pFileBuf = FX_Alloc(uint8_t, m_BufSize);
   m_HeaderOffset = HeaderOffset;
   m_FileLen = pFileAccess->GetSize();
   m_Pos = 0;
   m_pFileAccess = pFileAccess;
   m_BufOffset = 0;
   pFileAccess->ReadBlock(
       m_pFileBuf, 0,
-      (size_t)((FX_FILESIZE)m_BufSize > m_FileLen ? m_FileLen : m_BufSize));
+      static_cast<size_t>(static_cast<FX_FILESIZE>(m_BufSize) > m_FileLen

[Lei Zhang, 2016/11/03 08:11:27]

Something like:

std::min(m_BufSize, static_cast<uint32_t>(m_FileLen);

[npm, 2016/11/03 18:58:56]

Done.

+                              ? m_FileLen
+                              : m_BufSize));
 }
 
 uint32_t CPDF_SyntaxParser::GetDirectNum() {
   bool bIsNumber;
   GetNextWordInternal(&bIsNumber);
   if (!bIsNumber)
     return 0;
 
   m_WordBuffer[m_WordSize] = 0;
   return FXSYS_atoui(reinterpret_cast<const FX_CHAR*>(m_WordBuffer));
@@ skipping 90 matching lines @@
       pos--;
     }
 
     if (pos < 0)
       return false;
   }
 
   return false;
 }
 
-int32_t CPDF_SyntaxParser::SearchMultiWord(const CFX_ByteStringC& tags,
-                                           bool bWholeWord,
-                                           FX_FILESIZE limit) {
-  int32_t ntags = 1;
-  for (int i = 0; i < tags.GetLength(); ++i) {
-    if (tags[i] == 0)
-      ++ntags;
-  }
-
-  // Ensure that the input byte string happens to be nul-terminated. This
-  // need not be the case, but the loop below uses this guarantee to put
-  // the last pattern into the vector.
-  ASSERT(tags[tags.GetLength()] == 0);
-  std::vector<SearchTagRecord> patterns(ntags);
-  uint32_t start = 0;
-  uint32_t itag = 0;
-  uint32_t max_len = 0;
-  for (int i = 0; i <= tags.GetLength(); ++i) {
-    if (tags[i] == 0) {
-      uint32_t len = i - start;
-      max_len = std::max(len, max_len);
-      patterns[itag].m_bsTag = tags.Mid(start, len);
-      patterns[itag].m_Offset = 0;
-      start = i + 1;
-      ++itag;
-    }
-  }
-
-  const FX_FILESIZE pos_limit = m_Pos + limit;
-  for (FX_FILESIZE pos = m_Pos; !limit || pos < pos_limit; ++pos) {
-    uint8_t byte;
-    if (!GetCharAt(pos, byte))
-      break;
-
-    for (int i = 0; i < ntags; ++i) {
-      SearchTagRecord& pat = patterns[i];
-      if (pat.m_bsTag[pat.m_Offset] != byte) {
-        pat.m_Offset = (pat.m_bsTag[0] == byte) ? 1 : 0;
-        continue;
-      }
-
-      ++pat.m_Offset;
-      if (pat.m_Offset != pat.m_bsTag.GetLength())
-        continue;
-
-      if (!bWholeWord || IsWholeWord(pos - pat.m_bsTag.GetLength(), limit,
-                                     pat.m_bsTag, false)) {
-        return i;
-      }
-
-      pat.m_Offset = (pat.m_bsTag[0] == byte) ? 1 : 0;
-    }
-  }
-  return -1;
-}
-
 FX_FILESIZE CPDF_SyntaxParser::FindTag(const CFX_ByteStringC& tag,
                                        FX_FILESIZE limit) {
   int32_t taglen = tag.GetLength();
   int32_t match = 0;
   limit += m_Pos;
   FX_FILESIZE startpos = m_Pos;
 
   while (1) {
     uint8_t ch;
     if (!GetNextChar(ch))
@@ skipping 14 matching lines @@
 }
 
 void CPDF_SyntaxParser::SetEncrypt(
     std::unique_ptr<CPDF_CryptoHandler> pCryptoHandler) {
   m_pCryptoHandler = std::move(pCryptoHandler);
 }
 
 CFX_ByteString CPDF_SyntaxParser::MaybeIntern(const CFX_ByteString& str) {
   return m_pPool ? m_pPool->Intern(str) : str;
 }