Handle inheritable page attributes

Add getAttribute for handling inheritable page attributes, and fix getPageImages and annotation flattening code to use it.

Handle inheritable page attributes
Add getAttribute for handling inheritable page attributes, and fix getPageImages and annotation flattening code to use it.
Jay Berkenbilt
1 parent 2d32f4db
Showing 17 changed files with 2062 additions and 1797 deletions
ChangeLog
include/qpdf/QPDFPageObjectHelper.hh
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDFPageDocumentHelper.cc
libqpdf/QPDFPageObjectHelper.cc
libqpdf/QPDF_optimization.cc
qpdf/qpdf.cc
qpdf/qpdf.testcov
qpdf/qtest/qpdf.test
qpdf/qtest/qpdf/appearances-a-more.pdf
qpdf/qtest/qpdf/appearances-a-more2.pdf
qpdf/qtest/qpdf/appearances-a.pdf
qpdf/qtest/qpdf/form-filled-by-acrobat-out.pdf
qpdf/qtest/qpdf/need-appearances-more-out.pdf
qpdf/qtest/qpdf/need-appearances-out.pdf
qpdf/qtest/qpdf/sample-form-out.pdf
qpdf/qtest/qpdf/shared-images-show.out
+2019-01-25  Jay Berkenbilt  <ejb@ql.org>
+
+	* Add new method QPDFPageObjectHelper::getAttribute() that
+	properly handles inherited attributes and allows for creation of a
+	copy of shared attributes. This is very useful if you are getting
+	an attribute of a page dictionary with the intent to modify it
+	privately for that page.
+
+	* Fix QPDFPageObjectHelper::getPageImages (and the legacy
+	QPDFObjectHandle::getPageImages()) to properly handle images in
+	inherited resources dictionaries.
+
 2019-01-20  Jay Berkenbilt  <ejb@ql.org>
 	* Tweak the content code generated for variable text fields to
@@ -39,13 +39,22 @@ class QPDFPageObjectHelper: public QPDFObjectHelper
     {
     }
+    // Return the effective value of this attribute for the page. If
+    // the requested attribute is not present on the page but is
+    // inheritable, look up through the page's ancestors in the page
+    // tree. If copy_if_shared is true, then this method will replace
+    // the attribute with a shallow copy if it is in indirect or
+    // inherited and return the copy. You should do this if you are
+    // going to modify the returned object and want the modifications
+    // to apply to the current page only.
+    QPDF_DLL
+    QPDFObjectHandle
+    getAttribute(std::string const& name, bool copy_if_shared);
+
     // Returns an empty map if there are no images or no resources.
-    // This function does not presently support inherited resources.
-    // If this is a significant concern, call
-    // pushInheritedAttributesToPage() on the QPDF object that owns
-    // this page. See comment in the source for details. Return value
-    // is a map from XObject name to the image object, which is always
-    // a stream.
+    // Prior to qpdf 8.4.0, this function did not support inherited
+    // resources, but it does now. Return value is a map from XObject
+    // name to the image object, which is always a stream.
     QPDF_DLL
     std::map<std::string, QPDFObjectHandle> getPageImages();
@@ -18,6 +18,7 @@
 #include <qpdf/Pl_QPDFTokenizer.hh>
 #include <qpdf/BufferInputSource.hh>
 #include <qpdf/QPDFExc.hh>
+#include <qpdf/QPDFPageObjectHelper.hh>
 #include <qpdf/QTC.hh>
 #include <qpdf/QUtil.hh>
