Have QPDFObjectHandle notice when replaceObject was called

This results in a performance penalty of 1% to 2% when replaceObject
and swapObjects are never called and a somewhat larger penalty if they
are called, but it's worth it to avoid very confusing behavior as
discussed in depth in qpdf#507.

2 files changed, 33 insertions, 0 deletions

diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc
index 1935d420..b751e602 100644
--- a/libqpdf/QPDF.cc
+++ b/libqpdf/QPDF.cc
@@ -210,6 +210,7 @@ QPDF::Members::Members() :
     immediate_copy_from(false),
     in_parse(false),
     parsed(false),
+    ever_replaced_objects(false),
     first_xref_item_offset(0),
     uncompressed_after_compressed(false)
 {
@@ -2063,6 +2064,30 @@ QPDF::readObjectAtOffset(bool try_recovery,
     return oh;
 }
 
+bool
+QPDF::objectChanged(QPDFObjGen const& og, PointerHolder<QPDFObject>& oph)
+{
+    // See if the object cached at og, if any, is the one passed in.
+    // QPDFObjectHandle uses this to detect outdated handles to
+    // replaced or swapped objects. This is a somewhat expensive check
+    // because it happens with every dereference of a
+    // QPDFObjectHandle. To reduce the hit somewhat, short-circuit the
+    // check if we never called a function that replaces an object
+    // already in cache. It is important for functions that do this to
+    // set ever_replaced_objects = true.
+
+    if (! this->m->ever_replaced_objects)
+    {
+        return false;
+    }
+    auto c = this->m->obj_cache.find(og);
+    if (c == this->m->obj_cache.end())
+    {
+        return true;
+    }
+    return (c->second.object.getPointer() != oph.getPointer());
+}
+
 PointerHolder<QPDFObject>
 QPDF::resolve(int objid, int generation)
 {
@@ -2308,6 +2333,7 @@ QPDF::replaceObject(int objid, int generation, QPDFObjectHandle oh)
 
     // Replace the object in the object cache
     QPDFObjGen og(objid, generation);
+    this->m->ever_replaced_objects = true;
     this->m->obj_cache[og] =
 	ObjCache(QPDFObjectHandle::ObjAccessor::getObject(oh), -1, -1);
 }
@@ -2695,6 +2721,7 @@ QPDF::swapObjects(int objid1, int generation1, int objid2, int generation2)
     QPDFObjGen og1(objid1, generation1);
     QPDFObjGen og2(objid2, generation2);
     ObjCache t = this->m->obj_cache[og1];
+    this->m->ever_replaced_objects = true;
     this->m->obj_cache[og1] = this->m->obj_cache[og2];
     this->m->obj_cache[og2] = t;
 }
diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc
index c650bdea..6d4f10ce 100644
--- a/libqpdf/QPDFObjectHandle.cc
+++ b/libqpdf/QPDFObjectHandle.cc
@@ -3194,6 +3194,12 @@ QPDFObjectHandle::dereference()
         throw std::logic_error(
             "attempted to dereference an uninitialized QPDFObjectHandle");
     }
+    if (this->obj.getPointer() && this->objid &&
+        QPDF::Resolver::objectChanged(
+            this->qpdf, QPDFObjGen(this->objid, this->generation), this->obj))
+    {
+        this->obj = nullptr;
+    }
     if (this->obj.getPointer() == 0)
     {
         PointerHolder<QPDFObject> obj = QPDF::Resolver::resolve(