Revert getOwningQPDF, and add getQPDF that returns a reference

Jay Berkenbilt
1 parent bac55955
Showing 9 changed files with 88 additions and 70 deletions
ChangeLog
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDF.cc
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDFJob.cc
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDFPageObjectHelper.cc
libqpdf/QPDF_pages.cc
manual/release-notes.rst
+2022-09-07  Jay Berkenbilt  <ejb@ql.org>
+
+        * Add QPDFObjectHandle::getQPDF(), which returns a reference, as
+        an alternative to QPDFObjectHandle::getOwningQPDF().
+
 2022-09-06  Jay Berkenbilt  <ejb@ql.org>
  
         * For all bounding box methods in QPDFPageObjectHelper other than
@@ -661,14 +661,17 @@ class QPDFObjectHandle
     static QPDFObjectHandle newReserved(QPDF* qpdf);
  
     // Provide an owning qpdf and object description. The library does
-    // this automatically with objects that are read from the
-    // input PDF and with objects that are created programmatically
-    // and inserted into the QPDF by adding them to an array or a
-    // dictionary or creating a new indirect object. Most end user
+    // this automatically with objects that are read from the input
+    // PDF and with objects that are created programmatically and
+    // inserted into the QPDF as a new indirect object. Most end user
     // code will not need to call this. If an object has an owning
     // qpdf and object description, it enables qpdf to give warnings
     // with proper context in some cases where it would otherwise
-    // raise exceptions.
+    // raise exceptions. It is okay to add objects without an
+    // owning_qpdf to objects that have one, but it is an error to
+    // have a QPDF contain objects with owning_qpdf set to something
+    // else. To add objects from another qpdf, use copyForeignObject
+    // instead.
     QPDF_DLL
     void setObjectDescription(
         QPDF* owning_qpdf, std::string const& object_description);
@@ -978,23 +981,34 @@ class QPDFObjectHandle
         int& min_suffix,
         std::set<std::string>* resource_names = nullptr);
  
-    // If this is an indirect object, return a pointer to the QPDF
-    // object that owns an indirect object. Direct objects are not
-    // owned by a QPDF. Usage notes:
-    //
-    // * When allow_nullptr is true, this method will return a null
-    //   pointer if the object is not owned by a QPDF. Otherwise, an
-    //   exception is thrown.
+    // A QPDFObjectHandle has an owning QPDF if it is associated with
+    // ("owned by") a specific QPDF object. Indirect objects always
+    // have an owning QPDF. Direct objects that are read from the
+    // input source will also have an owning QPDF. Programmatically
+    // created objects will only have one if setObjectDescription was
+    // called.
     //
-    // * You should not retain the value returned by this method for
-    //   longer than the lifetime of the owning QPDF object. If you
-    //   retain a QPDFObjectHandle longer than the owning QPDF, when
-    //   the QPDF object is destroyed, the QPDFObjectHandle will turn
-    //   into a direct "null" object, and getOwningQPDF() called on it
-    //   at that time will return a null pointer.
-    QPDF_DLL
-    QPDF* getOwningQPDF(
-        bool allow_nullptr = true, std::string const& error_msg = "") const;
+    // When the QPDF object that owns an object is destroyed, the
+    // object is changed into a null, and its owner is cleared.
+    // Therefore you should not retain the value of an owning QPDF
+    // beyond the life of the QPDF. If in doubt, ask for it each time
+    // you need it.
+
+    // getOwningQPDF returns a pointer to the owning QPDF is the
+    // object has one. Otherwise, it returns a null pointer. Use this
+    // when you are able to handle the case of an object that doesn't
+    // have an owning QPDF.
+    QPDF_DLL
+    QPDF* getOwningQPDF() const;
+    // getQPDF, new in qpdf 11, returns a reference owning QPDF. If
+    // there is none, it throws a runtime_error. Use this when you
+    // know the object has to have an owning QPDF, such as when it's a
+    // known indirect object. Since streams are always indirect
+    // objects, this method can be used safely for streams. If
+    // error_msg is specified, it will be used at the contents of the
+    // runtime_error if there is now owner.
+    QPDF_DLL
+    QPDF& getQPDF(std::string const& error_msg = "") const;
  
     // Create a shallow copy of an object as a direct object, but do not
     // traverse across indirect object boundaries. That means that,
@@ -2299,14 +2299,14 @@ QPDF::copyForeignObject(QPDFObjectHandle foreign)
         throw std::logic_error(
             "QPDF::copyForeign called with direct object handle");
     }
