From ddd78c1b7f53f09710431d58cd94659271f325cc Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Thu, 21 Jun 2018 20:33:38 -0400 Subject: [PATCH] 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. --- include/qpdf/QPDFObjectHandle.hh | 11 +++---- libqpdf/QPDFObjectHandle.cc | 31 +++++++++++++------- qpdf/qtest/qpdf/good10.qdf | 50 ++++++++++++++++++-------------- 3 files changed, 54 insertions(+), 38 deletions(-) diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 868b5c07..6e45b5fd 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -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& visited); + void copyObject(std::set& visited, bool cross_indirect); void releaseResolved(); static void setObjectDescriptionFromInput( QPDFObjectHandle, QPDF*, std::string const&, diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index da609cc2..d1bc58ca 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -2025,11 +2025,14 @@ QPDFObjectHandle::shallowCopy() new_obj = *this; } + std::set visited; + new_obj.copyObject(visited, false); return new_obj; } void -QPDFObjectHandle::makeDirectInternal(std::set& visited) +QPDFObjectHandle::copyObject(std::set& visited, + bool cross_indirect) { assertInitialized(); @@ -2040,17 +2043,17 @@ QPDFObjectHandle::makeDirectInternal(std::set& 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& 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& 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& 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 visited; - makeDirectInternal(visited); + std::set visited; + copyObject(visited, true); } void diff --git a/qpdf/qtest/qpdf/good10.qdf b/qpdf/qtest/qpdf/good10.qdf index 957168c8..77b7d377 100644 --- a/qpdf/qtest/qpdf/good10.qdf +++ b/qpdf/qtest/qpdf/good10.qdf @@ -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