diff --git a/ChangeLog b/ChangeLog index 24bbc569..91301ae9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2021-11-04 Jay Berkenbilt + + * Add an extra check to the library to detect when foreign objects + are inserted directly (instead of using + QPDF::copyForeignObject) at the time of + insertion rather than when the file is written. Catching the error + sooner makes it much easier to locate the incorrect code. + 2021-11-03 Jay Berkenbilt * Bug fix: make overlay/underlay work on a page with no resource diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index e57179b5..2f2b3ebc 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -1336,6 +1336,7 @@ class QPDFObjectHandle std::vector arrayOrStreamToStreamArray( std::string const& description, std::string& all_description); static void warn(QPDF*, QPDFExc const&); + void checkOwnership(QPDFObjectHandle const&) const; bool initialized; diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 27740f09..af862bd4 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -864,6 +864,7 @@ QPDFObjectHandle::setArrayItem(int n, QPDFObjectHandle const& item) { if (isArray()) { + checkOwnership(item); dynamic_cast(obj.getPointer())->setItem(n, item); } else @@ -878,6 +879,10 @@ QPDFObjectHandle::setArrayFromVector(std::vector const& items) { if (isArray()) { + for (auto const& item: items) + { + checkOwnership(item); + } dynamic_cast(obj.getPointer())->setFromVector(items); } else @@ -906,6 +911,7 @@ QPDFObjectHandle::appendItem(QPDFObjectHandle const& item) { if (isArray()) { + checkOwnership(item); dynamic_cast(obj.getPointer())->appendItem(item); } else @@ -1283,6 +1289,7 @@ QPDFObjectHandle::replaceKey(std::string const& key, { if (isDictionary()) { + checkOwnership(value); dynamic_cast( obj.getPointer())->replaceKey(key, value); } @@ -1313,6 +1320,7 @@ QPDFObjectHandle::replaceOrRemoveKey(std::string const& key, { if (isDictionary()) { + checkOwnership(value); dynamic_cast( obj.getPointer())->replaceOrRemoveKey(key, value); } @@ -3269,6 +3277,20 @@ QPDFObjectHandle::isImage(bool exclude_imagemask) dict.getKey("/ImageMask").getBoolValue())))); } +void +QPDFObjectHandle::checkOwnership(QPDFObjectHandle const& item) const +{ + if ((this->qpdf != nullptr) && + (item.qpdf != nullptr) && + (this->qpdf != item.qpdf)) + { + QTC::TC("qpdf", "QPDFObjectHandle check ownership"); + throw std::logic_error( + "Attempting to add an object from a different QPDF." + " Use QPDF::copyForeignObject to add objects from another file."); + } +} + void QPDFObjectHandle::assertPageObject() { diff --git a/manual/qpdf-manual.xml b/manual/qpdf-manual.xml index 7cd22459..026d6039 100644 --- a/manual/qpdf-manual.xml +++ b/manual/qpdf-manual.xml @@ -5104,6 +5104,16 @@ print "\n"; receive warnings on certain recoverable conditions. + + + Add an extra check to the library to detect when foreign + objects are inserted directly (instead of using + QPDF::copyForeignObject) at the time of + insertion rather than when the file is written. Catching the + error sooner makes it much easier to locate the incorrect + code. + + diff --git a/qpdf/qpdf.cc b/qpdf/qpdf.cc index e1cd62da..9438886e 100644 --- a/qpdf/qpdf.cc +++ b/qpdf/qpdf.cc @@ -5705,7 +5705,9 @@ static QPDFObjectHandle added_page(QPDF& pdf, QPDFPageObjectHelper page) return added_page(pdf, page.getObjectHandle()); } -static void handle_page_specs(QPDF& pdf, Options& o, bool& warnings) +static void handle_page_specs( + QPDF& pdf, Options& o, bool& warnings, + std::vector>& page_heap) { // Parse all page specifications and translate them into lists of // actual pages. @@ -5757,7 +5759,6 @@ static void handle_page_specs(QPDF& pdf, Options& o, bool& warnings) } // Create a QPDF object for each file that we may take pages from. - std::vector > page_heap; std::map page_spec_qpdfs; std::map page_spec_cfis; page_spec_qpdfs[o.infilename] = &pdf; @@ -6009,7 +6010,7 @@ static void handle_page_specs(QPDF& pdf, Options& o, bool& warnings) pdf.warn( QPDFExc(qpdf_e_damaged_pdf, pdf.getFilename(), "", 0, "Exception caught while fixing copied" - " annotations. This may be a qpdf bug." + + " annotations. This may be a qpdf bug. " + std::string("Exception: ") + e.what())); } } @@ -6649,9 +6650,10 @@ int realmain(int argc, char* argv[]) } } bool other_warnings = false; + std::vector> page_heap; if (! o.page_specs.empty()) { - handle_page_specs(pdf, o, other_warnings); + handle_page_specs(pdf, o, other_warnings, page_heap); } if (! o.rotations.empty()) { diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 68a32d43..8f888209 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -597,3 +597,4 @@ QPDFWriter exclude from object stream 0 check unclosed --pages 1 QPDF_pages findPage not found 0 qpdf overlay page with no resources 0 +QPDFObjectHandle check ownership 0 diff --git a/qpdf/qtest/qpdf/foreign-in-write.out b/qpdf/qtest/qpdf/foreign-in-write.out index 14293aa9..a829577d 100644 --- a/qpdf/qtest/qpdf/foreign-in-write.out +++ b/qpdf/qtest/qpdf/foreign-in-write.out @@ -1,2 +1,3 @@ logic error: QPDFObjectHandle from different QPDF found while writing. Use QPDF::copyForeignObject to add objects from another file. +logic error: Attempting to add an object from a different QPDF. Use QPDF::copyForeignObject to add objects from another file. test 29 done diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 9a21b82c..1d995bb7 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -1274,6 +1274,18 @@ void runtest(int n, char const* filename1, char const* arg2) { std::cout << "logic error: " << e.what() << std::endl; } + + // Detect adding a foreign object + auto root1 = pdf.getRoot(); + auto root2 = other.getRoot(); + try + { + root1.replaceKey("/Oops", root2); + } + catch (std::logic_error const& e) + { + std::cout << "logic error: " << e.what() << std::endl; + } } else if (n == 30) {