Add a way to undo a previous FPDF_SetSystemFontInfo() call
Modify FPDF_SetSystemFontInfo() to be able to take a nullptr without
crashing. Handle the nullptr case by releasing PDFium's hold on the
previously passed in pointer. Since no existing embedder could have
passed in a nullptr before without crashing, this is a safe enhancement
to an existing stable API.
This gives FPDF_SetSystemFontInfo() callers that need to also call
FPDF_FreeDefaultSystemFontInfo() a way to safely release PDFium's
pointer to the default system font info, before freeing the font info.
Use this where appropriate to balance out prior FPDF_SetSystemFontInfo()
calls in existing code.
Update pdfium_test.cc to make the call at the appropriate time, before
the FPDF_DestroyLibrary() call, by adjusting std::unique_ptr lifetimes.
In a follow-up CL, PartitionAllocator class instances will stop living
forever. Thus after FPDF_DestroyLibrary(), it is too late to call
FPDF_FreeDefaultSystemFontInfo(), as the font info was allocated inside
a destroyed partition.
Bug: pdfium:1876
Change-Id: Ic3835bae5f95604f89afb35bfaf8c0d568c6bc4b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116771
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_sysfontinfo.cpp b/fpdfsdk/fpdf_sysfontinfo.cpp
index afb873e..57fdbc0 100644
--- a/fpdfsdk/fpdf_sysfontinfo.cpp
+++ b/fpdfsdk/fpdf_sysfontinfo.cpp
@@ -155,10 +155,17 @@
FPDF_EXPORT void FPDF_CALLCONV
FPDF_SetSystemFontInfo(FPDF_SYSFONTINFO* pFontInfoExt) {
+ auto* mapper = CFX_GEModule::Get()->GetFontMgr()->GetBuiltinMapper();
+ if (!pFontInfoExt) {
+ std::unique_ptr<SystemFontInfoIface> info = mapper->TakeSystemFontInfo();
+ // Delete `info` when it goes out of scope here.
+ return;
+ }
+
if (pFontInfoExt->version != 1)
return;
- CFX_GEModule::Get()->GetFontMgr()->GetBuiltinMapper()->SetSystemFontInfo(
+ mapper->SetSystemFontInfo(
std::make_unique<CFX_ExternalFontInfo>(pFontInfoExt));
#ifdef PDF_ENABLE_XFA
diff --git a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
index 24df525..589cf71 100644
--- a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
+++ b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
@@ -77,9 +77,10 @@
}
void TearDown() override {
+ FPDF_SetSystemFontInfo(nullptr);
EmbedderTest::TearDown();
- // Bouncing the library is the only reliable way to undo the
+ // Bouncing the library is the only reliable way to fully undo the initial
// FPDF_SetSystemFontInfo() call at the moment.
EmbedderTestEnvironment::GetInstance()->TearDown();
EmbedderTestEnvironment::GetInstance()->SetUp();
@@ -103,13 +104,14 @@
void TearDown() override {
EmbedderTest::TearDown();
- // Bouncing the library is the only reliable way to undo the
+ // After releasing `font_info_` from PDFium, it is safe to free it.
+ FPDF_SetSystemFontInfo(nullptr);
+ FPDF_FreeDefaultSystemFontInfo(font_info_);
+
+ // Bouncing the library is the only reliable way to fully undo the initial
// FPDF_SetSystemFontInfo() call at the moment.
EmbedderTestEnvironment::GetInstance()->TearDown();
- // After shutdown, it is safe to release the font info.
- FPDF_FreeDefaultSystemFontInfo(font_info_);
-
EmbedderTestEnvironment::GetInstance()->SetUp();
}
diff --git a/public/fpdf_sysfontinfo.h b/public/fpdf_sysfontinfo.h
index e30220c..0ba711d 100644
--- a/public/fpdf_sysfontinfo.h
+++ b/public/fpdf_sysfontinfo.h
@@ -275,6 +275,9 @@
* Platform support implementation should implement required methods of
* FFDF_SYSFONTINFO interface, then call this function during PDFium
* initialization process.
+ *
+ * Call this with NULL to tell PDFium to stop using a previously set
+ * |FPDF_SYSFONTINFO|.
*/
FPDF_EXPORT void FPDF_CALLCONV
FPDF_SetSystemFontInfo(FPDF_SYSFONTINFO* pFontInfo);
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc
index 94fd6a8..27b0d72 100644
--- a/samples/pdfium_test.cc
+++ b/samples/pdfium_test.cc
@@ -1944,63 +1944,69 @@
FPDF_InitLibraryWithConfig(&config);
- std::unique_ptr<FontRenamer> font_renamer;
- if (options.croscore_font_names)
- font_renamer = std::make_unique<FontRenamer>();
-
- UNSUPPORT_INFO unsupported_info = {};
- unsupported_info.version = 1;
- unsupported_info.FSDK_UnSupport_Handler = ExampleUnsupportedHandler;
-
- FSDK_SetUnSpObjProcessHandler(&unsupported_info);
-
- if (options.time > -1) {
- // This must be a static var to avoid explicit capture, so the lambda can be
- // converted to a function ptr.
- static time_t time_ret = options.time;
- FSDK_SetTimeFunction([]() { return time_ret; });
- FSDK_SetLocaltimeFunction([](const time_t* tp) { return gmtime(tp); });
- }
-
- Processor processor(&options, &idler);
- for (const std::string& filename : files) {
- std::vector<uint8_t> file_contents = GetFileContents(filename.c_str());
- if (file_contents.empty()) {
- continue;
+ {
+ std::unique_ptr<FontRenamer> font_renamer;
+ if (options.croscore_font_names) {
+ // Must be destroyed before FPDF_DestroyLibrary().
+ font_renamer = std::make_unique<FontRenamer>();
}
- fprintf(stderr, "Processing PDF file %s.\n", filename.c_str());
+
+ UNSUPPORT_INFO unsupported_info = {};
+ unsupported_info.version = 1;
+ unsupported_info.FSDK_UnSupport_Handler = ExampleUnsupportedHandler;
+
+ FSDK_SetUnSpObjProcessHandler(&unsupported_info);
+
+ if (options.time > -1) {
+ // This must be a static var to avoid explicit capture, so the lambda can
+ // be converted to a function ptr.
+ static time_t time_ret = options.time;
+ FSDK_SetTimeFunction([]() { return time_ret; });
+ FSDK_SetLocaltimeFunction([](const time_t* tp) { return gmtime(tp); });
+ }
+
+ Processor processor(&options, &idler);
+ for (const std::string& filename : files) {
+ std::vector<uint8_t> file_contents = GetFileContents(filename.c_str());
+ if (file_contents.empty()) {
+ continue;
+ }
+ fprintf(stderr, "Processing PDF file %s.\n", filename.c_str());
#ifdef ENABLE_CALLGRIND
- if (options.callgrind_delimiters)
- CALLGRIND_START_INSTRUMENTATION;
+ if (options.callgrind_delimiters) {
+ CALLGRIND_START_INSTRUMENTATION;
+ }
#endif // ENABLE_CALLGRIND
- std::string events;
- if (options.send_events) {
- std::string event_filename = filename;
- size_t extension_pos = event_filename.find(".pdf");
- if (extension_pos != std::string::npos) {
- event_filename.replace(extension_pos, 4, ".evt");
- if (access(event_filename.c_str(), R_OK) == 0) {
- fprintf(stderr, "Using event file %s.\n", event_filename.c_str());
- std::vector<uint8_t> event_contents =
- GetFileContents(event_filename.c_str());
- if (!event_contents.empty()) {
- fprintf(stderr, "Sending events from: %s\n",
- event_filename.c_str());
- std::copy(event_contents.begin(), event_contents.end(),
- std::back_inserter(events));
+ std::string events;
+ if (options.send_events) {
+ std::string event_filename = filename;
+ size_t extension_pos = event_filename.find(".pdf");
+ if (extension_pos != std::string::npos) {
+ event_filename.replace(extension_pos, 4, ".evt");
+ if (access(event_filename.c_str(), R_OK) == 0) {
+ fprintf(stderr, "Using event file %s.\n", event_filename.c_str());
+ std::vector<uint8_t> event_contents =
+ GetFileContents(event_filename.c_str());
+ if (!event_contents.empty()) {
+ fprintf(stderr, "Sending events from: %s\n",
+ event_filename.c_str());
+ std::copy(event_contents.begin(), event_contents.end(),
+ std::back_inserter(events));
+ }
}
}
}
- }
- processor.ProcessPdf(filename, file_contents, events);
+ processor.ProcessPdf(filename, file_contents, events);
#ifdef ENABLE_CALLGRIND
- if (options.callgrind_delimiters)
- CALLGRIND_STOP_INSTRUMENTATION;
+ if (options.callgrind_delimiters) {
+ CALLGRIND_STOP_INSTRUMENTATION;
+ }
#endif // ENABLE_CALLGRIND
+ }
}
FPDF_DestroyLibrary();
diff --git a/testing/font_renamer.cpp b/testing/font_renamer.cpp
index 6a3a81c..1f3dd49 100644
--- a/testing/font_renamer.cpp
+++ b/testing/font_renamer.cpp
@@ -87,5 +87,6 @@
}
FontRenamer::~FontRenamer() {
+ FPDF_SetSystemFontInfo(nullptr);
FPDF_FreeDefaultSystemFontInfo(impl_.ExtractAsDangling());
}
