Fix FPDF_FILEACCESS lifetime issue with FPDF_LoadCustomDocument().
The FPDF_FILEACCESS that gets passed to FPDF_LoadCustomDocument() does
not need to outlive the returned FPDF_DOCUMENT. See bug for discussion.
This is a partial revert of commit b2ae83e8, along with a test case for
this scenario and a clarification in the public API comments.
BUG=pdfium:1261
Change-Id: Ib1d1b759fe3af98aa4ecc1d8387a259ee4ffe07f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/51570
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_customaccess.cpp b/fpdfsdk/cpdfsdk_customaccess.cpp
index fcaede5..acdf93a 100644
--- a/fpdfsdk/cpdfsdk_customaccess.cpp
+++ b/fpdfsdk/cpdfsdk_customaccess.cpp
@@ -7,14 +7,12 @@
#include "fpdfsdk/cpdfsdk_customaccess.h"
CPDFSDK_CustomAccess::CPDFSDK_CustomAccess(FPDF_FILEACCESS* pFileAccess)
- : m_pFileAccess(pFileAccess) {
- ASSERT(m_pFileAccess);
-}
+ : m_FileAccess(*pFileAccess) {}
CPDFSDK_CustomAccess::~CPDFSDK_CustomAccess() = default;
FX_FILESIZE CPDFSDK_CustomAccess::GetSize() {
- return m_pFileAccess->m_FileLen;
+ return m_FileAccess.m_FileLen;
}
bool CPDFSDK_CustomAccess::ReadBlockAtOffset(void* buffer,
@@ -26,6 +24,6 @@
FX_SAFE_FILESIZE new_pos = pdfium::base::checked_cast<FX_FILESIZE>(size);
new_pos += offset;
return new_pos.IsValid() && new_pos.ValueOrDie() <= GetSize() &&
- m_pFileAccess->m_GetBlock(m_pFileAccess->m_Param, offset,
- static_cast<uint8_t*>(buffer), size);
+ m_FileAccess.m_GetBlock(m_FileAccess.m_Param, offset,
+ static_cast<uint8_t*>(buffer), size);
}
diff --git a/fpdfsdk/cpdfsdk_customaccess.h b/fpdfsdk/cpdfsdk_customaccess.h
index 341d4ab..76940ce 100644
--- a/fpdfsdk/cpdfsdk_customaccess.h
+++ b/fpdfsdk/cpdfsdk_customaccess.h
@@ -25,7 +25,7 @@
explicit CPDFSDK_CustomAccess(FPDF_FILEACCESS* pFileAccess);
~CPDFSDK_CustomAccess() override;
- UnownedPtr<FPDF_FILEACCESS> const m_pFileAccess;
+ FPDF_FILEACCESS m_FileAccess;
};
#endif // FPDFSDK_CPDFSDK_CUSTOMACCESS_H_
diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index cade1ca..3a314ba 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -276,6 +276,37 @@
EXPECT_FALSE(FPDF_LoadCustomDocument(nullptr, ""));
}
+// See https://crbug.com/pdfium/1261
+TEST_F(FPDFViewEmbedderTest, LoadCustomDocumentWithShortLivedFileAccess) {
+ std::string file_contents_string; // Must outlive |doc|.
+ ScopedFPDFDocument doc;
+ {
+ // Read a PDF, and copy it into |file_contents_string|.
+ std::string pdf_path;
+ size_t pdf_length;
+ ASSERT_TRUE(PathService::GetTestFilePath("rectangles.pdf", &pdf_path));
+ auto file_contents = GetFileContents(pdf_path.c_str(), &pdf_length);
+ ASSERT_TRUE(file_contents);
+ for (size_t i = 0; i < pdf_length; ++i)
+ file_contents_string.push_back(file_contents.get()[i]);
+
+ // Define a FPDF_FILEACCESS object that will go out of scope, while the
+ // loaded document in |doc| remains valid.
+ FPDF_FILEACCESS file_access = {};
+ file_access.m_FileLen = pdf_length;
+ file_access.m_GetBlock = GetBlockFromString;
+ file_access.m_Param = &file_contents_string;
+ doc.reset(FPDF_LoadCustomDocument(&file_access, nullptr));
+ ASSERT_TRUE(doc);
+ }
+
+ // Now try to access |doc| and make sure it still works.
+ ScopedFPDFPage page(FPDF_LoadPage(doc.get(), 0));
+ ASSERT_TRUE(page);
+ EXPECT_DOUBLE_EQ(200, FPDF_GetPageWidth(page.get()));
+ EXPECT_DOUBLE_EQ(300, FPDF_GetPageHeight(page.get()));
+}
+
TEST_F(FPDFViewEmbedderTest, Page) {
EXPECT_TRUE(OpenDocument("about_blank.pdf"));
FPDF_PAGE page = LoadPage(0);
diff --git a/public/fpdfview.h b/public/fpdfview.h
index acd7400..8892da5 100644
--- a/public/fpdfview.h
+++ b/public/fpdfview.h
@@ -453,10 +453,11 @@
// Return value:
// A handle to the loaded document, or NULL on failure.
// Comments:
-// The application must keep the file resources valid until the PDF
-// document is closed.
+// The application must keep the file resources |pFileAccess| points to
+// valid until the returned FPDF_DOCUMENT is closed. |pFileAccess|
+// itself does not need to outlive the FPDF_DOCUMENT.
//
-// The loaded document can be closed with FPDF_CloseDocument.
+// The loaded document can be closed with FPDF_CloseDocument().
//
// See the comments for FPDF_LoadDocument() regarding the encoding for
// |password|.
