diff --git a/ChangeLog b/ChangeLog index 66f4e262..fdb6b1b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2022-09-02 Jay Berkenbilt + + * Add new QPDF::create() factory method that returns + std::shared_ptr. + + * Prevent copying/assigning to QPDF objects in the API. It has + never been safe to do this, but the API wasn't preventing it. + 2022-09-01 Jay Berkenbilt * Remove QPDFObject.hh from include/qpdf. The only reason to diff --git a/TODO b/TODO index a817d3b2..89aed473 100644 --- a/TODO +++ b/TODO @@ -4,8 +4,6 @@ Next Before Release: -* Consider a transition symbol for deprecation of stack-allocated QPDF - objects and also make QPDF non-copiable. * Evaluate issues tagged with `next` * Stay on top of https://github.com/pikepdf/pikepdf/pull/315 * Consider whether otherwise unreferenced object streams should be diff --git a/fuzz/qpdf_fuzzer.cc b/fuzz/qpdf_fuzzer.cc index 00b33b17..c402a772 100644 --- a/fuzz/qpdf_fuzzer.cc +++ b/fuzz/qpdf_fuzzer.cc @@ -55,7 +55,7 @@ FuzzHelper::getQpdf() { auto is = std::shared_ptr( new BufferInputSource("fuzz input", &this->input_buffer)); - auto qpdf = std::make_shared(); + auto qpdf = QPDF::create(); qpdf->processInputSource(is); return qpdf; } diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh index 68cc333b..567817f1 100644 --- a/include/qpdf/QPDF.hh +++ b/include/qpdf/QPDF.hh @@ -64,6 +64,9 @@ class QPDF QPDF_DLL ~QPDF(); + QPDF_DLL + static std::shared_ptr create(); + // Associate a file with a QPDF object and do initial parsing of // the file. PDF objects are not read until they are needed. A // QPDF object may be associated with only one file in its @@ -929,6 +932,15 @@ class QPDF static bool test_json_validators(); private: + // It has never been safe to copy QPDF objects as there is code in + // the library that assumes there are no copies of a QPDF object. + // Copying QPDF objects was not prevented by the API until qpdf + // 11. If you have been copying QPDF objects, use + // std::shared_ptr instead. From qpdf 11, you can use + // QPDF::create to create them. + QPDF(QPDF const&) = delete; + QPDF& operator=(QPDF const&) = delete; + static std::string const qpdf_version; class ObjCache diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index e3da2e64..49a33633 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -264,6 +264,12 @@ QPDF::~QPDF() } } +std::shared_ptr +QPDF::create() +{ + return std::make_shared(); +} + void QPDF::processFile(char const* filename, char const* password) { diff --git a/libqpdf/QPDFJob.cc b/libqpdf/QPDFJob.cc index 7bd563aa..691f193c 100644 --- a/libqpdf/QPDFJob.cc +++ b/libqpdf/QPDFJob.cc @@ -1981,7 +1981,7 @@ QPDFJob::doProcessOnce( bool used_for_input, bool main_input) { - auto pdf = std::make_shared(); + auto pdf = QPDF::create(); setQPDFOptions(*pdf); if (empty) { pdf->emptyPDF(); diff --git a/libqpdf/qpdf-c.cc b/libqpdf/qpdf-c.cc index 390699db..b5bae727 100644 --- a/libqpdf/qpdf-c.cc +++ b/libqpdf/qpdf-c.cc @@ -148,7 +148,7 @@ qpdf_init() { QTC::TC("qpdf", "qpdf-c called qpdf_init"); qpdf_data qpdf = new _qpdf_data(); - qpdf->qpdf = std::make_shared(); + qpdf->qpdf = QPDF::create(); return qpdf; } diff --git a/manual/release-notes.rst b/manual/release-notes.rst index e3e65d88..abc3f42a 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -139,11 +139,18 @@ For a detailed list of changes, please see the file - See :ref:`breaking-crypto-api` for specific details, and see :ref:`weak-crypto` for a general discussion. - - QPDFObjectHandle::warnIfPossible no longer takes an optional + - ``QPDFObjectHandle::warnIfPossible`` no longer takes an optional argument to throw an exception if there is no description. If there is no description, it writes to the default logger's error stream. + - ``QPDF`` objects can no longer be copied or assigned to. It has + never been safe to do this because of assumptions made by + library code. Now it is prevented by the API. If you run into + trouble, use ``QPDF::create()`` to create ``QPDF`` shared + pointers (or create them in some other way if you need backward + compatibility with older qpdf versions). + - CLI Enhancements - ``qpdf --list-attachments --verbose`` include some additional @@ -196,6 +203,9 @@ For a detailed list of changes, please see the file generally not happen for correct code, but at least the situation is detectible now. + - Add new factory method ``QPDF::create()`` that returns a + ``std::shared_ptr``. + - Add new ``Pipeline`` methods to reduce the amount of casting that is needed: diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 095543c1..ea4ac73a 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -3262,12 +3262,12 @@ static void test_92(QPDF& pdf, char const* arg2) { // Exercise indirect objects owned by destroyed QPDF object. - QPDF* qpdf = new QPDF(); + auto qpdf = QPDF::create(); qpdf->emptyPDF(); auto root = qpdf->getRoot(); assert(root.getOwningQPDF() != nullptr); assert(root.isIndirect()); - delete qpdf; + qpdf = nullptr; assert(root.getOwningQPDF() == nullptr); assert(!root.isIndirect()); }