From 62b218074ea2bc658488c583362930a798b39107 Mon Sep 17 00:00:00 2001 From: Artem Strygin Date: Mon, 2 Oct 2017 22:07:55 +0300 Subject: Improve CPDF_ReadVaildator Merge CPDF_ReadValidator::IsXXXAvailable and CPDF_ReadValidator::SheduleXXXDowload methods into one CheckXXXAndRequestIfUnavailable. Change-Id: Ib108d5bc3589df17269c841b0024dd4690909304 Reviewed-on: https://pdfium-review.googlesource.com/13730 Reviewed-by: dsinclair Commit-Queue: Art Snake --- core/fpdfapi/parser/cpdf_data_avail.cpp | 26 ++++------ core/fpdfapi/parser/cpdf_data_avail.h | 6 +-- core/fpdfapi/parser/cpdf_read_validator.cpp | 55 +++++++++++----------- core/fpdfapi/parser/cpdf_read_validator.h | 6 +-- .../parser/cpdf_read_validator_unittest.cpp | 4 +- fpdfsdk/fpdf_dataavail.cpp | 4 +- 6 files changed, 48 insertions(+), 53 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index 8b378aab0e..b2415915c4 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -322,11 +322,10 @@ bool CPDF_DataAvail::CheckPageStatus() { } bool CPDF_DataAvail::LoadAllFile() { - if (GetValidator()->IsWholeFileAvailable()) { + if (GetValidator()->CheckWholeFileAndRequestIfUnavailable()) { m_docStatus = PDF_DATAAVAIL_DONE; return true; } - GetValidator()->ScheduleDownloadWholeFile(); return false; } @@ -590,18 +589,17 @@ bool CPDF_DataAvail::CheckFirstPage() { dwEnd = (uint32_t)m_dwFileLen; const FX_FILESIZE start_pos = m_dwFileLen > 1024 ? 1024 : m_dwFileLen; - const uint32_t data_size = dwEnd > 1024 ? dwEnd - 1024 : 0; - if (!GetValidator()->IsDataRangeAvailable(start_pos, data_size)) { - GetValidator()->ScheduleDataDownload(start_pos, data_size); + const size_t data_size = dwEnd > 1024 ? static_cast(dwEnd - 1024) : 0; + if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(start_pos, + data_size)) return false; - } m_docStatus = m_bSupportHintTable ? PDF_DATAAVAIL_HINTTABLE : PDF_DATAAVAIL_DONE; return true; } -bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, uint32_t size) { +bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, size_t size) { if (offset < 0 || offset > m_dwFileLen) return true; @@ -613,10 +611,9 @@ bool CPDF_DataAvail::IsDataAvail(FX_FILESIZE offset, uint32_t size) { else size += 512; - if (!GetValidator()->IsDataRangeAvailable(offset, size)) { - GetValidator()->ScheduleDataDownload(offset, size); + if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(offset, size)) return false; - } + return true; } @@ -1207,17 +1204,14 @@ CPDF_DataAvail::DocAvailStatus CPDF_DataAvail::CheckLinearizedData() { return DataError; if (!m_bMainXRefLoadTried) { - FX_SAFE_UINT32 data_size = m_dwFileLen; + FX_SAFE_SIZE_T data_size = m_dwFileLen; data_size -= m_pLinearized->GetLastXRefOffset(); if (!data_size.IsValid()) return DataError; - if (!GetValidator()->IsDataRangeAvailable( - m_pLinearized->GetLastXRefOffset(), data_size.ValueOrDie())) { - GetValidator()->ScheduleDataDownload(m_pLinearized->GetLastXRefOffset(), - data_size.ValueOrDie()); + if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable( + m_pLinearized->GetLastXRefOffset(), data_size.ValueOrDie())) return DataNotAvailable; - } CPDF_Parser::Error eRet = m_pDocument->GetParser()->LoadLinearizedMainXRefTable(); diff --git a/core/fpdfapi/parser/cpdf_data_avail.h b/core/fpdfapi/parser/cpdf_data_avail.h index ab7f045fec..24b5b7f357 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.h +++ b/core/fpdfapi/parser/cpdf_data_avail.h @@ -84,13 +84,13 @@ class CPDF_DataAvail final { class FileAvail { public: virtual ~FileAvail(); - virtual bool IsDataAvail(FX_FILESIZE offset, uint32_t size) = 0; + virtual bool IsDataAvail(FX_FILESIZE offset, size_t size) = 0; }; class DownloadHints { public: virtual ~DownloadHints(); - virtual void AddSegment(FX_FILESIZE offset, uint32_t size) = 0; + virtual void AddSegment(FX_FILESIZE offset, size_t size) = 0; }; CPDF_DataAvail(FileAvail* pFileAvail, @@ -98,7 +98,7 @@ class CPDF_DataAvail final { bool bSupportHintTable); ~CPDF_DataAvail(); - bool IsDataAvail(FX_FILESIZE offset, uint32_t size); + 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_read_validator.cpp b/core/fpdfapi/parser/cpdf_read_validator.cpp index bf086008ca..0f7eaa70a2 100644 --- a/core/fpdfapi/parser/cpdf_read_validator.cpp +++ b/core/fpdfapi/parser/cpdf_read_validator.cpp @@ -59,22 +59,18 @@ void CPDF_ReadValidator::ResetErrors() { bool CPDF_ReadValidator::ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) { - // correct values checks: - if (!pdfium::base::IsValueInRangeForNumericType(size)) - return false; - FX_SAFE_FILESIZE end_offset = offset; end_offset += size; if (!end_offset.IsValid() || end_offset.ValueOrDie() > GetSize()) return false; - if (!file_avail_ || - file_avail_->IsDataAvail(offset, static_cast(size))) { - if (file_read_->ReadBlock(buffer, offset, size)) - return true; - read_error_ = true; - } - has_unavailable_data_ = true; + if (!CheckDataRangeAndRequestIfUnavailable(offset, size)) + return false; + + if (file_read_->ReadBlock(buffer, offset, size)) + return true; + + read_error_ = true; ScheduleDownload(offset, size); return false; } @@ -84,13 +80,7 @@ FX_FILESIZE CPDF_ReadValidator::GetSize() { } void CPDF_ReadValidator::ScheduleDownload(FX_FILESIZE offset, size_t size) { - const FX_SAFE_UINT32 safe_size = size; - if (safe_size.IsValid()) - ScheduleDataDownload(offset, safe_size.ValueOrDie()); -} - -void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset, - uint32_t size) { + has_unavailable_data_ = true; if (!hints_ || size == 0) return; @@ -104,7 +94,7 @@ void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset, end_segment_offset = std::min(GetSize(), AlignUp(end_segment_offset.ValueOrDie())); - FX_SAFE_UINT32 segment_size = end_segment_offset; + FX_SAFE_SIZE_T segment_size = end_segment_offset; segment_size -= start_segment_offset; if (!segment_size.IsValid()) { NOTREACHED(); @@ -113,19 +103,30 @@ void CPDF_ReadValidator::ScheduleDataDownload(FX_FILESIZE offset, hints_->AddSegment(start_segment_offset, segment_size.ValueOrDie()); } -void CPDF_ReadValidator::ScheduleDownloadWholeFile() { - const FX_SAFE_UINT32 safe_size = GetSize(); - if (safe_size.IsValid()) - ScheduleDataDownload(0, safe_size.ValueOrDie()); -} - bool CPDF_ReadValidator::IsDataRangeAvailable(FX_FILESIZE offset, - uint32_t size) const { + size_t size) const { return !file_avail_ || file_avail_->IsDataAvail(offset, size); } bool CPDF_ReadValidator::IsWholeFileAvailable() { - const FX_SAFE_UINT32 safe_size = GetSize(); + const FX_SAFE_SIZE_T safe_size = GetSize(); return safe_size.IsValid() ? IsDataRangeAvailable(0, safe_size.ValueOrDie()) : false; } + +bool CPDF_ReadValidator::CheckDataRangeAndRequestIfUnavailable( + FX_FILESIZE offset, + size_t size) { + if (IsDataRangeAvailable(offset, size)) + return true; + + ScheduleDownload(offset, size); + return false; +} + +bool CPDF_ReadValidator::CheckWholeFileAndRequestIfUnavailable() { + const FX_SAFE_SIZE_T safe_size = GetSize(); + return safe_size.IsValid() + ? CheckDataRangeAndRequestIfUnavailable(0, safe_size.ValueOrDie()) + : false; +} diff --git a/core/fpdfapi/parser/cpdf_read_validator.h b/core/fpdfapi/parser/cpdf_read_validator.h index 0bbf6ab924..0fb15b20f8 100644 --- a/core/fpdfapi/parser/cpdf_read_validator.h +++ b/core/fpdfapi/parser/cpdf_read_validator.h @@ -37,11 +37,10 @@ class CPDF_ReadValidator : public IFX_SeekableReadStream { void ResetErrors(); - bool IsDataRangeAvailable(FX_FILESIZE offset, uint32_t size) const; bool IsWholeFileAvailable(); - void ScheduleDataDownload(FX_FILESIZE offset, uint32_t size); - void ScheduleDownloadWholeFile(); + bool CheckDataRangeAndRequestIfUnavailable(FX_FILESIZE offset, size_t size); + bool CheckWholeFileAndRequestIfUnavailable(); // IFX_SeekableReadStream overrides: bool ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) override; @@ -54,6 +53,7 @@ class CPDF_ReadValidator : public IFX_SeekableReadStream { private: void ScheduleDownload(FX_FILESIZE offset, size_t size); + bool IsDataRangeAvailable(FX_FILESIZE offset, size_t size) const; RetainPtr file_read_; UnownedPtr file_avail_; diff --git a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp index 308704f762..13ace078d3 100644 --- a/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_read_validator_unittest.cpp @@ -26,7 +26,7 @@ class MockFileAvail : public CPDF_DataAvail::FileAvail { MockFileAvail() : available_range_(0, 0) {} ~MockFileAvail() override {} - bool IsDataAvail(FX_FILESIZE offset, uint32_t size) override { + bool IsDataAvail(FX_FILESIZE offset, size_t size) override { return available_range_.first <= offset && available_range_.second >= static_cast(offset + size); } @@ -48,7 +48,7 @@ class MockDownloadHints : public CPDF_DataAvail::DownloadHints { MockDownloadHints() : last_requested_range_(0, 0) {} ~MockDownloadHints() override {} - void AddSegment(FX_FILESIZE offset, uint32_t size) override { + void AddSegment(FX_FILESIZE offset, size_t size) override { last_requested_range_.first = offset; last_requested_range_.second = offset + size; } diff --git a/fpdfsdk/fpdf_dataavail.cpp b/fpdfsdk/fpdf_dataavail.cpp index 3e1f713477..0a74c9c245 100644 --- a/fpdfsdk/fpdf_dataavail.cpp +++ b/fpdfsdk/fpdf_dataavail.cpp @@ -51,7 +51,7 @@ class FPDF_FileAvailContext : public CPDF_DataAvail::FileAvail { void Set(FX_FILEAVAIL* pfileAvail) { m_pfileAvail = pfileAvail; } // CPDF_DataAvail::FileAvail: - bool IsDataAvail(FX_FILESIZE offset, uint32_t size) override { + bool IsDataAvail(FX_FILESIZE offset, size_t size) override { return !!m_pfileAvail->IsDataAvail(m_pfileAvail, offset, size); } @@ -91,7 +91,7 @@ class FPDF_DownloadHintsContext : public CPDF_DataAvail::DownloadHints { public: // IFX_DownloadHints - void AddSegment(FX_FILESIZE offset, uint32_t size) override { + void AddSegment(FX_FILESIZE offset, size_t size) override { if (m_pDownloadHints) m_pDownloadHints->AddSegment(m_pDownloadHints, offset, size); } -- cgit v1.2.3