-    QPDF* other = foreign.getOwningQPDF(false);
-    if (other == this) {
+    QPDF& other = foreign.getQPDF();
+    if (&other == this) {
         QTC::TC("qpdf", "QPDF copyForeign not foreign");
         throw std::logic_error(
             "QPDF::copyForeign called with object from this QPDF");
     }
  
-    ObjCopier& obj_copier = this->m->object_copiers[other->m->unique_id];
+    ObjCopier& obj_copier = this->m->object_copiers[other.m->unique_id];
     if (!obj_copier.visiting.empty()) {
         throw std::logic_error("obj_copier.visiting is not empty"
                                " at the beginning of copyForeignObject");
@@ -2490,8 +2490,8 @@ QPDF::copyStreamData(QPDFObjectHandle result, QPDFObjectHandle foreign)
     // Copy information from the foreign stream so we can pipe its
     // data later without keeping the original QPDF object around.
  
-    QPDF* foreign_stream_qpdf = foreign.getOwningQPDF(
-        false, "unable to retrieve owning qpdf from foreign stream");
+    QPDF& foreign_stream_qpdf =
+        foreign.getQPDF("unable to retrieve owning qpdf from foreign stream");
  
     auto stream = QPDFObjectHandle::ObjAccessor::asStream(foreign);
     if (stream == nullptr) {
@@ -2499,7 +2499,7 @@ QPDF::copyStreamData(QPDFObjectHandle result, QPDFObjectHandle foreign)
                                " stream object from foreign stream");
     }
     std::shared_ptr<Buffer> stream_buffer = stream->getStreamDataBuffer();
-    if ((foreign_stream_qpdf->m->immediate_copy_from) &&
+    if ((foreign_stream_qpdf.m->immediate_copy_from) &&
         (stream_buffer == nullptr)) {
         // Pull the stream data into a buffer before attempting
         // the copy operation. Do it on the source stream so that
@@ -2529,8 +2529,8 @@ QPDF::copyStreamData(QPDFObjectHandle result, QPDFObjectHandle foreign)
             dict.getKey("/DecodeParms"));
     } else {
         auto foreign_stream_data = std::make_shared<ForeignStreamData>(
-            foreign_stream_qpdf->m->encp,
-            foreign_stream_qpdf->m->file,
+            foreign_stream_qpdf.m->encp,
+            foreign_stream_qpdf.m->file,
             foreign.getObjGen(),
             stream->getOffset(),
             stream->getLength(),
@@ -362,12 +362,10 @@ QPDFFormFieldObjectHelper::setV(QPDFObjectHandle value, bool need_appearances)
         setFieldAttribute("/V", value);
     }
     if (need_appearances) {
-        QPDF* qpdf = this->oh.getOwningQPDF(
-            false,
+        QPDF& qpdf = this->oh.getQPDF(
             "QPDFFormFieldObjectHelper::setV called with need_appearances = "
             "true on an object that is not associated with an owning QPDF");
-
-        QPDFAcroFormDocumentHelper(*qpdf).setNeedAppearances(true);
+        QPDFAcroFormDocumentHelper(qpdf).setNeedAppearances(true);
     }
 }
  
@@ -881,7 +879,7 @@ QPDFFormFieldObjectHelper::generateTextAppearance(
         if (found_font_in_dr && resources.isDictionary()) {
             QTC::TC("qpdf", "QPDFFormFieldObjectHelper get font from /DR");
             if (resources.isIndirect()) {
-                resources = resources.getOwningQPDF(false)->makeIndirectObject(
+                resources = resources.getQPDF().makeIndirectObject(
                     resources.shallowCopy());
                 AS.getDict().replaceKey("/Resources", resources);
             }
@@ -2194,8 +2194,8 @@ QPDFJob::doUnderOverlayForPage(
     std::map<unsigned long long, std::shared_ptr<QPDFAcroFormDocumentHelper>>
         afdh;
     auto make_afdh = [&](QPDFPageObjectHelper& ph) {
-        QPDF* q = ph.getObjectHandle().getOwningQPDF(false);
-        return get_afdh_for_qpdf(afdh, q);
+        QPDF& q = ph.getObjectHandle().getQPDF();
+        return get_afdh_for_qpdf(afdh, &q);
     };
     auto dest_afdh = make_afdh(dest_page);
  
@@ -2635,7 +2635,7 @@ static QPDFObjectHandle
 added_page(QPDF& pdf, QPDFObjectHandle page)
 {
     QPDFObjectHandle result = page;
-    if (page.getOwningQPDF(false) != &pdf) {
+    if (&page.getQPDF() != &pdf) {
         // Calling copyForeignObject on an object we already copied
         // will give us the already existing copy.
         result = pdf.copyForeignObject(page);
@@ -1668,11 +1668,10 @@ QPDFObjectHandle::coalesceContentStreams()
     // incorrect way. However, it can happen in a PDF file whose
     // page structure is direct, which is against spec but still
     // possible to hand construct, as in fuzz issue 27393.
-    QPDF* qpdf = getOwningQPDF(
-        false,
+    QPDF& qpdf = getQPDF(
         "coalesceContentStreams called on object  with no associated PDF file");
  
-    QPDFObjectHandle new_contents = newStream(qpdf);
+    QPDFObjectHandle new_contents = newStream(&qpdf);
     this->replaceKey("/Contents", new_contents);
  
     auto provider = std::shared_ptr<StreamDataProvider>(
@@ -2775,16 +2774,20 @@ QPDFObjectHandle::getObjGen() const
  
 // Indirect object accessors
 QPDF*
-QPDFObjectHandle::getOwningQPDF(
-    bool allow_nullptr, std::string const& error_msg) const
+QPDFObjectHandle::getOwningQPDF() const
+{
+    return isInitialized() ? this->obj->getQPDF() : nullptr;
+}
+
+QPDF&
+QPDFObjectHandle::getQPDF(std::string const& error_msg) const
 {
-    // Will be null for direct objects
     auto result = isInitialized() ? this->obj->getQPDF() : nullptr;
-    if (!allow_nullptr && (result == nullptr)) {
+    if (result == nullptr) {
         throw std::runtime_error(
             error_msg == "" ? "attempt to use a null qpdf object" : error_msg);
     }
-    return result;
+    return *result;
 }
  
 void
@@ -477,8 +477,7 @@ QPDFPageObjectHelper::externalizeInlineImages(size_t min_size, bool shallow)
                 this->oh.replaceKey(
                     "/Contents",
                     QPDFObjectHandle::newStream(
-                        this->oh.getOwningQPDF(false),
-                        b.getBufferSharedPointer()));
+                        &this->oh.getQPDF(), b.getBufferSharedPointer()));
             }
         }
     } else {
@@ -729,12 +728,10 @@ QPDFPageObjectHelper::removeUnreferencedResources()
 QPDFPageObjectHelper
 QPDFPageObjectHelper::shallowCopyPage()
 {
-    QPDF* qpdf = this->oh.getOwningQPDF(
-        false,
+    QPDF& qpdf = this->oh.getQPDF(
         "QPDFPageObjectHelper::shallowCopyPage called with a direct object");
-
     QPDFObjectHandle new_page = this->oh.shallowCopy();
-    return QPDFPageObjectHelper(qpdf->makeIndirectObject(new_page));
+    return QPDFPageObjectHelper(qpdf.makeIndirectObject(new_page));
 }
  
 QPDFObjectHandle::Matrix
@@ -788,11 +785,10 @@ QPDFPageObjectHelper::getMatrixForTransformations(bool invert)
 QPDFObjectHandle
 QPDFPageObjectHelper::getFormXObjectForPage(bool handle_transformations)
 {
-    QPDF* qpdf = this->oh.getOwningQPDF(
-        false,
+    QPDF& qpdf = this->oh.getQPDF(
         "QPDFPageObjectHelper::getFormXObjectForPage called with a direct "
         "object");
-    QPDFObjectHandle result = QPDFObjectHandle::newStream(qpdf);
+    QPDFObjectHandle result = QPDFObjectHandle::newStream(&qpdf);
     QPDFObjectHandle newdict = result.getDict();
     newdict.replaceKey("/Type", QPDFObjectHandle::newName("/XObject"));
     newdict.replaceKey("/Subtype", QPDFObjectHandle::newName("/Form"));
@@ -961,10 +957,8 @@ QPDFPageObjectHelper::placeFormXObject(
 void
 QPDFPageObjectHelper::flattenRotation(QPDFAcroFormDocumentHelper* afdh)
 {
-    QPDF* qpdf = this->oh.getOwningQPDF(
-        false,
+    QPDF& qpdf = this->oh.getQPDF(
         "QPDFPageObjectHelper::flattenRotation called with a direct object");
-
     auto rotate_oh = this->oh.getKey("/Rotate");
     int rotate = 0;
     if (rotate_oh.isInteger()) {
@@ -1067,8 +1061,9 @@ QPDFPageObjectHelper::flattenRotation(QPDFAcroFormDocumentHelper* afdh)
         break;
     }
     std::string cm_str = std::string("q\n") + cm.unparse() + " cm\n";
-    this->oh.addPageContents(QPDFObjectHandle::newStream(qpdf, cm_str), true);
-    this->oh.addPageContents(QPDFObjectHandle::newStream(qpdf, "\nQ\n"), false);
+    this->oh.addPageContents(QPDFObjectHandle::newStream(&qpdf, cm_str), true);
+    this->oh.addPageContents(
+        QPDFObjectHandle::newStream(&qpdf, "\nQ\n"), false);
     this->oh.removeKey("/Rotate");
     QPDFObjectHandle rotate_obj = getAttribute("/Rotate", false);
     if (!rotate_obj.isNull()) {
@@ -1083,7 +1078,7 @@ QPDFPageObjectHelper::flattenRotation(QPDFAcroFormDocumentHelper* afdh)
         std::set<QPDFObjGen> old_fields;
         std::shared_ptr<QPDFAcroFormDocumentHelper> afdhph;
         if (!afdh) {
-            afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(*qpdf);
+            afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(qpdf);
             afdh = afdhph.get();
         }
         afdh->transformAnnotations(
@@ -1108,11 +1103,9 @@ QPDFPageObjectHelper::copyAnnotations(
         return;
     }
  
-    QPDF* from_qpdf = from_page.getObjectHandle().getOwningQPDF(
-        false,
+    QPDF& from_qpdf = from_page.getObjectHandle().getQPDF(
         "QPDFPageObjectHelper::copyAnnotations: from page is a direct object");
-    QPDF* this_qpdf = this->oh.getOwningQPDF(
-        false,
+    QPDF& this_qpdf = this->oh.getQPDF(
         "QPDFPageObjectHelper::copyAnnotations: this page is a direct object");
  
     std::vector<QPDFObjectHandle> new_annots;
@@ -1121,19 +1114,19 @@ QPDFPageObjectHelper::copyAnnotations(
     std::shared_ptr<QPDFAcroFormDocumentHelper> afdhph;
     std::shared_ptr<QPDFAcroFormDocumentHelper> from_afdhph;
     if (!afdh) {
-        afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(*this_qpdf);
+        afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(this_qpdf);
         afdh = afdhph.get();
     }
-    if (this_qpdf == from_qpdf) {
+    if (&this_qpdf == &from_qpdf) {
         from_afdh = afdh;
     } else if (from_afdh) {
-        if (from_afdh->getQPDF().getUniqueId() != from_qpdf->getUniqueId()) {
+        if (from_afdh->getQPDF().getUniqueId() != from_qpdf.getUniqueId()) {
             throw std::logic_error(
                 "QPDFAcroFormDocumentHelper::copyAnnotations: from_afdh"
                 " is not from the same QPDF as from_page");
         }
     } else {
-        from_afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(*from_qpdf);
+        from_afdhph = std::make_shared<QPDFAcroFormDocumentHelper>(from_qpdf);
         from_afdh = from_afdhph.get();
     }
  
@@ -1143,7 +1136,7 @@ QPDFPageObjectHelper::copyAnnotations(
         new_fields,
         old_fields,
         cm,
-        from_qpdf,
+        &from_qpdf,
         from_afdh);
     afdh->addAndRenameFormFields(new_fields);
     auto annots = this->oh.getKey("/Annots");
@@ -233,7 +233,7 @@ QPDF::insertPage(QPDFObjectHandle newpage, int pos)
         newpage = makeIndirectObject(newpage);
     } else if (newpage.getOwningQPDF() != this) {
         QTC::TC("qpdf", "QPDF insert foreign page");
-        newpage.getOwningQPDF(false)->pushInheritedAttributesToPage();
+        newpage.getQPDF().pushInheritedAttributesToPage();
         newpage = copyForeignObject(newpage);
     } else {
         QTC::TC("qpdf", "QPDF insert indirect page");
@@ -208,6 +208,11 @@ For a detailed list of changes, please see the file
       generally not happen for correct code, but at least the
       situation is detectible now.
  
+    - The method ``QPDFObjectHandle::getQPDF`` returns a ``QPDF&``
+      (rather than a ``QPDF*``) and is an alternative to
+      ``QPDFObjectHandle::getOwningQPDF`` for when the object is known
+      to have an owning ``QPDF``.
+
     - It is now possible to test ``QPDFObjectHandle`` equality with
       ``==`` and ``!=``. Two ``QPDFObjectHandle`` objects are equal if
       they point to the same underlying object, meaning changes to one