Ignore /Prev for hybrid-reference files
Per spec, files whose trailers have /XRefStm and /Prev should ignore the
/Prev value. Do this to properly fix the rendering for bug_1484283.pdf,
without causing any known regressions. Re-enable that pixel test.
Bug: chromium:1484283
Change-Id: I371adaba8b22577b24fd8e000a30992840e651d9
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116093
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index 43984d0..d502c42 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -389,33 +389,39 @@
std::vector<FX_FILESIZE> xref_list{xref_offset};
std::set<FX_FILESIZE> seen_xref_offset{xref_offset};
- // When the trailer doesn't have Prev entry or Prev entry value is not
- // numerical, GetDirectInteger() returns 0. Loading will end.
- xref_offset = GetTrailer()->GetDirectIntegerFor("Prev");
- while (xref_offset > 0) {
- // Check for circular references.
- if (pdfium::Contains(seen_xref_offset, xref_offset))
- return false;
+ // Ignore /Prev for hybrid-reference files.
+ // See ISO 32000-1:2008 spec, table 17.
+ if (!xref_stm) {
+ // When the trailer doesn't have Prev entry or Prev entry value is not
+ // numerical, GetDirectInteger() returns 0. Loading will end.
+ xref_offset = GetTrailer()->GetDirectIntegerFor("Prev");
+ while (xref_offset > 0) {
+ // Check for circular references.
+ if (pdfium::Contains(seen_xref_offset, xref_offset)) {
+ return false;
+ }
- seen_xref_offset.insert(xref_offset);
- xref_list.insert(xref_list.begin(), xref_offset);
+ seen_xref_offset.insert(xref_offset);
+ xref_list.insert(xref_list.begin(), xref_offset);
- // SLOW ...
- LoadCrossRefV4(xref_offset, true);
+ // SLOW ...
+ LoadCrossRefV4(xref_offset, true);
- RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
- if (!pDict)
- return false;
+ RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
+ if (!pDict) {
+ return false;
+ }
- xref_offset = pDict->GetDirectIntegerFor("Prev");
- xref_stm = pDict->GetIntegerFor("XRefStm");
- xref_stream_list.insert(xref_stream_list.begin(), xref_stm);
+ xref_offset = pDict->GetDirectIntegerFor("Prev");
+ xref_stm = pDict->GetIntegerFor("XRefStm");
+ xref_stream_list.insert(xref_stream_list.begin(), xref_stm);
- // SLOW ...
- m_CrossRefTable = CPDF_CrossRefTable::MergeUp(
- std::make_unique<CPDF_CrossRefTable>(std::move(pDict),
- kNoV4TrailerObjectNumber),
- std::move(m_CrossRefTable));
+ // SLOW ...
+ m_CrossRefTable = CPDF_CrossRefTable::MergeUp(
+ std::make_unique<CPDF_CrossRefTable>(std::move(pDict),
+ kNoV4TrailerObjectNumber),
+ std::move(m_CrossRefTable));
+ }
}
for (size_t i = 0; i < xref_list.size(); ++i) {
@@ -459,33 +465,39 @@
kNoV4TrailerObjectNumber),
std::move(m_CrossRefTable));
- // Now GetTrailer() returns the merged trailer, where /Prev is from the
- // main-trailer.
- FX_FILESIZE xref_offset = GetTrailer()->GetDirectIntegerFor("Prev");
- while (xref_offset > 0) {
- // Check for circular references.
- if (pdfium::Contains(seen_xref_offset, xref_offset))
- return false;
+ // Ignore /Prev for hybrid-reference files.
+ // See ISO 32000-1:2008 spec, table 17.
+ if (!xref_stm) {
+ // Now GetTrailer() returns the merged trailer, where /Prev is from the
+ // main-trailer.
+ FX_FILESIZE xref_offset = GetTrailer()->GetDirectIntegerFor("Prev");
+ while (xref_offset > 0) {
+ // Check for circular references.
+ if (pdfium::Contains(seen_xref_offset, xref_offset)) {
+ return false;
+ }
- seen_xref_offset.insert(xref_offset);
- xref_list.insert(xref_list.begin(), xref_offset);
+ seen_xref_offset.insert(xref_offset);
+ xref_list.insert(xref_list.begin(), xref_offset);
- // SLOW ...
- LoadCrossRefV4(xref_offset, true);
+ // SLOW ...
+ LoadCrossRefV4(xref_offset, true);
- RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
- if (!pDict)
- return false;
+ RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
+ if (!pDict) {
+ return false;
+ }
- xref_offset = pDict->GetDirectIntegerFor("Prev");
- xref_stm = pDict->GetIntegerFor("XRefStm");
- xref_stream_list.insert(xref_stream_list.begin(), xref_stm);
+ xref_offset = pDict->GetDirectIntegerFor("Prev");
+ xref_stm = pDict->GetIntegerFor("XRefStm");
+ xref_stream_list.insert(xref_stream_list.begin(), xref_stm);
- // SLOW ...
- m_CrossRefTable = CPDF_CrossRefTable::MergeUp(
- std::make_unique<CPDF_CrossRefTable>(std::move(pDict),
- kNoV4TrailerObjectNumber),
- std::move(m_CrossRefTable));
+ // SLOW ...
+ m_CrossRefTable = CPDF_CrossRefTable::MergeUp(
+ std::make_unique<CPDF_CrossRefTable>(std::move(pDict),
+ kNoV4TrailerObjectNumber),
+ std::move(m_CrossRefTable));
+ }
}
if (xref_stream_list[0] > 0 &&
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index 28c98ff..96c84d12 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -639,9 +639,6 @@
# TODO(pdfium:1457): Remove after associated bug is fixed
bug_1457.in * * * *
-# TODO(chromium:1484283): Remove after associated bug is fixed
-bug_1484283.pdf * * * *
-
# TODO(pdfium:1519): Remove after associated bug is fixed
bug_1519.in * * * *
