From aafdc153a2b7609968b315ca6c6900717f6d6c36 Mon Sep 17 00:00:00 2001 From: Artem Strygin Date: Tue, 3 Oct 2017 17:33:19 +0300 Subject: Unify CPDF_HintsTable. Break relationship from CPDF_DataAvail for CPDF_HintsTable. Change-Id: If3e6d4910dd368742f095c05adb122ab08c0d804 Reviewed-on: https://pdfium-review.googlesource.com/15270 Commit-Queue: Art Snake Reviewed-by: dsinclair --- core/fpdfapi/parser/cpdf_data_avail.cpp | 25 ++--------- core/fpdfapi/parser/cpdf_data_avail.h | 1 - core/fpdfapi/parser/cpdf_hint_tables.cpp | 12 +++--- core/fpdfapi/parser/cpdf_hint_tables.h | 7 ++-- core/fpdfapi/parser/cpdf_read_validator.cpp | 28 +++++++++++-- .../parser/cpdf_read_validator_unittest.cpp | 48 ++++++++++++++++++++++ 6 files changed, 87 insertions(+), 34 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index b2415915c4..aaec4c7708 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -599,24 +599,6 @@ bool CPDF_DataAvail::CheckFirstPage() { return true; } -bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, size_t size) { - if (offset < 0 || offset > m_dwFileLen) - return true; - - FX_SAFE_FILESIZE safeSize = offset; - safeSize += size; - safeSize += 512; - if (!safeSize.IsValid() || safeSize.ValueOrDie() > m_dwFileLen) - size = m_dwFileLen - offset; - else - size += 512; - - if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(offset, size)) - return false; - - return true; -} - bool CPDF_DataAvail::CheckHintTables() { if (m_pLinearized->GetPageCount() <= 1) { m_docStatus = PDF_DATAAVAIL_DONE; @@ -630,13 +612,14 @@ bool CPDF_DataAvail::CheckHintTables() { const FX_FILESIZE szHintStart = m_pLinearized->GetHintStart(); const uint32_t szHintLength = m_pLinearized->GetHintLength(); - if (!IsDataAvail(szHintStart, szHintLength)) + if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(szHintStart, + szHintLength)) return false; m_syntaxParser.InitParser(m_pFileRead, m_dwHeaderOffset); - auto pHintTables = - pdfium::MakeUnique(this, m_pLinearized.get()); + auto pHintTables = pdfium::MakeUnique(GetValidator().Get(), + m_pLinearized.get()); std::unique_ptr pHintStream = ParseIndirectObjectAt(szHintStart, 0); CPDF_Stream* pStream = ToStream(pHintStream.get()); diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h index 24b5b7f357..1ebee8768a 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.h +++ b/core/fpdfapi/parser/cpdf_data_avail.h @@ -98,7 +98,6 @@ class CPDF_DataAvail final { bool bSupportHintTable); ~CPDF_DataAvail(); - bool IsDataAvail(FX_FILESIZE offset, size_t size); DocAvailStatus IsDocAvail(DownloadHints* pHints); void SetDocument(CPDF_Document* pDoc); DocAvailStatus IsPageAvail(uint32_t dwPage, DownloadHints* pHints); diff --git a/core/fpdfapi/parser/cpdf_hint_tables.cpp b/core/fpdfapi/parser/cpdf_hint_tables.cpp index 7a5eb2397b..e88da2eea8 100644 --- a/core/fpdfapi/parser/cpdf_hint_tables.cpp +++ b/core/fpdfapi/parser/cpdf_hint_tables.cpp @@ -13,6 +13,7 @@ #include "core/fpdfapi/parser/cpdf_dictionary.h" #include "core/fpdfapi/parser/cpdf_document.h" #include "core/fpdfapi/parser/cpdf_linearized_header.h" +#include "core/fpdfapi/parser/cpdf_read_validator.h" #include "core/fpdfapi/parser/cpdf_stream.h" #include "core/fpdfapi/parser/cpdf_stream_acc.h" #include "core/fxcrt/cfx_bitstream.h" @@ -35,9 +36,9 @@ bool IsValidPageOffsetHintTableBitCount(uint32_t bits) { } // namespace -CPDF_HintTables::CPDF_HintTables(CPDF_DataAvail* pDataAvail, +CPDF_HintTables::CPDF_HintTables(CPDF_ReadValidator* pValidator, CPDF_LinearizedHeader* pLinearized) - : m_pDataAvail(pDataAvail), + : m_pValidator(pValidator), m_pLinearized(pLinearized), m_nFirstPageSharedObjs(0), m_szFirstPageObjOffset(0) { @@ -413,7 +414,8 @@ CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(uint32_t index) { if (!dwLength) return CPDF_DataAvail::DataError; - if (!m_pDataAvail->IsDataAvail(m_szPageOffsetArray[index], dwLength)) + if (!m_pValidator->CheckDataRangeAndRequestIfUnavailable( + m_szPageOffsetArray[index], dwLength)) return CPDF_DataAvail::DataNotAvailable; // Download data of shared objects in the page. @@ -444,8 +446,8 @@ CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(uint32_t index) { if (!dwLength) return CPDF_DataAvail::DataError; - if (!m_pDataAvail->IsDataAvail(m_szSharedObjOffsetArray[dwIndex], - dwLength)) { + if (!m_pValidator->CheckDataRangeAndRequestIfUnavailable( + m_szSharedObjOffsetArray[dwIndex], dwLength)) { return CPDF_DataAvail::DataNotAvailable; } } diff --git a/core/fpdfapi/parser/cpdf_hint_tables.h b/core/fpdfapi/parser/cpdf_hint_tables.h index 9658be5462..a9aa5d42d9 100644 --- a/core/fpdfapi/parser/cpdf_hint_tables.h +++ b/core/fpdfapi/parser/cpdf_hint_tables.h @@ -16,10 +16,11 @@ class CFX_BitStream; class CPDF_LinearizedHeader; class CPDF_Stream; +class CPDF_ReadValidator; class CPDF_HintTables { public: - CPDF_HintTables(CPDF_DataAvail* pDataAvail, + CPDF_HintTables(CPDF_ReadValidator* pValidator, CPDF_LinearizedHeader* pLinearized); virtual ~CPDF_HintTables(); @@ -48,8 +49,8 @@ class CPDF_HintTables { uint32_t GetItemLength(uint32_t index, const std::vector& szArray); - // Owner, outlives this object. - UnownedPtr const m_pDataAvail; + // Owned by |m_pDataAvail|. + UnownedPtr m_pValidator; // Owned by |m_pDataAvail|. UnownedPtr const m_pLinearized; diff --git a/core/fpdfapi/parser/cpdf_read_validator.cpp b/core/fpdfapi/parser/cpdf_read_validator.cpp index 6c311f7571..2363f851ec 100644 --- a/core/fpdfapi/parser/cpdf_read_validator.cpp +++ b/core/fpdfapi/parser/cpdf_read_validator.cpp @@ -6,11 +6,12 @@ #include +#include "core/fpdfapi/cpdf_modulemgr.h" #include "third_party/base/logging.h" namespace { -constexpr FX_FILESIZE kAlignBlockValue = 512; +constexpr FX_FILESIZE kAlignBlockValue = CPDF_ModuleMgr::kFileBufSize; FX_FILESIZE AlignDown(FX_FILESIZE offset) { return offset > 0 ? (offset - offset % kAlignBlockValue) : 0; @@ -64,8 +65,10 @@ bool CPDF_ReadValidator::ReadBlock(void* buffer, if (!end_offset.IsValid() || end_offset.ValueOrDie() > file_size_) return false; - if (!CheckDataRangeAndRequestIfUnavailable(offset, size)) + if (!IsDataRangeAvailable(offset, size)) { + ScheduleDownload(offset, size); return false; + } if (file_read_->ReadBlock(buffer, offset, size)) return true; @@ -122,10 +125,27 @@ bool CPDF_ReadValidator::IsWholeFileAvailable() { bool CPDF_ReadValidator::CheckDataRangeAndRequestIfUnavailable( FX_FILESIZE offset, size_t size) { - if (IsDataRangeAvailable(offset, size)) + FX_SAFE_FILESIZE end_segment_offset = offset; + end_segment_offset += size; + // Increase checked range to allow CPDF_SyntaxParser read whole buffer. + end_segment_offset += CPDF_ModuleMgr::kFileBufSize; + if (!end_segment_offset.IsValid()) { + NOTREACHED(); + return false; + } + end_segment_offset = std::min( + file_size_, static_cast(end_segment_offset.ValueOrDie())); + FX_SAFE_SIZE_T segment_size = end_segment_offset; + segment_size -= offset; + if (!segment_size.IsValid()) { + NOTREACHED(); + return false; + } + + if (IsDataRangeAvailable(offset, segment_size.ValueOrDie())) return true; - ScheduleDownload(offset, size); + ScheduleDownload(offset, segment_size.ValueOrDie()); return false; } diff --git a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp index 13ace078d3..89b7e6b4de 100644 --- a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp @@ -243,3 +243,51 @@ TEST(CPDF_ReadValidatorTest, SessionReset) { EXPECT_TRUE(validator->has_unavailable_data()); EXPECT_FALSE(validator->read_error()); } + +TEST(CPDF_ReadValidatorTest, CheckDataRangeAndRequestIfUnavailable) { + std::vector test_data(kTestDataSize); + auto file = pdfium::MakeRetain(test_data.data(), + test_data.size(), false); + MockFileAvail file_avail; + auto validator = pdfium::MakeRetain(file, &file_avail); + + MockDownloadHints hints; + validator->SetDownloadHints(&hints); + + EXPECT_FALSE(validator->CheckDataRangeAndRequestIfUnavailable(5000, 100)); + EXPECT_FALSE(validator->read_error()); + EXPECT_TRUE(validator->has_unavailable_data()); + + // Requested range should be enlarged and aligned. + EXPECT_EQ(MakeRange(4608, 5632), hints.GetLastRequstedRange()); + + file_avail.SetAvailableRange(hints.GetLastRequstedRange()); + hints.Reset(); + + validator->ResetErrors(); + EXPECT_TRUE(validator->CheckDataRangeAndRequestIfUnavailable(5000, 100)); + // No new request on already available data. + EXPECT_EQ(MakeRange(0, 0), hints.GetLastRequstedRange()); + EXPECT_FALSE(validator->read_error()); + EXPECT_FALSE(validator->has_unavailable_data()); + + std::vector read_buffer(100); + EXPECT_TRUE( + validator->ReadBlock(read_buffer.data(), 5000, read_buffer.size())); + // No new request on already available data. + EXPECT_EQ(MakeRange(0, 0), hints.GetLastRequstedRange()); + EXPECT_FALSE(validator->read_error()); + EXPECT_FALSE(validator->has_unavailable_data()); + + validator->ResetErrors(); + // Try request unavailable data at file end. + EXPECT_FALSE(validator->CheckDataRangeAndRequestIfUnavailable( + validator->GetSize() - 100, 100)); + + // Should not enlarge request at file end. + EXPECT_EQ(validator->GetSize(), hints.GetLastRequstedRange().second); + EXPECT_FALSE(validator->read_error()); + EXPECT_TRUE(validator->has_unavailable_data()); + + validator->SetDownloadHints(nullptr); +} -- cgit v1.2.3