diff --git a/ChangeLog b/ChangeLog index adc6181d..71f241d3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2022-09-07 Jay Berkenbilt + + * Add QPDFObjectHandle::getQPDF(), which returns a reference, as + an alternative to QPDFObjectHandle::getOwningQPDF(). + 2022-09-06 Jay Berkenbilt * For all bounding box methods in QPDFPageObjectHelper other than diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 0730d8a3..f3c8c8eb 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -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* 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: + // 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. // - // * 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. - // - // * 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. + // 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( - bool allow_nullptr = true, std::string const& error_msg = "") const; + 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, diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index aa97de4b..921e943b 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -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 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( - 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(), diff --git a/libqpdf/QPDFFormFieldObjectHelper.cc b/libqpdf/QPDFFormFieldObjectHelper.cc index e56024af..0d405252 100644 --- a/libqpdf/QPDFFormFieldObjectHelper.cc +++ b/libqpdf/QPDFFormFieldObjectHelper.cc @@ -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); } diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index 9530420e..df93b859 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -2194,8 +2194,8 @@ QPDFJob::doUnderOverlayForPage( std::map> 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); diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 885abf3c..3d011c1d 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -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( @@ -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 diff --git a/libqpdf/QPDFPageObjectHelper.cc b/libqpdf/QPDFPageObjectHelper.cc index 1c810318..7da09cf5 100644 --- a/libqpdf/QPDFPageObjectHelper.cc +++ b/libqpdf/QPDFPageObjectHelper.cc @@ -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 old_fields; std::shared_ptr afdhph; if (!afdh) { - afdhph = std::make_shared(*qpdf); + afdhph = std::make_shared(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 new_annots; @@ -1121,19 +1114,19 @@ QPDFPageObjectHelper::copyAnnotations( std::shared_ptr afdhph; std::shared_ptr from_afdhph; if (!afdh) { - afdhph = std::make_shared(*this_qpdf); + afdhph = std::make_shared(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(*from_qpdf); + from_afdhph = std::make_shared(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"); diff --git a/libqpdf/QPDF_pages.cc b/libqpdf/QPDF_pages.cc index 80e89b02..0dad3b53 100644 --- a/libqpdf/QPDF_pages.cc +++ b/libqpdf/QPDF_pages.cc @@ -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"); diff --git a/manual/release-notes.rst b/manual/release-notes.rst index ada6d661..9cf09840 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -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