Merge pull request #1530 from m-holger/afdh

Cache QPDFAcroFormDocumentHelpers

Merge pull request #1530 from m-holger/afdh
Cache QPDFAcroFormDocumentHelpers
m-holger · GitHub
2 parents ef14676b e6200bbc
Showing 8 changed files with 118 additions and 102 deletions
include/qpdf/QPDF.hh
include/qpdf/QPDFAcroFormDocumentHelper.hh
libqpdf/QPDFAcroFormDocumentHelper.cc
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDFJob.cc
libqpdf/QPDFPageDocumentHelper.cc
libqpdf/qpdf/QPDF_private.hh
qpdf/qpdf.testcov
@@ -57,6 +57,7 @@ class BitWriter;
 class BufferInputSource;
 class QPDFLogger;
 class QPDFParser;
+class QPDFAcroFormDocumentHelper;
 class QPDF
 {
@@ -792,6 +793,7 @@ class QPDF
     class JobSetter;
     inline bool reconstructed_xref() const;
+    inline QPDFAcroFormDocumentHelper& acroform();
     // For testing only -- do not add to DLL
     static bool test_json_validators();
@@ -68,6 +68,21 @@
 class QPDFAcroFormDocumentHelper: public QPDFDocumentHelper
 {
   public:
+    // Get a shared document helper for a given QPDF object.
+    //
+    // Retrieving a document helper for a QPDF object rather than creating a new one avoids repeated
+    // validation of the Acroform structure, which can be expensive.
+    QPDF_DLL
+    static QPDFAcroFormDocumentHelper& get(QPDF& qpdf);
+
+    // Re-validate the AcroForm structure. This is useful if you have modified the structure of the
+    // AcroForm dictionary in a way that would invalidate the cache.
+    //
+    // If repair is true, the document will be repaired if possible if the validation encounters
+    // errors.
+    QPDF_DLL
+    void validate(bool repair = true);
+
     QPDF_DLL
     QPDFAcroFormDocumentHelper(QPDF&);
@@ -226,23 +241,7 @@ class QPDFAcroFormDocumentHelper: public QPDFDocumentHelper
     void adjustAppearanceStream(
         QPDFObjectHandle stream, std::map<std::string, std::map<std::string, std::string>> dr_map);
-    class Members
-    {
-        friend class QPDFAcroFormDocumentHelper;
-
-      public:
-        ~Members() = default;
-
-      private:
-        Members() = default;
-        Members(Members const&) = delete;
-
-        bool cache_valid{false};
-        std::map<QPDFObjGen, std::vector<QPDFAnnotationObjectHelper>> field_to_annotations;
-        std::map<QPDFObjGen, QPDFFormFieldObjectHelper> annotation_to_field;
-        std::map<QPDFObjGen, std::string> field_to_name;
-        std::map<std::string, std::set<QPDFObjGen>> name_to_fields;
-    };
+    class Members;
     std::shared_ptr<Members> m;
 };
@@ -3,6 +3,7 @@
 #include <qpdf/Pl_Buffer.hh>
 #include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QPDFPageDocumentHelper.hh>
+#include <qpdf/QPDF_private.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
 #include <qpdf/ResourceFinder.hh>
@@ -12,15 +13,42 @@
 using namespace qpdf;
 using namespace std::literals;
+class QPDFAcroFormDocumentHelper::Members
+{
+  public:
+    Members() = default;
+    Members(Members const&) = delete;
+    ~Members() = default;
+
+    bool cache_valid{false};
+    std::map<QPDFObjGen, std::vector<QPDFAnnotationObjectHelper>> field_to_annotations;
+    std::map<QPDFObjGen, QPDFFormFieldObjectHelper> annotation_to_field;
+    std::map<QPDFObjGen, std::string> field_to_name;
+    std::map<std::string, std::set<QPDFObjGen>> name_to_fields;
+};
+
 QPDFAcroFormDocumentHelper::QPDFAcroFormDocumentHelper(QPDF& qpdf) :
     QPDFDocumentHelper(qpdf),
-    m(new Members())
+    m(std::make_shared<Members>())
 {
     // We have to analyze up front. Otherwise, when we are adding annotations and fields, we are in
     // a temporarily unstable configuration where some widget annotations are not reachable.
     analyze();
 }
+QPDFAcroFormDocumentHelper&
+QPDFAcroFormDocumentHelper::get(QPDF& qpdf)
+{
+    return qpdf.acroform();
+}
+
+void
+QPDFAcroFormDocumentHelper::validate(bool repair)
+{
+    invalidateCache();
+    analyze();
+}
+
 void
 QPDFAcroFormDocumentHelper::invalidateCache()
 {
@@ -5,6 +5,7 @@
 #include <qpdf/QPDFAcroFormDocumentHelper.hh>
 #include <qpdf/QPDFAnnotationObjectHelper.hh>
 #include <qpdf/QPDFObjectHandle_private.hh>
+#include <qpdf/QPDF_private.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
 #include <cstdlib>
@@ -88,23 +89,21 @@ QPDFFormFieldObjectHelper::getInheritableFieldValue(std::string const&amp; name)
 std::string
 QPDFFormFieldObjectHelper::getInheritableFieldValueAsString(std::string const& name)
 {
-    QPDFObjectHandle fv = getInheritableFieldValue(name);
-    std::string result;
+    auto fv = getInheritableFieldValue(name);
     if (fv.isString()) {
-        result = fv.getUTF8Value();
+        return fv.getUTF8Value();
     }
-    return result;
+    return {};
 }
 std::string
 QPDFFormFieldObjectHelper::getInheritableFieldValueAsName(std::string const& name)
 {
-    QPDFObjectHandle fv = getInheritableFieldValue(name);
-    std::string result;
+    auto fv = getInheritableFieldValue(name);
     if (fv.isName()) {
-        result = fv.getName();
+        return fv.getName();
     }
-    return result;
+    return {};
 }
 std::string
@@ -203,12 +202,11 @@ QPDFFormFieldObjectHelper::getDefaultAppearance()
         value = getFieldFromAcroForm("/DA");
         looked_in_acroform = true;
     }
-    std::string result;
     if (value.isString()) {
         QTC::TC("qpdf", "QPDFFormFieldObjectHelper DA present", looked_in_acroform ? 0 : 1);
-        result = value.getUTF8Value();
+        return value.getUTF8Value();
     }
-    return result;
+    return {};
 }
 int
