summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--ChangeLog8
-rw-r--r--include/qpdf/QPDFObjectHandle.hh1
-rw-r--r--libqpdf/QPDFObjectHandle.cc22
-rw-r--r--manual/qpdf-manual.xml10
-rw-r--r--qpdf/qpdf.cc10
-rw-r--r--qpdf/qpdf.testcov1
-rw-r--r--qpdf/qtest/qpdf/foreign-in-write.out1
-rw-r--r--qpdf/test_driver.cc12
8 files changed, 61 insertions, 4 deletions
diff --git a/ChangeLog b/ChangeLog
index 24bbc569..91301ae9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2021-11-04 Jay Berkenbilt <ejb@ql.org>
+
+ * Add an extra check to the library to detect when foreign objects
+ are inserted directly (instead of using
+ <function>QPDF::copyForeignObject</function>) 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 <ejb@ql.org>
* 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<QPDFObjectHandle> 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<QPDF_Array*>(obj.getPointer())->setItem(n, item);
}
else
@@ -878,6 +879,10 @@ QPDFObjectHandle::setArrayFromVector(std::vector<QPDFObjectHandle> const& items)
{
if (isArray())
{
+ for (auto const& item: items)
+ {
+ checkOwnership(item);
+ }
dynamic_cast<QPDF_Array*>(obj.getPointer())->setFromVector(items);
}
else
@@ -906,6 +911,7 @@ QPDFObjectHandle::appendItem(QPDFObjectHandle const& item)
{
if (isArray())
{
+ checkOwnership(item);
dynamic_cast<QPDF_Array*>(obj.getPointer())->appendItem(item);
}
else
@@ -1283,6 +1289,7 @@ QPDFObjectHandle::replaceKey(std::string const& key,
{
if (isDictionary())
{
+ checkOwnership(value);
dynamic_cast<QPDF_Dictionary*>(
obj.getPointer())->replaceKey(key, value);
}
@@ -1313,6 +1320,7 @@ QPDFObjectHandle::replaceOrRemoveKey(std::string const& key,
{
if (isDictionary())
{
+ checkOwnership(value);
dynamic_cast<QPDF_Dictionary*>(
obj.getPointer())->replaceOrRemoveKey(key, value);
}
@@ -3270,6 +3278,20 @@ QPDFObjectHandle::isImage(bool exclude_imagemask)
}
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()
{
if (! isPageObject())
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.
</para>
</listitem>
+ <listitem>
+ <para>
+ Add an extra check to the library to detect when foreign
+ objects are inserted directly (instead of using
+ <function>QPDF::copyForeignObject</function>) 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.
+ </para>
+ </listitem>
</itemizedlist>
</listitem>
<listitem>
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<PointerHolder<QPDF>>& 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<PointerHolder<QPDF> > page_heap;
std::map<std::string, QPDF*> page_spec_qpdfs;
std::map<std::string, ClosedFileInputSource*> 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<PointerHolder<QPDF>> 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)
{