Merge pull request #1028 from m-holger/i1003

Maintain links to foreign pages when copying foreign objects (fixes #1003)

Merge pull request #1028 from m-holger/i1003
Maintain links to foreign pages when copying foreign objects (fixes #1003)
Jay Berkenbilt · GitHub
2 parents fc656816 ac32dbc3
Showing 5 changed files with 382 additions and 363 deletions
include/qpdf/QPDF.hh
libqpdf/QPDF.cc
libqpdf/QPDF_Dictionary.cc
qpdf/qtest/qpdf/test80b1.pdf
qpdf/qtest/qpdf/test80b2.pdf
@@ -351,6 +351,8 @@ class QPDF
     // QPDF with QPDFWriter if it has any reserved objects in it.
     QPDF_DLL
     QPDFObjectHandle newReserved();
+    QPDF_DLL
+    QPDFObjectHandle newIndirectNull();
  
     // Install this object handle as an indirect object and return an indirect reference to it.
     QPDF_DLL
@@ -391,8 +393,8 @@ class QPDF
     void swapObjects(int objid1, int generation1, int objid2, int generation2);
  
     // Replace a reserved object.  This is a wrapper around replaceObject but it guarantees that the
-    // underlying object is a reserved object.  After this call, reserved will be a reference to
-    // replacement.
+    // underlying object is a reserved object or a null object.  After this call, reserved will
+    // be a reference to replacement.
     QPDF_DLL
     void replaceReserved(QPDFObjectHandle reserved, QPDFObjectHandle replacement);
  
@@ -1861,6 +1861,12 @@ QPDF::newReserved()
 }
  
 QPDFObjectHandle
+QPDF::newIndirectNull()
+{
+    return makeIndirectFromQPDFObject(QPDF_Null::create());
+}
+
+QPDFObjectHandle
 QPDF::newStream()
 {
     return makeIndirectFromQPDFObject(
@@ -1948,7 +1954,10 @@ void
 QPDF::replaceReserved(QPDFObjectHandle reserved, QPDFObjectHandle replacement)
 {
     QTC::TC("qpdf", "QPDF replaceReserved");
-    reserved.assertReserved();
+    auto tc = reserved.getTypeCode();
+    if (!(tc == ::ot_reserved || tc == ::ot_null)) {
+        throw std::logic_error("replaceReserved called with non-reserverd object");
+    }
     replaceObject(reserved.getObjGen(), replacement);
 }
  
@@ -2012,8 +2021,7 @@ QPDF::copyForeignObject(QPDFObjectHandle foreign)
     reserveObjects(foreign, obj_copier, true);
  
     if (!obj_copier.visiting.empty()) {
-        throw std::logic_error("obj_copier.visiting is not empty"
-                               " after reserving objects");
+        throw std::logic_error("obj_copier.visiting is not empty after reserving objects");
     }
  
     // Copy any new objects and replace the reservations.
@@ -2026,7 +2034,13 @@ QPDF::copyForeignObject(QPDFObjectHandle foreign)
     }
     obj_copier.to_copy.clear();
  
-    return obj_copier.object_map[foreign.getObjGen()];
+    auto& result = obj_copier.object_map[foreign.getObjGen()];
+    if (!result.isInitialized()) {
+        result = QPDFObjectHandle::newNull();
+        warn(damagedPDF("Unexpected reference to /Pages object while copying foreign object. "
+                        "Replacing with Null object."));
+    }
+    return result;
 }
  
 void
@@ -2042,11 +2056,6 @@ QPDF::reserveObjects(QPDFObjectHandle foreign, ObjCopier&amp; obj_copier, bool top)
         return;
     }
  
-    if ((!top) && foreign.isPageObject()) {
-        QTC::TC("qpdf", "QPDF not crossing page boundary");
-        return;
-    }
-
     if (foreign.isIndirect()) {
         QPDFObjGen foreign_og(foreign.getObjGen());
         if (obj_copier.object_map.count(foreign_og) > 0) {
@@ -2061,8 +2070,14 @@ QPDF::reserveObjects(QPDFObjectHandle foreign, ObjCopier&amp; obj_copier, bool top)
         }
         QTC::TC("qpdf", "QPDF copy indirect");
         if (obj_copier.object_map.count(foreign_og) == 0) {
+            obj_copier.object_map[foreign_og] =
+                foreign.isStream() ? newStream() : newIndirectNull();
+            if ((!top) && foreign.isPageObject()) {
+                QTC::TC("qpdf", "QPDF not crossing page boundary");
+                obj_copier.visiting.erase(foreign);
+                return;
+            }
             obj_copier.to_copy.push_back(foreign);
-            obj_copier.object_map[foreign_og] = foreign.isStream() ? newStream() : newReserved();
         }
     }
  
@@ -2094,7 +2109,7 @@ QPDF::replaceForeignIndirectObjects(QPDFObjectHandle foreign, ObjCopier&amp; obj_cop
         QTC::TC("qpdf", "QPDF replace indirect");
         auto mapping = obj_copier.object_map.find(foreign.getObjGen());
         if (mapping == obj_copier.object_map.end()) {
-            // This case would occur if this is a reference to a Page or Pages object that we didn't
+            // This case would occur if this is a reference to a Pages object that we didn't
             // traverse into.
             QTC::TC("qpdf", "QPDF replace foreign indirect with null");
             result = QPDFObjectHandle::newNull();
@@ -121,8 +121,10 @@ QPDF_Dictionary::getAsMap() const
 void
 QPDF_Dictionary::replaceKey(std::string const& key, QPDFObjectHandle value)
 {
-    if (value.isNull()) {
-        // The PDF spec doesn't distinguish between keys with null values and missing keys.
+    if (value.isNull() && !value.isIndirect()) {
+        // The PDF spec doesn't distinguish between keys with null values and missing keys. Allow
+        // indirect nulls which are equivalent to a dangling reference, which is permitted by the
+        // spec.
         removeKey(key);
     } else {
         // add or replace value