Fix QPDFObjectHandle::shallowCopy

It's not really a shallow copy. It just doesn't cross indirect object boundaries. The old implementation had a bug that would cause multiple shallow copies of the same object to share memory, which was not the intention.

Fix QPDFObjectHandle::shallowCopy
It's not really a shallow copy. It just doesn't cross indirect object boundaries. The old implementation had a bug that would cause multiple shallow copies of the same object to share memory, which was not the intention.
Jay Berkenbilt
1 parent 84cd53f5
Showing 3 changed files with 54 additions and 38 deletions
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDFObjectHandle.cc
qpdf/qtest/qpdf/good10.qdf
@@ -514,10 +514,11 @@ class QPDFObjectHandle
     QPDF_DLL
     QPDF* getOwningQPDF();
  
-    // Create a shallow copy of an object as a direct object.  Since
-    // this is a shallow copy, for dictionaries and arrays, any keys
-    // or items that were indirect objects will still be indirect
-    // objects that point to the same place.
+    // Create a shallow of an object as a direct object, but do not
+    // traverse across indirect object boundaries. That means that,
+    // for dictionaries and arrays, any keys or items that were
+    // indirect objects will still be indirect objects that point to
+    // the same place.
     QPDF_DLL
     QPDFObjectHandle shallowCopy();
  
@@ -880,7 +881,7 @@ class QPDFObjectHandle
     void objectWarning(std::string const& warning);
     void assertType(char const* type_name, bool istype);
     void dereference();
-    void makeDirectInternal(std::set<int>& visited);
+    void copyObject(std::set<QPDFObjGen>& visited, bool cross_indirect);
     void releaseResolved();
     static void setObjectDescriptionFromInput(
         QPDFObjectHandle, QPDF*, std::string const&,
@@ -2025,11 +2025,14 @@ QPDFObjectHandle::shallowCopy()
         new_obj = *this;
     }
  
+    std::set<QPDFObjGen> visited;
+    new_obj.copyObject(visited, false);
     return new_obj;
 }
  
 void
-QPDFObjectHandle::makeDirectInternal(std::set<int>& visited)
+QPDFObjectHandle::copyObject(std::set<QPDFObjGen>& visited,
+                             bool cross_indirect)
 {
     assertInitialized();
  
@@ -2040,17 +2043,17 @@ QPDFObjectHandle::makeDirectInternal(std::set&lt;int&gt;&amp; visited)
 	    "attempt to make a stream into a direct object");
     }
  
-    int cur_objid = this->m->objid;
-    if (cur_objid != 0)
+    QPDFObjGen cur_og(this->m->objid, this->m->generation);
+    if (cur_og.getObj() != 0)
     {
-	if (visited.count(cur_objid))
+	if (visited.count(cur_og))
 	{
 	    QTC::TC("qpdf", "QPDFObjectHandle makeDirect loop");
 	    throw std::runtime_error(
 		"loop detected while converting object from "
 		"indirect to direct");
 	}
-	visited.insert(cur_objid);
+	visited.insert(cur_og);
     }
  
     if (isReserved())
@@ -2105,7 +2108,10 @@ QPDFObjectHandle::makeDirectInternal(std::set&lt;int&gt;&amp; visited)
 	for (int i = 0; i < n; ++i)
 	{
 	    items.push_back(getArrayItem(i));
-	    items.back().makeDirectInternal(visited);
+            if (cross_indirect || (! items.back().isIndirect()))
+            {
+                items.back().copyObject(visited, cross_indirect);
+            }
 	}
 	new_obj = new QPDF_Array(items);
     }
@@ -2118,7 +2124,10 @@ QPDFObjectHandle::makeDirectInternal(std::set&lt;int&gt;&amp; visited)
 	     iter != keys.end(); ++iter)
 	{
 	    items[*iter] = getKey(*iter);
-	    items[*iter].makeDirectInternal(visited);
+            if (cross_indirect || (! items[*iter].isIndirect()))
+            {
+                items[*iter].copyObject(visited, cross_indirect);
+            }
 	}
 	new_obj = new QPDF_Dictionary(items);
     }
@@ -2130,17 +2139,17 @@ QPDFObjectHandle::makeDirectInternal(std::set&lt;int&gt;&amp; visited)
  
     this->m->obj = new_obj;
  
-    if (cur_objid)
+    if (cur_og.getObj())
     {
-	visited.erase(cur_objid);
+	visited.erase(cur_og);
     }
 }
  
 void
 QPDFObjectHandle::makeDirect()
 {
-    std::set<int> visited;
-    makeDirectInternal(visited);
+    std::set<QPDFObjGen> visited;
+    copyObject(visited, true);
 }
  
 void
@@ -5,17 +5,22 @@
 %% Original object ID: 1 0
 1 0 obj
 <<
-  /Pages 2 0 R
+  /Pages 3 0 R
   /Type /Catalog
 >>
 endobj
  
-%% Original object ID: 2 0
+%% Original object ID: 8 0
 2 0 obj
+null
+endobj
+
+%% Original object ID: 2 0
+3 0 obj
 <<
   /Count 1
   /Kids [
-    3 0 R
+    4 0 R
   ]
   /Type /Pages
 >>
@@ -23,21 +28,21 @@ endobj
  
 %% Page 1
 %% Original object ID: 3 0
-3 0 obj
+4 0 obj
 <<
-  /Contents 4 0 R
+  /Contents 5 0 R
   /MediaBox [
     0
     0
     612
     792
   ]
-  /Parent 2 0 R
+  /Parent 3 0 R
   /Resources <<
     /Font <<
-      /F1 6 0 R
+      /F1 7 0 R
     >>
-    /ProcSet 7 0 R
+    /ProcSet 8 0 R
   >>
   /Type /Page
 >>
@@ -45,9 +50,9 @@ endobj
  
 %% Contents for page 1
 %% Original object ID: 4 0
-4 0 obj
+5 0 obj
 <<
-  /Length 5 0 R
+  /Length 6 0 R
 >>
 stream
 BT
@@ -58,12 +63,12 @@ ET
 endstream
 endobj
  
-5 0 obj
+6 0 obj
 44
 endobj
  
 %% Original object ID: 6 0
-6 0 obj
+7 0 obj
 <<
   /BaseFont /Helvetica
   /Encoding /WinAnsiEncoding
@@ -74,7 +79,7 @@ endobj
 endobj
  
 %% Original object ID: 5 0
-7 0 obj
+8 0 obj
 [
   /PDF
   /Text
@@ -82,29 +87,30 @@ endobj
 endobj
  
 xref
-0 8
+0 9
 0000000000 65535 f 
 0000000052 00000 n 
 0000000133 00000 n 
-0000000242 00000 n 
-0000000484 00000 n 
-0000000583 00000 n 
-0000000629 00000 n 
-0000000774 00000 n 
+0000000181 00000 n 
+0000000290 00000 n 
+0000000532 00000 n 
+0000000631 00000 n 
+0000000677 00000 n 
+0000000822 00000 n 
 trailer <<
   /QTest [
     1
     (2)
-    null
+    2 0 R
     0.0
     -0.0
     0.
     -0.
   ]
   /Root 1 0 R
-  /Size 8
+  /Size 9
   /ID [<31415926535897932384626433832795><31415926535897932384626433832795>]
 >>
 startxref
-809
+857
 %%EOF