@@ -220,12 +218,11 @@ QPDFFormFieldObjectHelper::getQuadding()
         fv = getFieldFromAcroForm("/Q");
         looked_in_acroform = true;
     }
-    int result = 0;
     if (fv.isInteger()) {
         QTC::TC("qpdf", "QPDFFormFieldObjectHelper Q present", looked_in_acroform ? 0 : 1);
-        result = QIntC::to_int(fv.getIntValue());
+        return QIntC::to_int(fv.getIntValue());
     }
-    return result;
+    return 0;
 }
 int
@@ -238,46 +235,46 @@ QPDFFormFieldObjectHelper::getFlags()
 bool
 QPDFFormFieldObjectHelper::isText()
 {
-    return (getFieldType() == "/Tx");
+    return getFieldType() == "/Tx";
 }
 bool
 QPDFFormFieldObjectHelper::isCheckbox()
 {
-    return ((getFieldType() == "/Btn") && ((getFlags() & (ff_btn_radio | ff_btn_pushbutton)) == 0));
+    return getFieldType() == "/Btn" && (getFlags() & (ff_btn_radio | ff_btn_pushbutton)) == 0;
 }
 bool
 QPDFFormFieldObjectHelper::isChecked()
 {
-    return isCheckbox() && getValue().isName() && (getValue().getName() != "/Off");
+    return isCheckbox() && getValue().isName() && getValue().getName() != "/Off";
 }
 bool
 QPDFFormFieldObjectHelper::isRadioButton()
 {
-    return ((getFieldType() == "/Btn") && ((getFlags() & ff_btn_radio) == ff_btn_radio));
+    return getFieldType() == "/Btn" && (getFlags() & ff_btn_radio) == ff_btn_radio;
 }
 bool
 QPDFFormFieldObjectHelper::isPushbutton()
 {
-    return ((getFieldType() == "/Btn") && ((getFlags() & ff_btn_pushbutton) == ff_btn_pushbutton));
+    return getFieldType() == "/Btn" && (getFlags() & ff_btn_pushbutton) == ff_btn_pushbutton;
 }
 bool
 QPDFFormFieldObjectHelper::isChoice()
 {
-    return (getFieldType() == "/Ch");
+    return getFieldType() == "/Ch";
 }
 std::vector<std::string>
 QPDFFormFieldObjectHelper::getChoices()
 {
-    std::vector<std::string> result;
     if (!isChoice()) {
-        return result;
+        return {};
     }
+    std::vector<std::string> result;
     for (auto const& item: getInheritableFieldValue("/Opt").as_array()) {
         if (item.isString()) {
             result.emplace_back(item.getUTF8Value());
@@ -308,25 +305,26 @@ QPDFFormFieldObjectHelper::setV(QPDFObjectHandle value, bool need_appearances)
 {
     if (getFieldType() == "/Btn") {
         if (isCheckbox()) {
-            bool okay = false;
-            if (value.isName()) {
-                std::string name = value.getName();
-                okay = true;
-                // Accept any value other than /Off to mean checked. Files have been seen that use
-                // /1 or other values.
-                setCheckBoxValue((name != "/Off"));
-            }
-            if (!okay) {
+            if (!value.isName()) {
                 warn("ignoring attempt to set a checkbox field to a value whose type is not name");
+                return;
             }
-        } else if (isRadioButton()) {
-            if (value.isName()) {
-                setRadioButtonValue(value);
-            } else {
+            std::string name = value.getName();
+            // Accept any value other than /Off to mean checked. Files have been seen that use
+            // /1 or other values.
+            setCheckBoxValue(name != "/Off");
+            return;
+        }
+        if (isRadioButton()) {
+            if (!value.isName()) {
                 warn(
                     "ignoring attempt to set a radio button field to an object that is not a name");
+                return;
             }
-        } else if (isPushbutton()) {
+            setRadioButtonValue(value);
+            return;
+        }
+        if (isPushbutton()) {
             warn("ignoring attempt set the value of a pushbutton field");
         }
         return;
@@ -340,7 +338,7 @@ QPDFFormFieldObjectHelper::setV(QPDFObjectHandle value, bool need_appearances)
         QPDF& qpdf = oh().getQPDF(
             "QPDFFormFieldObjectHelper::setV called with need_appearances = "
             "true on an object that is not associated with an owning QPDF");
-        QPDFAcroFormDocumentHelper(qpdf).setNeedAppearances(true);
+        qpdf.acroform().setNeedAppearances(true);
     }
 }
@@ -1180,7 +1180,7 @@ void
 QPDFJob::doJSONAcroform(Pipeline* p, bool& first, QPDF& pdf)
 {
     JSON j_acroform = JSON::makeDictionary();
-    QPDFAcroFormDocumentHelper afdh(pdf);
+    auto& afdh = pdf.acroform();
     j_acroform.addDictionaryMember("hasacroform", JSON::makeBool(afdh.hasAcroForm()));
     j_acroform.addDictionaryMember("needappearances", JSON::makeBool(afdh.getNeedAppearances()));
     JSON j_fields = j_acroform.addDictionaryMember("fields", JSON::makeArray());
@@ -1888,17 +1888,6 @@ QPDFJob::validateUnderOverlay(QPDF&amp; pdf, UnderOverlay* uo)
     }
 }
-static QPDFAcroFormDocumentHelper*
-get_afdh_for_qpdf(
-    std::map<unsigned long long, std::shared_ptr<QPDFAcroFormDocumentHelper>>& afdh_map, QPDF* q)
-{
-    auto uid = q->getUniqueId();
-    if (!afdh_map.contains(uid)) {
-        afdh_map[uid] = std::make_shared<QPDFAcroFormDocumentHelper>(*q);
-    }
-    return afdh_map[uid].get();
-}
-
 std::string
 QPDFJob::doUnderOverlayForPage(
     QPDF& pdf,
@@ -1914,13 +1903,7 @@ QPDFJob::doUnderOverlayForPage(
     if (!(pagenos.contains(pageno) && pagenos[pageno].contains(uo_idx))) {
         return "";
     }
-
-    std::map<unsigned long long, std::shared_ptr<QPDFAcroFormDocumentHelper>> afdh;
-    auto make_afdh = [&](QPDFPageObjectHelper& ph) {
-        QPDF& q = ph.getObjectHandle().getQPDF();
-        return get_afdh_for_qpdf(afdh, &q);
-    };
-    auto dest_afdh = make_afdh(dest_page);
+    auto& dest_afdh = dest_page.qpdf()->acroform();
     std::string content;
     int min_suffix = 1;
@@ -1940,7 +1923,7 @@ QPDFJob::doUnderOverlayForPage(
         QPDFMatrix cm;
         std::string new_content = dest_page.placeFormXObject(
             fo[from_pageno][uo_idx], name, dest_page.getTrimBox().getArrayAsRectangle(), cm);
-        dest_page.copyAnnotations(from_page, cm, dest_afdh, make_afdh(from_page));
+        dest_page.copyAnnotations(from_page, cm, &dest_afdh, &from_page.qpdf()->acroform());
         if (!new_content.empty()) {
             resources.mergeResources("<< /XObject << >> >>"_qpdf);
             auto xobject = resources.getKey("/XObject");
@@ -2182,15 +2165,15 @@ void
 QPDFJob::handleTransformations(QPDF& pdf)
 {
     QPDFPageDocumentHelper dh(pdf);
-    std::shared_ptr<QPDFAcroFormDocumentHelper> afdh;
-    auto make_afdh = [&]() {
-        if (!afdh.get()) {
-            afdh = std::make_shared<QPDFAcroFormDocumentHelper>(pdf);
+    QPDFAcroFormDocumentHelper* afdh_ptr = nullptr;
+    auto afdh = [&]() -> QPDFAcroFormDocumentHelper& {
+        if (!afdh_ptr) {
+            afdh_ptr = &pdf.acroform();
         }
+        return *afdh_ptr;
     };
     if (m->remove_restrictions) {
-        make_afdh();
-        afdh->disableDigitalSignatures();
+        afdh().disableDigitalSignatures();
     }
     if (m->externalize_inline_images || (m->optimize_images && (!m->keep_inline_images))) {
         for (auto& ph: dh.getAllPages()) {
@@ -2225,8 +2208,7 @@ QPDFJob::handleTransformations(QPDF&amp; pdf)
         }
     }
     if (m->generate_appearances) {
-        make_afdh();
-        afdh->generateAppearancesIfNeeded();
+        afdh().generateAppearancesIfNeeded();
     }
     if (m->flatten_annotations) {
         dh.flattenAnnotations(m->flatten_annotations_required, m->flatten_annotations_forbidden);
@@ -2237,9 +2219,8 @@ QPDFJob::handleTransformations(QPDF&amp; pdf)
         }
     }
     if (m->flatten_rotation) {
-        make_afdh();
         for (auto& page: dh.getAllPages()) {
-            page.flattenRotation(afdh.get());
+            page.flattenRotation(&afdh());
         }
     }
     if (m->remove_page_labels) {
@@ -2559,8 +2540,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf, std::vector&lt;std::unique_ptr&lt;QPDF&gt;&gt;&amp; page_hea
     std::vector<QPDFObjectHandle> new_labels;
     bool any_page_labels = false;
     int out_pageno = 0;
-    std::map<unsigned long long, std::shared_ptr<QPDFAcroFormDocumentHelper>> afdh_map;
-    auto this_afdh = get_afdh_for_qpdf(afdh_map, &pdf);
+    auto& this_afdh = pdf.acroform();
     std::set<QPDFObjGen> referenced_fields;
     for (auto& page_data: parsed_specs) {
         ClosedFileInputSource* cis = nullptr;
@@ -2569,7 +2549,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf, std::vector&lt;std::unique_ptr&lt;QPDF&gt;&gt;&amp; page_hea
             cis->stayOpen(true);
         }
         QPDFPageLabelDocumentHelper pldh(*page_data.qpdf);
-        auto other_afdh = get_afdh_for_qpdf(afdh_map, page_data.qpdf);
+        auto& other_afdh = page_data.qpdf->acroform();
         if (pldh.hasPageLabels()) {
             any_page_labels = true;
         }
@@ -2611,15 +2591,15 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf, std::vector&lt;std::unique_ptr&lt;QPDF&gt;&gt;&amp; page_hea
             // the original file until all copy operations are completed, any foreign pages that
             // conflict with original pages will be adjusted. If we copy any page from the original
             // file more than once, that page would be in conflict with the previous copy of itself.
-            if ((!this_file && other_afdh->hasAcroForm()) || !first_copy_from_orig) {
+            if ((!this_file && other_afdh.hasAcroForm()) || !first_copy_from_orig) {
                 if (!this_file) {
                     QTC::TC("qpdf", "QPDFJob copy fields not this file");
                 } else if (!first_copy_from_orig) {
                     QTC::TC("qpdf", "QPDFJob copy fields non-first from orig");
                 }
                 try {
-                    this_afdh->fixCopiedAnnotations(
-                        new_page, to_copy.getObjectHandle(), *other_afdh, &referenced_fields);
+                    this_afdh.fixCopiedAnnotations(
+                        new_page, to_copy.getObjectHandle(), other_afdh, &referenced_fields);
                 } catch (std::exception& e) {
                     pdf.warn(
                         qpdf_e_damaged_pdf,
@@ -2647,7 +2627,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf, std::vector&lt;std::unique_ptr&lt;QPDF&gt;&gt;&amp; page_hea
     for (size_t pageno = 0; pageno < orig_pages.size(); ++pageno) {
         auto page = orig_pages.at(pageno);
         if (selected_from_orig.contains(QIntC::to_int(pageno))) {
-            for (auto field: this_afdh->getFormFieldsForPage(page)) {
+            for (auto field: this_afdh.getFormFieldsForPage(page)) {
                 QTC::TC("qpdf", "QPDFJob pages keeping field from original");
                 referenced_fields.insert(field.getObjectHandle().getObjGen());
             }
@@ -2656,7 +2636,7 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf, std::vector&lt;std::unique_ptr&lt;QPDF&gt;&gt;&amp; page_hea
         }
     }
     // Remove unreferenced form fields
-    if (this_afdh->hasAcroForm()) {
+    if (this_afdh.hasAcroForm()) {
         auto acroform = pdf.getRoot().getKey("/AcroForm");
         auto fields = acroform.getKey("/Fields");
         if (fields.isArray()) {
@@ -3013,7 +2993,7 @@ QPDFJob::doSplitPages(QPDF&amp; pdf)
         dh.removeUnreferencedResources();
     }
     QPDFPageLabelDocumentHelper pldh(pdf);
-    QPDFAcroFormDocumentHelper afdh(pdf);
+    auto& afdh = pdf.acroform();
     std::vector<QPDFObjectHandle> const& pages = pdf.getAllPages();
     size_t pageno_len = std::to_string(pages.size()).length();
     size_t num_pages = pages.size();
@@ -3025,10 +3005,7 @@ QPDFJob::doSplitPages(QPDF&amp; pdf)
         }
         QPDF outpdf;
         outpdf.emptyPDF();
-        std::shared_ptr<QPDFAcroFormDocumentHelper> out_afdh;
-        if (afdh.hasAcroForm()) {
-            out_afdh = std::make_shared<QPDFAcroFormDocumentHelper>(outpdf);
-        }
+        QPDFAcroFormDocumentHelper* out_afdh = afdh.hasAcroForm() ? &outpdf.acroform() : nullptr;
         if (m->suppress_warnings) {
             outpdf.setSuppressWarnings(true);
         }
@@ -3036,8 +3013,7 @@ QPDFJob::doSplitPages(QPDF&amp; pdf)
             QPDFObjectHandle page = pages.at(pageno - 1);
             outpdf.addPage(page, false);
             auto new_page = added_page(outpdf, page);
-            if (out_afdh.get()) {
-                QTC::TC("qpdf", "QPDFJob copy form fields in split_pages");
+            if (out_afdh) {
                 try {
                     out_afdh->fixCopiedAnnotations(new_page, page, afdh);
                 } catch (std::exception& e) {
 #include <qpdf/QPDFPageDocumentHelper.hh>
 #include <qpdf/QPDFAcroFormDocumentHelper.hh>
+#include <qpdf/QPDF_private.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
@@ -55,7 +56,7 @@ QPDFPageDocumentHelper::removePage(QPDFPageObjectHelper page)
 void
 QPDFPageDocumentHelper::flattenAnnotations(int required_flags, int forbidden_flags)
 {
-    QPDFAcroFormDocumentHelper afdh(qpdf);
+    auto& afdh = qpdf.acroform();
     if (afdh.getNeedAppearances()) {
         qpdf.getRoot()
             .getKey("/AcroForm")
@@ -3,6 +3,7 @@
 #include <qpdf/QPDF.hh>
+#include <qpdf/QPDFAcroFormDocumentHelper.hh>
 #include <qpdf/QPDFObject_private.hh>
 #include <qpdf/QPDFTokenizer_private.hh>
@@ -547,6 +548,9 @@ class QPDF::Members
     // Optimization data
     std::map<ObjUser, std::set<QPDFObjGen>> obj_user_to_objects;
     std::map<QPDFObjGen, std::set<ObjUser>> object_to_obj_users;
+
+    // Document Helpers;
+    std::unique_ptr<QPDFAcroFormDocumentHelper> acroform;
 };
 // JobSetter class is restricted to QPDFJob.
@@ -569,4 +573,13 @@ QPDF::reconstructed_xref() const
     return m->reconstructed_xref;
 }
+inline QPDFAcroFormDocumentHelper&
+QPDF::acroform()
+{
+    if (!m->acroform) {
+        m->acroform = std::make_unique<QPDFAcroFormDocumentHelper>(*this);
+    }
+    return *m->acroform;
+}
+
 #endif // QPDF_PRIVATE_HH
@@ -519,7 +519,6 @@ QPDFPageObjectHelper flatten inherit rotate 0
 QPDFAcroFormDocumentHelper copy annotation 3
 QPDFAcroFormDocumentHelper field with parent 3
 QPDFAcroFormDocumentHelper modify ap matrix 0
-QPDFJob copy form fields in split_pages 0
 QPDFJob pages keeping field from original 0
 QPDFObjectHandle merge reuse 0
 QPDFObjectHandle merge generate 0