Use unsigned page indexes in CPDF_HintTables.

Fix up callers from CPDF_DataAvail.

Review-Url: https://codereview.chromium.org/2294383003

4 files changed, 65 insertions, 58 deletions

diff --git a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
index 58b69257ad..b7395ee7a5 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
@@ -479,12 +479,12 @@ FX_BOOL CPDF_DataAvail::PreparePageItem() {
   return TRUE;
 }
 
-bool CPDF_DataAvail::IsFirstCheck(int iPage) {
-  return m_pageMapCheckState.insert(iPage).second;
+bool CPDF_DataAvail::IsFirstCheck(uint32_t dwPage) {
+  return m_pageMapCheckState.insert(dwPage).second;
 }
 
-void CPDF_DataAvail::ResetFirstCheck(int iPage) {
-  m_pageMapCheckState.erase(iPage);
+void CPDF_DataAvail::ResetFirstCheck(uint32_t dwPage) {
+  m_pageMapCheckState.erase(dwPage);
 }
 
 FX_BOOL CPDF_DataAvail::CheckPage(DownloadHints* pHints) {
@@ -1235,7 +1235,7 @@ FX_BOOL CPDF_DataAvail::CheckTrailer(DownloadHints* pHints) {
   return FALSE;
 }
 
-FX_BOOL CPDF_DataAvail::CheckPage(int32_t iPage, DownloadHints* pHints) {
+FX_BOOL CPDF_DataAvail::CheckPage(uint32_t dwPage, DownloadHints* pHints) {
   while (TRUE) {
     switch (m_docStatus) {
       case PDF_DATAAVAIL_PAGETREE:
@@ -1243,7 +1243,7 @@ FX_BOOL CPDF_DataAvail::CheckPage(int32_t iPage, DownloadHints* pHints) {
           return FALSE;
         break;
       case PDF_DATAAVAIL_PAGE:
-        if (!LoadDocPage(iPage, pHints))
+        if (!LoadDocPage(dwPage, pHints))
           return FALSE;
         break;
       case PDF_DATAAVAIL_ERROR:
@@ -1420,7 +1420,9 @@ FX_BOOL CPDF_DataAvail::CheckPageNode(CPDF_DataAvail::PageNode& pageNodes,
   return TRUE;
 }
 
-FX_BOOL CPDF_DataAvail::LoadDocPage(int32_t iPage, DownloadHints* pHints) {
+FX_BOOL CPDF_DataAvail::LoadDocPage(uint32_t dwPage, DownloadHints* pHints) {
+  FX_SAFE_INT32 safePage = pdfium::base::checked_cast<int32_t>(dwPage);
+  int32_t iPage = safePage.ValueOrDie();
   if (m_pDocument->GetPageCount() <= iPage ||
       m_pDocument->m_PageList.GetAt(iPage)) {
     m_docStatus = PDF_DATAAVAIL_DONE;
@@ -1531,12 +1533,14 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::CheckLinearizedData(
   return m_bLinearedDataOK ? DataAvailable : DataNotAvailable;
 }
 
-FX_BOOL CPDF_DataAvail::CheckPageAnnots(int32_t iPage, DownloadHints* pHints) {
+FX_BOOL CPDF_DataAvail::CheckPageAnnots(uint32_t dwPage,
+                                        DownloadHints* pHints) {
   if (!m_objs_array.GetSize()) {
     m_objs_array.RemoveAll();
     m_ObjectSet.clear();
 
-    CPDF_Dictionary* pPageDict = m_pDocument->GetPage(iPage);
+    FX_SAFE_INT32 safePage = pdfium::base::checked_cast<int32_t>(dwPage);
+    CPDF_Dictionary* pPageDict = m_pDocument->GetPage(safePage.ValueOrDie());
     if (!pPageDict)
       return TRUE;
 
@@ -1564,10 +1568,10 @@ FX_BOOL CPDF_DataAvail::CheckPageAnnots(int32_t iPage, DownloadHints* pHints) {
 }
 
 CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::CheckLinearizedFirstPage(
-    int32_t iPage,
+    uint32_t dwPage,
     DownloadHints* pHints) {
   if (!m_bAnnotsLoad) {
-    if (!CheckPageAnnots(iPage, pHints))
+    if (!CheckPageAnnots(dwPage, pHints))
       return DataNotAvailable;
     m_bAnnotsLoad = TRUE;
   }
@@ -1601,12 +1605,12 @@ FX_BOOL CPDF_DataAvail::HaveResourceAncestor(CPDF_Dictionary* pDict) {
 }
 
 CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
-    int32_t iPage,
+    uint32_t dwPage,
     DownloadHints* pHints) {
   if (!m_pDocument)
     return DataError;
 
-  if (IsFirstCheck(iPage)) {
+  if (IsFirstCheck(dwPage)) {
     m_bCurPageDictLoadOK = FALSE;
     m_bPageLoadedOK = FALSE;
     m_bAnnotsLoad = FALSE;
@@ -1615,14 +1619,14 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
     m_ObjectSet.clear();
   }
 
-  if (pdfium::ContainsKey(m_pagesLoadState, iPage))
+  if (pdfium::ContainsKey(m_pagesLoadState, dwPage))
     return DataAvailable;
 
   if (m_bLinearized) {
-    if ((uint32_t)iPage == m_dwFirstPageNo) {
-      DocAvailStatus nRet = CheckLinearizedFirstPage(iPage, pHints);
+    if (dwPage == m_dwFirstPageNo) {
+      DocAvailStatus nRet = CheckLinearizedFirstPage(dwPage, pHints);
       if (nRet == DataAvailable)
-        m_pagesLoadState.insert(iPage);
+        m_pagesLoadState.insert(dwPage);
       return nRet;
     }
 
@@ -1631,10 +1635,10 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
       return nResult;
 
     if (m_pHintTables) {
-      nResult = m_pHintTables->CheckPage(iPage, pHints);
+      nResult = m_pHintTables->CheckPage(dwPage, pHints);
       if (nResult != DataAvailable)
         return nResult;
-      m_pagesLoadState.insert(iPage);
+      m_pagesLoadState.insert(dwPage);
       return DataAvailable;
     }
 
@@ -1643,19 +1647,19 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
         if (!LoadPages(pHints))
           return DataNotAvailable;
       } else {
-        if (!m_bCurPageDictLoadOK && !CheckPage(iPage, pHints))
+        if (!m_bCurPageDictLoadOK && !CheckPage(dwPage, pHints))
           return DataNotAvailable;
       }
     } else {
       if (!LoadAllFile(pHints))
         return DataNotAvailable;
       m_pDocument->GetParser()->RebuildCrossRef();
-      ResetFirstCheck(iPage);
+      ResetFirstCheck(dwPage);
       return DataAvailable;
     }
   } else {
     if (!m_bTotalLoadPageTree && !m_bCurPageDictLoadOK &&
-        !CheckPage(iPage, pHints)) {
+        !CheckPage(dwPage, pHints)) {
       return DataNotAvailable;
     }
   }
@@ -1671,9 +1675,10 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
       m_objs_array.RemoveAll();
       m_ObjectSet.clear();
 
-      m_pPageDict = m_pDocument->GetPage(iPage);
+      FX_SAFE_INT32 safePage = pdfium::base::checked_cast<int32_t>(dwPage);
+      m_pPageDict = m_pDocument->GetPage(safePage.ValueOrDie());
       if (!m_pPageDict) {
-        ResetFirstCheck(iPage);
+        ResetFirstCheck(dwPage);
         return DataAvailable;
       }
 
@@ -1699,22 +1704,19 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
   }
 
   if (!m_bAnnotsLoad) {
-    if (!CheckPageAnnots(iPage, pHints))
+    if (!CheckPageAnnots(dwPage, pHints))
       return DataNotAvailable;
     m_bAnnotsLoad = TRUE;
   }
 
   if (m_pPageDict && !m_bNeedDownLoadResource) {
     m_pPageResource = m_pPageDict->GetObjectBy("Resources");
-    if (!m_pPageResource)
-      m_bNeedDownLoadResource = HaveResourceAncestor(m_pPageDict);
-    else
-      m_bNeedDownLoadResource = TRUE;
+    m_bNeedDownLoadResource =
+        m_pPageResource || HaveResourceAncestor(m_pPageDict);
   }
 
   if (m_bNeedDownLoadResource) {
-    FX_BOOL bRet = CheckResources(pHints);
-    if (!bRet)
+    if (!CheckResources(pHints))
       return DataNotAvailable;
     m_bNeedDownLoadResource = FALSE;
   }
@@ -1723,8 +1725,8 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::IsPageAvail(
   m_bAnnotsLoad = FALSE;
   m_bCurPageDictLoadOK = FALSE;
 
-  ResetFirstCheck(iPage);
-  m_pagesLoadState.insert(iPage);
+  ResetFirstCheck(dwPage);
+  m_pagesLoadState.insert(dwPage);
   return DataAvailable;
 }
 
diff --git a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp
index 445f3bf433..680939b186 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp
@@ -15,6 +15,7 @@
 #include "core/fpdfapi/fpdf_parser/include/cpdf_stream.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h"
 #include "core/fxcrt/include/fx_safe_types.h"
+#include "third_party/base/numerics/safe_conversions.h"
 
 namespace {
 
@@ -44,10 +45,9 @@ CPDF_HintTables::CPDF_HintTables(CPDF_DataAvail* pDataAvail,
 CPDF_HintTables::~CPDF_HintTables() {}
 
 uint32_t CPDF_HintTables::GetItemLength(
-    int index,
+    uint32_t index,
     const std::vector<FX_FILESIZE>& szArray) {
-  if (index < 0 || szArray.size() < 2 ||
-      static_cast<size_t>(index) > szArray.size() - 2 ||
+  if (szArray.size() < 2 || index > szArray.size() - 2 ||
       szArray[index] > szArray[index + 1]) {
     return 0;
   }
@@ -362,13 +362,10 @@ bool CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream,
   return true;
 }
 
-bool CPDF_HintTables::GetPagePos(int index,
+bool CPDF_HintTables::GetPagePos(uint32_t index,
                                  FX_FILESIZE* szPageStartPos,
                                  FX_FILESIZE* szPageLength,
                                  uint32_t* dwObjNum) {
-  if (index < 0)
-    return false;
-
   *szPageStartPos = m_szPageOffsetArray[index];
   *szPageLength = GetItemLength(index, m_szPageOffsetArray);
 
@@ -377,18 +374,19 @@ bool CPDF_HintTables::GetPagePos(int index,
     return false;
 
   int nFirstPageNum = GetFirstPageNumber();
-  if (nFirstPageNum < 0)
+  if (!pdfium::base::IsValueInRangeForNumericType<uint32_t>(nFirstPageNum))
     return false;
 
-  if (index == nFirstPageNum) {
+  uint32_t dwFirstPageNum = static_cast<uint32_t>(nFirstPageNum);
+  if (index == dwFirstPageNum) {
     *dwObjNum = nFirstPageObjNum;
     return true;
   }
 
   // The object number of remaining pages starts from 1.
   *dwObjNum = 1;
-  for (int i = 0; i < index; ++i) {
-    if (i == nFirstPageNum)
+  for (uint32_t i = 0; i < index; ++i) {
+    if (i == dwFirstPageNum)
       continue;
     *dwObjNum += m_dwDeltaNObjsArray[i];
   }
@@ -396,12 +394,16 @@ bool CPDF_HintTables::GetPagePos(int index,
 }
 
 CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(
-    int index,
+    uint32_t index,
     CPDF_DataAvail::DownloadHints* pHints) {
-  if (!pHints || index < 0)
+  if (!pHints)
+    return CPDF_DataAvail::DataError;
+
+  int nFirstPageNum = GetFirstPageNumber();
+  if (!pdfium::base::IsValueInRangeForNumericType<uint32_t>(nFirstPageNum))
     return CPDF_DataAvail::DataError;
 
-  if (index == GetFirstPageNumber())
+  if (index == static_cast<uint32_t>(nFirstPageNum))
     return CPDF_DataAvail::DataAvailable;
 
   uint32_t dwLength = GetItemLength(index, m_szPageOffsetArray);
@@ -414,7 +416,7 @@ CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(
 
   // Download data of shared objects in the page.
   uint32_t offset = 0;
-  for (int i = 0; i < index; ++i)
+  for (uint32_t i = 0; i < index; ++i)
     offset += m_dwNSharedObjsArray[i];
 
   int nFirstPageObjNum = GetFirstPageObjectNumber();
diff --git a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
index cda0925de8..1943c79f44 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
+++ b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
@@ -22,13 +22,13 @@ class CPDF_HintTables {
   CPDF_HintTables(CPDF_DataAvail* pDataAvail, CPDF_Dictionary* pLinearized);
   virtual ~CPDF_HintTables();
 
-  bool GetPagePos(int index,
+  bool GetPagePos(uint32_t index,
                   FX_FILESIZE* szPageStartPos,
                   FX_FILESIZE* szPageLength,
                   uint32_t* dwObjNum);
 
   CPDF_DataAvail::DocAvailStatus CheckPage(
-      int index,
+      uint32_t index,
       CPDF_DataAvail::DownloadHints* pHints);
 
   bool LoadHintStream(CPDF_Stream* pHintStream);
@@ -36,7 +36,6 @@ class CPDF_HintTables {
  protected:
   bool ReadPageHintTable(CFX_BitStream* hStream);
   bool ReadSharedObjHintTable(CFX_BitStream* hStream, uint32_t offset);
-  uint32_t GetItemLength(int index, const std::vector<FX_FILESIZE>& szArray);
 
  private:
   // Tests can override.
@@ -50,6 +49,9 @@ class CPDF_HintTables {
   // Helper for the ReadPrimaryHintStream methods above.
   int ReadPrimaryHintStream(int index) const;
 
+  uint32_t GetItemLength(uint32_t index,
+                         const std::vector<FX_FILESIZE>& szArray);
+
   // Owner, outlives this object.
   CPDF_DataAvail* const m_pDataAvail;
 
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h b/core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h
index 4adedba36c..b93394cd8f 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h
@@ -97,7 +97,7 @@ class CPDF_DataAvail final {
 
   DocAvailStatus IsDocAvail(DownloadHints* pHints);
   void SetDocument(CPDF_Document* pDoc);
-  DocAvailStatus IsPageAvail(int iPage, DownloadHints* pHints);
+  DocAvailStatus IsPageAvail(uint32_t dwPage, DownloadHints* pHints);
   DocFormStatus IsFormAvail(DownloadHints* pHints);
   DocLinearizationStatus IsLinearizedPDF();
   FX_BOOL IsLinearized();
@@ -166,13 +166,14 @@ class CPDF_DataAvail final {
   FX_BOOL LoadAllXref(DownloadHints* pHints);
   FX_BOOL LoadAllFile(DownloadHints* pHints);
   DocAvailStatus CheckLinearizedData(DownloadHints* pHints);
-  FX_BOOL CheckPageAnnots(int iPage, DownloadHints* pHints);
+  FX_BOOL CheckPageAnnots(uint32_t dwPage, DownloadHints* pHints);
 
-  DocAvailStatus CheckLinearizedFirstPage(int iPage, DownloadHints* pHints);
+  DocAvailStatus CheckLinearizedFirstPage(uint32_t dwPage,
+                                          DownloadHints* pHints);
   FX_BOOL HaveResourceAncestor(CPDF_Dictionary* pDict);
-  FX_BOOL CheckPage(int32_t iPage, DownloadHints* pHints);
+  FX_BOOL CheckPage(uint32_t dwPage, DownloadHints* pHints);
   FX_BOOL LoadDocPages(DownloadHints* pHints);
-  FX_BOOL LoadDocPage(int32_t iPage, DownloadHints* pHints);
+  FX_BOOL LoadDocPage(uint32_t dwPage, DownloadHints* pHints);
   FX_BOOL CheckPageNode(PageNode& pageNodes,
                         int32_t iPage,
                         int32_t& iCount,
@@ -185,8 +186,8 @@ class CPDF_DataAvail final {
                              PageNode* pPageNode,
                              DownloadHints* pHints);
   FX_BOOL CheckPageCount(DownloadHints* pHints);
-  bool IsFirstCheck(int iPage);
-  void ResetFirstCheck(int iPage);
+  bool IsFirstCheck(uint32_t dwPage);
+  void ResetFirstCheck(uint32_t dwPage);
   FX_BOOL IsDataAvail(FX_FILESIZE offset, uint32_t size, DownloadHints* pHints);
 
   FileAvail* const m_pFileAvail;