Revert ProcessCrossRefV5Entry() overwrite change
This effectively reverts the code changes in
https://pdfium-review.googlesource.com/112932 and puts the
bug_1484283.pdf pixel test back in a suppressed state, while retaining
some of the code cleanups that do not affect the bug. In the reverted
state, bug_2122.pdf renders correctly again, so unsuppress that test
case.
A follow-up CL [1] will fix the bug_1484283.pdf pixel test in a
different manner. Do that separately to make the changes clearer.
[1] https://pdfium-review.googlesource.com/116093
Bug: chromium:1484283,pdfium:2122
Change-Id: I4dabeac00ec236458d20d29b5e65992c76b6eddc
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116092
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@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 f1d9cbc..43984d0 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -418,15 +418,12 @@
std::move(m_CrossRefTable));
}
- // Traverse the xref data structures from oldest to newest. So entries from
- // later iterations should overwrite existing entries.
for (size_t i = 0; i < xref_list.size(); ++i) {
if (xref_list[i] > 0 && !LoadCrossRefV4(xref_list[i], false))
return false;
if (xref_stream_list[i] > 0 &&
- !LoadCrossRefV5(&xref_stream_list[i], /*is_main_xref=*/false,
- /*overwrite_existing=*/true)) {
+ !LoadCrossRefV5(&xref_stream_list[i], /*is_main_xref=*/false)) {
return false;
}
@@ -492,20 +489,16 @@
}
if (xref_stream_list[0] > 0 &&
- !LoadCrossRefV5(&xref_stream_list[0], /*is_main_xref=*/false,
- /*overwrite_existing=*/true)) {
+ !LoadCrossRefV5(&xref_stream_list[0], /*is_main_xref=*/false)) {
return false;
}
- // Traverse the xref data structures from oldest to newest. So entries from
- // later iterations should overwrite existing entries.
for (size_t i = 1; i < xref_list.size(); ++i) {
if (xref_list[i] > 0 && !LoadCrossRefV4(xref_list[i], false))
return false;
if (xref_stream_list[i] > 0 &&
- !LoadCrossRefV5(&xref_stream_list[i], /*is_main_xref=*/false,
- /*overwrite_existing=*/true)) {
+ !LoadCrossRefV5(&xref_stream_list[i], /*is_main_xref=*/false)) {
return false;
}
}
@@ -668,18 +661,14 @@
}
bool CPDF_Parser::LoadAllCrossRefV5(FX_FILESIZE xref_offset) {
- if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/true,
- /*overwrite_existing=*/false)) {
+ if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/true)) {
return false;
}
- // Traverse the xref objects from newest to older. So entries from later
- // iterations should not overwrite existing entries.
std::set<FX_FILESIZE> seen_xref_offset;
while (xref_offset > 0) {
seen_xref_offset.insert(xref_offset);
- if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false,
- /*overwrite_existing=*/false)) {
+ if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false)) {
return false;
}
@@ -774,9 +763,7 @@
return GetTrailer() && !m_CrossRefTable->objects_info().empty();
}
-bool CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos,
- bool is_main_xref,
- bool overwrite_existing) {
+bool CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, bool is_main_xref) {
RetainPtr<const CPDF_Stream> pStream =
ToStream(ParseIndirectObjectAt(*pos, 0));
if (!pStream || !pStream->GetObjNum()) {
@@ -863,7 +850,7 @@
}
ProcessCrossRefV5Entry(seg_span.subspan(i * total_width, total_width),
- field_widths, obj_num, overwrite_existing);
+ field_widths, obj_num);
}
segindex += index.obj_count;
@@ -874,8 +861,7 @@
void CPDF_Parser::ProcessCrossRefV5Entry(
pdfium::span<const uint8_t> entry_span,
pdfium::span<const uint32_t> field_widths,
- uint32_t obj_num,
- bool overwrite_existing) {
+ uint32_t obj_num) {
DCHECK_GE(field_widths.size(), kMinFieldCount);
ObjectType type;
if (field_widths[0]) {
@@ -901,7 +887,7 @@
return;
}
- if (!overwrite_existing && existing_type != ObjectType::kFree) {
+ if (existing_type != ObjectType::kFree) {
return;
}
@@ -1131,8 +1117,7 @@
m_LastXRefOffset = m_pLinearized->GetLastXRefOffset();
FX_FILESIZE dwFirstXRefOffset = m_LastXRefOffset;
bool bLoadV4 = LoadCrossRefV4(dwFirstXRefOffset, false);
- if (!bLoadV4 && !LoadCrossRefV5(&dwFirstXRefOffset, /*is_main_xref=*/true,
- /*overwrite_existing=*/false)) {
+ if (!bLoadV4 && !LoadCrossRefV5(&dwFirstXRefOffset, /*is_main_xref=*/true)) {
if (!RebuildCrossRef())
return FORMAT_ERROR;
@@ -1198,18 +1183,14 @@
bool CPDF_Parser::LoadLinearizedAllCrossRefV5(FX_FILESIZE main_xref_offset) {
FX_FILESIZE xref_offset = main_xref_offset;
- if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false,
- /*overwrite_existing=*/false)) {
+ if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false)) {
return false;
}
- // Traverse the xref objects from newest to older. So entries from later
- // iterations should not overwrite existing entries.
std::set<FX_FILESIZE> seen_xref_offset;
while (xref_offset) {
seen_xref_offset.insert(xref_offset);
- if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false,
- /*overwrite_existing=*/false)) {
+ if (!LoadCrossRefV5(&xref_offset, /*is_main_xref=*/false)) {
return false;
}
diff --git a/core/fpdfapi/parser/cpdf_parser.h b/core/fpdfapi/parser/cpdf_parser.h
index a2d429c..7949854 100644
--- a/core/fpdfapi/parser/cpdf_parser.h
+++ b/core/fpdfapi/parser/cpdf_parser.h
@@ -145,13 +145,10 @@
bool LoadAllCrossRefV4(FX_FILESIZE xref_offset);
bool LoadAllCrossRefV5(FX_FILESIZE xref_offset);
- bool LoadCrossRefV5(FX_FILESIZE* pos,
- bool is_main_xref,
- bool overwrite_existing);
+ bool LoadCrossRefV5(FX_FILESIZE* pos, bool is_main_xref);
void ProcessCrossRefV5Entry(pdfium::span<const uint8_t> entry_span,
pdfium::span<const uint32_t> field_widths,
- uint32_t obj_num,
- bool overwrite_existing);
+ uint32_t obj_num);
RetainPtr<CPDF_Dictionary> LoadTrailerV4();
Error SetEncryptHandler();
void ReleaseEncryptHandler();
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index ac75c08d..28c98ff 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -639,6 +639,9 @@
# 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 * * * *
@@ -659,9 +662,6 @@
# TODO(pdfium:2001): Remove after associated bug is fixed
bug_2001.pdf * * * *
-# TODO(pdfium:2122): Remove after associated bug is fixed
-bug_2122.pdf * * * *
-
# TODO(chromium:237527): Remove after associated bug is fixed
bug_237527_1.in * * * *
