Fix page movement handling in XFA-based PDFs
Fixes issues with FPDF_MovePages() when used on XFA-enabled PDFs by
adding separate logic for XFA extensions. This ensures consistent page
movement behavior between standard and XFA-based documents.
The patch refines handling of XFA objects during page move operations to
preserve form data and structure. Tests for both page movement and
insertion in XFA PDFs have been added and verified.
Original patch and code contributions by Stefan Ziegler, with further
modifications and testing to ensure stability and backward
compatibility.
Bug: 433689235
Change-Id: Ifc6ad58d149cfb9ec31d3cdb12dc761ef57493ec
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/134810
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/AUTHORS b/AUTHORS
index 612fdbc..bb9fa62 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -15,6 +15,7 @@
Abdelkarim Sellamna <abdelkarim.se@gmail.com>
Aleksei Skotnikov <fineaskotnikov@gmail.com>
Antonio Gomes <tonikitoo@igalia.com>
+Aryan P Krishnan <aryankrishnan4b@gmail.com>
Chenguang Shao <chenguangshao1@gmail.com>
Chery Cherian <cherycherian@gmail.com>
Claudio DeSouza <claudiomdsjr@gmail.com>
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index 873e058..5a2df04 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -697,8 +697,8 @@
// Check for if XFA is enabled.
Extension* extension = GetExtension();
- if (extension && extension->ContainsExtensionForm()) {
- // Don't manipulate XFA PDFs.
+ if (extension && extension->ContainsExtensionFullForm()) {
+ // Don't manipulate XFA-full PDFs.
return false;
}
@@ -748,6 +748,10 @@
}
}
+ if (extension) {
+ extension->PagesInserted(dest_page_index, pages_to_move.size());
+ }
+
return true;
}
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index e22f546..e6924ab 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -37,6 +37,7 @@
virtual bool ContainsExtensionForm() const = 0;
virtual bool ContainsExtensionFullForm() const = 0;
virtual bool ContainsExtensionForegroundForm() const = 0;
+ virtual void PagesInserted(int page_index, size_t num_pages) = 0;
};
class LinkListIface {
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index 3f75cae..91ec4b1 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -88,26 +88,41 @@
return false;
}
+ size_t length_size_t = pdfium::checked_cast<size_t>(length);
+ CPDF_Document::Extension* extension = cdest_doc->GetExtension();
+
CPDF_PageExporter exporter(cdest_doc, csrc_doc);
if (!page_indices) {
std::vector<uint32_t> page_indices_vec(csrc_doc->GetPageCount());
std::iota(page_indices_vec.begin(), page_indices_vec.end(), 0);
- return exporter.ExportPages(page_indices_vec, index);
+ if (!exporter.ExportPages(page_indices_vec, index)) {
+ return false;
+ }
+ if (extension) {
+ extension->PagesInserted(index, length_size_t);
+ }
+ return true;
}
- if (length == 0) {
+ if (length_size_t == 0) {
return false;
}
// SAFETY: required from caller.
- auto page_span = UNSAFE_BUFFERS(pdfium::span(page_indices, length));
+ auto page_span = UNSAFE_BUFFERS(pdfium::span(page_indices, length_size_t));
for (int page_index : page_span) {
if (page_index < 0) {
return false;
}
}
- return exporter.ExportPages(
- fxcrt::reinterpret_span<const uint32_t>(page_span), index);
+ if (!exporter.ExportPages(fxcrt::reinterpret_span<const uint32_t>(page_span),
+ index)) {
+ return false;
+ }
+ if (extension) {
+ extension->PagesInserted(index, length_size_t);
+ }
+ return true;
}
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDF_ImportPages(FPDF_DOCUMENT dest_doc,
@@ -130,7 +145,16 @@
}
CPDF_PageExporter exporter(cdest_doc, csrc_doc);
- return exporter.ExportPages(page_indices, index);
+ if (!exporter.ExportPages(page_indices, index)) {
+ return false;
+ }
+
+ CPDF_Document::Extension* extension = cdest_doc->GetExtension();
+ if (extension) {
+ extension->PagesInserted(index, page_indices.size());
+ }
+
+ return true;
}
FPDF_EXPORT FPDF_DOCUMENT FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_ppo_embeddertest.cpp b/fpdfsdk/fpdf_ppo_embeddertest.cpp
index 5e88207..10118be 100644
--- a/fpdfsdk/fpdf_ppo_embeddertest.cpp
+++ b/fpdfsdk/fpdf_ppo_embeddertest.cpp
@@ -32,7 +32,14 @@
namespace {
-class FPDFPPOEmbedderTest : public EmbedderTest {};
+class FPDFPPOEmbedderTest : public EmbedderTest {
+ public:
+ std::string GetPageChecksum(FPDF_DOCUMENT doc, int index) {
+ ScopedFPDFPage page(FPDF_LoadPage(doc, index));
+ auto bitmap = RenderPage(page.get());
+ return HashBitmap(bitmap.get());
+ }
+};
int FakeBlockWriter(FPDF_FILEWRITE* pThis,
const void* pData,
@@ -699,3 +706,70 @@
CloseSavedPage(page);
}
}
+
+TEST_F(FPDFPPOEmbedderTest, XFAImportTest) {
+ // Load dest_doc with OpenDocument for XFA extension
+ ASSERT_TRUE(OpenDocument("rectangles_multi_page_xfa.pdf"));
+ FPDF_DOCUMENT dest_doc = document();
+ ASSERT_TRUE(dest_doc);
+ EXPECT_EQ(FPDF_GetFormType(dest_doc), FORMTYPE_XFA_FOREGROUND);
+ EXPECT_EQ(FPDF_GetPageCount(dest_doc), 5);
+
+ // Capture initial checksums of dest_doc (all original pages)
+ std::vector<std::string> dest_doc_checksums;
+ for (int i = 0; i < 5; i++) {
+ dest_doc_checksums.push_back(GetPageChecksum(dest_doc, i));
+ }
+
+ ScopedFPDFDocument source_doc(FPDF_LoadDocument(
+ PathService::GetTestFilePath("rectangles_multi_pages.pdf").c_str(),
+ nullptr));
+ ASSERT_TRUE(source_doc);
+ EXPECT_EQ(FPDF_GetFormType(source_doc.get()), FORMTYPE_NONE);
+ EXPECT_EQ(FPDF_GetPageCount(source_doc.get()), 5);
+
+ // Capture checksums of source_doc (all pages)
+ std::vector<std::string> source_doc_checksums;
+ for (int i = 0; i < 5; i++) {
+ source_doc_checksums.push_back(GetPageChecksum(source_doc.get(), i));
+ }
+
+ // Insert 3 pages into `dest_doc`
+ ASSERT_TRUE(FPDF_ImportPages(dest_doc, source_doc.get(), "1, 4, 2", 4));
+ EXPECT_EQ(FPDF_GetPageCount(dest_doc), 8);
+
+ for (int i = 0; i < 4; i++) {
+ EXPECT_EQ(GetPageChecksum(dest_doc, i), dest_doc_checksums[i]);
+ }
+
+ EXPECT_EQ(GetPageChecksum(dest_doc, 4), source_doc_checksums[0]);
+ EXPECT_EQ(GetPageChecksum(dest_doc, 5), source_doc_checksums[3]);
+ EXPECT_EQ(GetPageChecksum(dest_doc, 6), source_doc_checksums[1]);
+
+ EXPECT_EQ(GetPageChecksum(dest_doc, 7), dest_doc_checksums[4]);
+}
+
+TEST_F(FPDFPPOEmbedderTest, XFAMoveTest) {
+ // Load the document with OpenDocument for XFA extension
+ ASSERT_TRUE(OpenDocument("rectangles_multi_page_xfa.pdf"));
+ ASSERT_TRUE(document());
+ EXPECT_EQ(FPDF_GetFormType(document()), FORMTYPE_XFA_FOREGROUND);
+ EXPECT_EQ(FPDF_GetPageCount(document()), 5);
+
+ // Capture initial checksums of the document (all original pages)
+ std::vector<std::string> target_doc_checksums;
+ for (int i = 0; i < 5; i++) {
+ target_doc_checksums.push_back(GetPageChecksum(document(), i));
+ }
+
+ // Move 3 pages to the start of the PDF
+ constexpr int kPages[] = {1, 2, 3};
+ ASSERT_TRUE(FPDF_MovePages(document(), kPages, std::size(kPages), 0));
+ EXPECT_EQ(FPDF_GetPageCount(document()), 5);
+
+ EXPECT_EQ(GetPageChecksum(document(), 0), target_doc_checksums[1]);
+ EXPECT_EQ(GetPageChecksum(document(), 1), target_doc_checksums[2]);
+ EXPECT_EQ(GetPageChecksum(document(), 2), target_doc_checksums[3]);
+ EXPECT_EQ(GetPageChecksum(document(), 3), target_doc_checksums[0]);
+ EXPECT_EQ(GetPageChecksum(document(), 4), target_doc_checksums[4]);
+}
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
index 48a2e3a..c3806d3 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
@@ -301,8 +301,12 @@
// if it's a valid page in the document.
uint32_t page_obj_num = pdfdoc_->DeletePage(page_index);
+ if (page_obj_num != 0) {
+ --page_count_;
+ }
+
if (fxcrt::IndexInBounds(xfa_page_list_, page_index)) {
- xfa_page_list_.erase(xfa_page_list_.begin() + page_index);
+ xfa_page_list_.erase(std::next(xfa_page_list_.begin(), page_index));
for (int i = page_index; i < fxcrt::CollectionSize<int>(xfa_page_list_);
i++) {
if (xfa_page_list_[i]) {
@@ -314,6 +318,14 @@
return page_obj_num;
}
+void CPDFXFA_Context::PagesInserted(int page_index, size_t num_pages) {
+ if (fxcrt::IndexInBounds(xfa_page_list_, page_index)) {
+ xfa_page_list_.insert(std::next(xfa_page_list_.begin(), page_index),
+ num_pages, nullptr);
+ page_count_ += num_pages;
+ }
+}
+
bool CPDFXFA_Context::ContainsExtensionForm() const {
return form_type_ == FormType::kXFAFull ||
form_type_ == FormType::kXFAForeground;
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.h b/fpdfsdk/fpdfxfa/cpdfxfa_context.h
index 45fe053..2434b45 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.h
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.h
@@ -73,6 +73,7 @@
bool ContainsExtensionForm() const override;
bool ContainsExtensionFullForm() const override;
bool ContainsExtensionForegroundForm() const override;
+ void PagesInserted(int page_index, size_t num_pages) override;
// CXFA_FFApp::CallbackIface:
WideString GetLanguage() override;
diff --git a/testing/resources/rectangles_multi_page_xfa.pdf b/testing/resources/rectangles_multi_page_xfa.pdf
new file mode 100644
index 0000000..d3d867a
--- /dev/null
+++ b/testing/resources/rectangles_multi_page_xfa.pdf
Binary files differ
