Fix PDF loading when an initial xref stream references a xref table
In CPDF_Parser, LoadAllCrossRefV4() will try to call LoadCrossRefV5() if
LoadCrossRefV4() fails for secondary xref data. However,
LoadAllCrossRefV5() only calls LoadCrossRefV5(). As a result, if
LoadAllCrossRefV5() only knows how to handle cross reference streams,
and fails if it ever encounters a cross reference table. Update
LoadAllCrossRefV5() to call LoadCrossRefV4() if LoadCrossRefV5() fails.
Then unsuppress the pixel test that renders correctly as a result of
this fix.
Along the way, fix some nits for how CPDF_Dictionary variables in
CPDF_Parser are named and assigned to.
Bug: pdfium:2123
Change-Id: Id77056b459dcd58864e34821940e73a29caed927
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116274
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index b28deeb..89a846e 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -407,18 +407,18 @@
// SLOW ...
LoadCrossRefV4(xref_offset, true);
- RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
- if (!pDict) {
+ RetainPtr<CPDF_Dictionary> trailer_dict = LoadTrailerV4();
+ if (!trailer_dict) {
return false;
}
- xref_offset = pDict->GetDirectIntegerFor("Prev");
- xref_stm = pDict->GetIntegerFor("XRefStm");
+ xref_offset = trailer_dict->GetDirectIntegerFor("Prev");
+ xref_stm = trailer_dict->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),
+ std::make_unique<CPDF_CrossRefTable>(std::move(trailer_dict),
kNoV4TrailerObjectNumber),
std::move(m_CrossRefTable));
}
@@ -483,18 +483,18 @@
// SLOW ...
LoadCrossRefV4(xref_offset, true);
- RetainPtr<CPDF_Dictionary> pDict(LoadTrailerV4());
- if (!pDict) {
+ RetainPtr<CPDF_Dictionary> trailer_dict = LoadTrailerV4();
+ if (!trailer_dict) {
return false;
}
- xref_offset = pDict->GetDirectIntegerFor("Prev");
- xref_stm = pDict->GetIntegerFor("XRefStm");
+ xref_offset = trailer_dict->GetDirectIntegerFor("Prev");
+ xref_stm = trailer_dict->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),
+ std::make_unique<CPDF_CrossRefTable>(std::move(trailer_dict),
kNoV4TrailerObjectNumber),
std::move(m_CrossRefTable));
}
@@ -680,13 +680,27 @@
std::set<FX_FILESIZE> seen_xref_offset;
while (xref_offset > 0) {
seen_xref_offset.insert(xref_offset);
+ FX_FILESIZE save_xref_offset = xref_offset;
if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false)) {
- return false;
+ // If a cross-reference stream failed to load at `xref_offset`, try
+ // loading a cross-reference table at the same location. Use
+ // `save_xref_offset` instead of `xref_offset`, as `xref_offset` may have
+ // changed.
+ if (!LoadCrossRefV4(save_xref_offset, /*bSkip=*/false)) {
+ return false;
+ }
+
+ RetainPtr<CPDF_Dictionary> trailer_dict = LoadTrailerV4();
+ if (!trailer_dict) {
+ return false;
+ }
+ xref_offset = trailer_dict->GetDirectIntegerFor("Prev");
}
// Check for circular references.
- if (pdfium::Contains(seen_xref_offset, xref_offset))
+ if (pdfium::Contains(seen_xref_offset, xref_offset)) {
return false;
+ }
}
m_ObjectStreamMap.clear();
m_bXRefStream = true;
@@ -893,9 +907,10 @@
const ObjectType existing_type = GetObjectType(obj_num);
if (existing_type == ObjectType::kNull) {
const uint32_t offset = GetSecondXRefStreamEntry(entry_span, field_widths);
- if (pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(offset))
+ if (pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(offset)) {
m_CrossRefTable->AddNormal(obj_num, 0, /*is_object_stream=*/false,
offset);
+ }
return;
}
@@ -910,9 +925,10 @@
if (type == ObjectType::kNormal) {
const uint32_t offset = GetSecondXRefStreamEntry(entry_span, field_widths);
- if (pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(offset))
+ if (pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(offset)) {
m_CrossRefTable->AddNormal(obj_num, 0, /*is_object_stream=*/false,
offset);
+ }
return;
}
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index 77d15a8..96c84d12 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -659,9 +659,6 @@
# TODO(pdfium:2001): Remove after associated bug is fixed
bug_2001.pdf * * * *
-# TODO(pdfium:2123): Remove after associated bug is fixed
-bug_2123.pdf * * * *
-
# TODO(chromium:237527): Remove after associated bug is fixed
bug_237527_1.in * * * *
