From e0cb687c71d555b55cc30ec59c9b338a0e83f77f Mon Sep 17 00:00:00 2001 From: thestig Date: Thu, 1 Sep 2016 13:41:10 -0700 Subject: Use unsigned page indexes in CPDF_HintTables. Fix up callers from CPDF_DataAvail. Review-Url: https://codereview.chromium.org/2294383003 --- core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp | 68 +++++++++++----------- core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp | 32 +++++----- core/fpdfapi/fpdf_parser/cpdf_hint_tables.h | 8 ++- core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h | 15 ++--- fpdfsdk/fpdf_dataavail.cpp | 2 + 5 files changed, 67 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(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(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(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& szArray) { - if (index < 0 || szArray.size() < 2 || - static_cast(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(nFirstPageNum)) return false; - if (index == nFirstPageNum) { + uint32_t dwFirstPageNum = static_cast(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(nFirstPageNum)) return CPDF_DataAvail::DataError; - if (index == GetFirstPageNumber()) + if (index == static_cast(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& 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& 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; diff --git a/fpdfsdk/fpdf_dataavail.cpp b/fpdfsdk/fpdf_dataavail.cpp index 7b9ba32fb0..1a83fdf64b 100644 --- a/fpdfsdk/fpdf_dataavail.cpp +++ b/fpdfsdk/fpdf_dataavail.cpp @@ -162,6 +162,8 @@ DLLEXPORT int STDCALL FPDFAvail_IsPageAvail(FPDF_AVAIL avail, FX_DOWNLOADHINTS* hints) { if (!avail || !hints) return PDF_DATA_ERROR; + if (page_index < 0) + return PDF_DATA_NOTAVAIL; CFPDF_DownloadHintsWrap hints_wrap(hints); return CFPDFDataAvailFromFPDFAvail(avail)->m_pDataAvail->IsPageAvail( page_index, &hints_wrap); -- cgit v1.2.3