@@ -1109,24 +1110,11 @@ QPDFObjectHandle::getGeneration() const
 std::map<std::string, QPDFObjectHandle>
 QPDFObjectHandle::getPageImages()
 {
-    // Note: this code doesn't handle inherited resources.  If this
-    // page dictionary doesn't have a /Resources key or has one whose
-    // value is null or an empty dictionary, you are supposed to walk
-    // up the page tree until you find a /Resources dictionary.  As of
-    // this writing, I don't have any test files that use inherited
-    // resources, and hand-generating one won't be a good test because
-    // any mistakes in my understanding would be present in both the
-    // code and the test file.
-
-    // NOTE: If support of inherited resources (see above comment) is
-    // implemented, edit comment in QPDFObjectHandle.hh for this
-    // function.  Also remove call to pushInheritedAttributesToPage
-    // from qpdf.cc when show_page_images is true.
-
     std::map<std::string, QPDFObjectHandle> result;
-    if (this->hasKey("/Resources"))
+    QPDFObjectHandle resources =
+        QPDFPageObjectHelper(*this).getAttribute("/Resources", false);
+    if (resources.isDictionary())
     {
-	QPDFObjectHandle resources = this->getKey("/Resources");
 	if (resources.hasKey("/XObject"))
 	{
 	    QPDFObjectHandle xobject = resources.getKey("/XObject");
@@ -79,20 +79,12 @@ QPDFPageDocumentHelper::flattenAnnotations(
             "document does not have updated appearance streams,"
             " so form fields will not be flattened");
     }
-    pushInheritedAttributesToPage();
     std::vector<QPDFPageObjectHelper> pages = getAllPages();
     for (std::vector<QPDFPageObjectHelper>::iterator iter = pages.begin();
          iter != pages.end(); ++iter)
     {
         QPDFPageObjectHelper ph(*iter);
-        QPDFObjectHandle page_oh = ph.getObjectHandle();
-        if (page_oh.getKey("/Resources").isIndirect())
-        {
-            QTC::TC("qpdf", "QPDFPageDocumentHelper indirect resources");
-            page_oh.replaceKey("/Resources",
-                               page_oh.getKey("/Resources").shallowCopy());
-        }
-        QPDFObjectHandle resources = ph.getObjectHandle().getKey("/Resources");
+        QPDFObjectHandle resources = ph.getAttribute("/Resources", true);
         if (! resources.isDictionary())
         {
             // This should never happen and is not exercised in the
@@ -15,6 +15,42 @@ QPDFPageObjectHelper::QPDFPageObjectHelper(QPDFObjectHandle oh) :
 {
 }
+QPDFObjectHandle
+QPDFPageObjectHelper::getAttribute(std::string const& name,
+                                   bool copy_if_shared)
+{
+    bool inheritable = ((name == "/MediaBox") || (name == "/CropBox") ||
+                        (name == "/Resources") || (name == "/Rotate"));
+
+    QPDFObjectHandle node = this->oh;
+    QPDFObjectHandle result(node.getKey(name));
+    std::set<QPDFObjGen> seen;
+    bool inherited = false;
+    while (inheritable && result.isNull() && node.hasKey("/Parent"))
+    {
+        seen.insert(node.getObjGen());
+        node = node.getKey("/Parent");
+        if (seen.count(node.getObjGen()))
+        {
+            break;
+        }
+        result = node.getKey(name);
+        if (! result.isNull())
+        {
+            QTC::TC("qpdf", "QPDFPageObjectHelper non-trivial inheritance");
+            inherited = true;
+        }
+    }
+    if (copy_if_shared && (inherited || result.isIndirect()))
+    {
+        QTC::TC("qpdf", "QPDFPageObjectHelper copy shared attribute");
+        result = result.shallowCopy();
+        this->oh.replaceKey(name, result);
+    }
+    return result;
+}
+
+
 std::map<std::string, QPDFObjectHandle>
 QPDFPageObjectHelper::getPageImages()
 {
@@ -159,12 +195,7 @@ QPDFPageObjectHelper::removeUnreferencedResources()
     std::vector<std::string> to_filter;
     to_filter.push_back("/Font");
     to_filter.push_back("/XObject");
-    QPDFObjectHandle resources = this->oh.getKey("/Resources");
-    if (resources.isDictionary())
-    {
-        resources = resources.shallowCopy();
-        this->oh.replaceKey("/Resources", resources);
-    }
+    QPDFObjectHandle resources = getAttribute("/Resources", true);
     for (std::vector<std::string>::iterator d_iter = to_filter.begin();
          d_iter != to_filter.end(); ++d_iter)
     {
@@ -208,10 +208,10 @@ QPDF::pushInheritedAttributesToPageInternal2(
     if (type == "/Pages")
     {
-	// Make a list of inheritable keys.  Any key other than /Type,
-	// /Parent, Kids, or /Count is an inheritable attribute.  Push
-	// this object onto the stack of pages nodes that have values
-	// for this attribute.
+	// Make a list of inheritable keys. Only the keys /MediaBox,
+	// /CropBox, /Resources, and /Rotate are inheritable
+	// attributes. Push this object onto the stack of pages nodes
+	// that have values for this attribute.
 	std::set<std::string> inheritable_keys;
 	std::set<std::string> keys = cur_pages.getKeys();
@@ -3070,10 +3070,6 @@ static void do_show_obj(QPDF&amp; pdf, Options&amp; o, int&amp; exit_code)
 static void do_show_pages(QPDF& pdf, Options& o)
 {
     QPDFPageDocumentHelper dh(pdf);
-    if (o.show_page_images)
-    {
-        dh.pushInheritedAttributesToPage();
-    }
     std::vector<QPDFPageObjectHelper> pages = dh.getAllPages();
     int pageno = 0;
     for (std::vector<QPDFPageObjectHelper>::iterator iter = pages.begin();
@@ -3862,7 +3858,6 @@ static void handle_transformations(QPDF&amp; pdf, Options&amp; o)
     QPDFPageDocumentHelper dh(pdf);
     if (o.optimize_images)
     {
-        dh.pushInheritedAttributesToPage();
         int pageno = 0;
         std::vector<QPDFPageObjectHelper> pages = dh.getAllPages();
         for (std::vector<QPDFPageObjectHelper>::iterator iter = pages.begin();
@@ -3891,8 +3886,9 @@ static void handle_transformations(QPDF&amp; pdf, Options&amp; o)
                         sdp,
                         QPDFObjectHandle::newName("/DCTDecode"),
                         QPDFObjectHandle::newNull());
-                    page.getKey("/Resources").getKey("/XObject").replaceKey(
-                        name, new_image);
+                    ph.getAttribute("/Resources", true).
+                        getKey("/XObject").replaceKey(
+                            name, new_image);
                 }
             }
         }
@@ -4054,7 +4050,6 @@ static void handle_page_specs(QPDF&amp; pdf, Options&amp; o)
                 cis->stayOpen(true);
             }
             QPDFPageDocumentHelper dh(*((*iter).second));
-            dh.pushInheritedAttributesToPage();
             dh.removeUnreferencedResources();
             if (cis)
             {
@@ -4532,7 +4527,6 @@ static void write_outfile(QPDF&amp; pdf, Options&amp; o)
         if (! o.preserve_unreferenced_page_resources)
         {
             QPDFPageDocumentHelper dh(pdf);
-            dh.pushInheritedAttributesToPage();
             dh.removeUnreferencedResources();
         }
         QPDFPageLabelDocumentHelper pldh(pdf);
@@ -378,7 +378,6 @@ QPDFAnnotationObjectHelper default matrix 0
 QPDFAnnotationObjectHelper rotate 90 0
 QPDFAnnotationObjectHelper rotate 180 0
 QPDFAnnotationObjectHelper rotate 270 0
-QPDFPageDocumentHelper indirect resources 0
 QPDFPageDocumentHelper skip widget need appearances 0
 QPDFPageDocumentHelper merge DR 0
 QPDFPageDocumentHelper non-widget annotation 0
@@ -426,3 +425,5 @@ QPDFFormFieldObjectHelper create AS from scratch 0
 QPDFFormFieldObjectHelper create AP from scratch 0
 QPDFFormFieldObjectHelper replaced BMC at EOF 0
 QPDFFormFieldObjectHelper fallback Tf 0
+QPDFPageObjectHelper non-trivial inheritance 0
+QPDFPageObjectHelper copy shared attribute 0
@@ -2409,7 +2409,7 @@ $td-&gt;runtest(&quot;stream detected&quot;,
 show_ntests();
 # ----------
 $td->notify("--- Extraction Tests ---");
-$n_tests += 12;
+$n_tests += 13;
 $td->runtest("show xref",
 	     {$td->COMMAND => "qpdf encrypted-with-images.pdf" .
@@ -2432,6 +2432,13 @@ $td-&gt;runtest(&quot;show-pages-images&quot;,
 	      $td->EXIT_STATUS => 0},
 	     $td->NORMALIZE_NEWLINES);
+$td->runtest("show-pages-images",
+	     {$td->COMMAND => "qpdf shared-images.pdf" .
+		  " --show-pages --with-images"},
+	     {$td->FILE => "shared-images-show.out",
+	      $td->EXIT_STATUS => 0},
+	     $td->NORMALIZE_NEWLINES);
+
 $td->runtest("show-page-1",
 	     {$td->COMMAND => "qpdf encrypted-with-images.pdf" .
 		  " --show-object=5,0"},
+page 1: 3 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    13 0 R
+page 2: 4 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    19 0 R
+page 3: 5 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    23 0 R
+page 4: 6 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    27 0 R
+page 5: 7 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    31 0 R
+page 6: 8 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    35 0 R
+page 7: 9 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    39 0 R
+page 8: 10 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    43 0 R
+page 9: 11 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    47 0 R
+page 10: 12 0 R
+  images:
+    /Im1: 17 0 R, 50 x 50
+    /Im10: 53 0 R, 50 x 50
+    /Im2: 21 0 R, 50 x 50
+    /Im3: 25 0 R, 50 x 50
+    /Im4: 29 0 R, 50 x 50
+    /Im5: 33 0 R, 50 x 50
+    /Im6: 37 0 R, 50 x 50
+    /Im7: 41 0 R, 50 x 50
+    /Im8: 45 0 R, 50 x 50
+    /Im9: 49 0 R, 50 x 50
+  content:
+    51 0 R