Unify CPDF_HintsTable.chromium/3232

Break relationship from CPDF_DataAvail for CPDF_HintsTable.

Change-Id: If3e6d4910dd368742f095c05adb122ab08c0d804
Reviewed-on: https://pdfium-review.googlesource.com/15270
Commit-Queue: Art Snake <art-snake@yandex-team.ru>
Reviewed-by: dsinclair <dsinclair@chromium.org>

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<CPDF_HintTables>(this, m_pLinearized.get());
+  auto pHintTables = pdfium::MakeUnique<CPDF_HintTables>(GetValidator().Get(),
+                                                         m_pLinearized.get());
   std::unique_ptr<CPDF_Object> 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<FX_FILESIZE>& szArray);
 
-  // Owner, outlives this object.
-  UnownedPtr<CPDF_DataAvail> const m_pDataAvail;
+  // Owned by |m_pDataAvail|.
+  UnownedPtr<CPDF_ReadValidator> m_pValidator;
 
   // Owned by |m_pDataAvail|.
   UnownedPtr<CPDF_LinearizedHeader> 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 <algorithm>
 
+#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<FX_FILESIZE>(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<uint8_t> test_data(kTestDataSize);
+  auto file = pdfium::MakeRetain<CFX_MemoryStream>(test_data.data(),
+                                                   test_data.size(), false);
+  MockFileAvail file_avail;
+  auto validator = pdfium::MakeRetain<CPDF_ReadValidator>(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<uint8_t> 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);
+}