From e2ac1dcd1e3c0ac4be48095f0a543875ee16d0ce Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Wed, 29 Dec 2021 09:55:56 +0100 Subject: [PATCH] PDF: Fix memory leak when the file has Optional Content links --- core/document.cpp | 31 ++++++++++++++++++++++++++++ core/form.cpp | 7 +++++++ core/form.h | 6 ++++++ core/generator.cpp | 4 ++++ core/generator.h | 7 +++++++ core/page.cpp | 5 +++++ core/page.h | 7 +++++++ generators/poppler/generator_pdf.cpp | 5 +++++ generators/poppler/generator_pdf.h | 1 + 9 files changed, 73 insertions(+) diff --git a/core/document.cpp b/core/document.cpp index c927b55c6..8d0a6ff85 100644 --- a/core/document.cpp +++ b/core/document.cpp @@ -2513,6 +2513,37 @@ void Document::closeDocument() // close the current document and save document info if a document is still opened if (d->m_generator && d->m_pagesVector.size() > 0) { d->saveDocumentInfo(); + + // free the content of the opaque backend actions (if any) + // this is a bit awkward since backends can store "random stuff" in the + // BackendOpaqueAction nativeId qvariant so we need to tell them to free it + // ideally we would just do that in the BackendOpaqueAction destructor + // but that's too late in the cleanup process, i.e. the generator has already closed its document + // and the document generator is nullptr + for (Page *p : qAsConst(d->m_pagesVector)) { + const QLinkedList &oRects = p->objectRects(); + for (ObjectRect *oRect : oRects) { + if (oRect->objectType() == ObjectRect::Action) { + const Action *a = static_cast(oRect->object()); + const BackendOpaqueAction *backendAction = dynamic_cast(a); + if (backendAction) { + d->m_generator->freeOpaqueActionContents(*backendAction); + } + } + } + + const QLinkedList forms = p->formFields(); + for (const FormField *form : forms) { + const QList additionalActions = form->additionalActions(); + for (const Action *a : additionalActions) { + const BackendOpaqueAction *backendAction = dynamic_cast(a); + if (backendAction) { + d->m_generator->freeOpaqueActionContents(*backendAction); + } + } + } + } + d->m_generator->closeDocument(); } diff --git a/core/form.cpp b/core/form.cpp index 0cfb80dfe..10e364cee 100644 --- a/core/form.cpp +++ b/core/form.cpp @@ -115,6 +115,13 @@ void FormField::setAdditionalAction(Annotation::AdditionalActionType type, Actio d->m_additionalAnnotActions[type] = action; } +QList FormField::additionalActions() const +{ + Q_D(const FormField); + // yes, calling values() is not great but it's a list of ~10 elements, we can live with that + return d->m_additionalAnnotActions.values() + d->m_additionalActions.values(); // clazy:exclude=container-anti-pattern +} + class Okular::FormFieldButtonPrivate : public Okular::FormFieldPrivate { public: diff --git a/core/form.h b/core/form.h index 477092fbf..3e543ab28 100644 --- a/core/form.h +++ b/core/form.h @@ -157,6 +157,12 @@ public: */ Action *additionalAction(Annotation::AdditionalActionType type) const; + /* Returns all the additional actions for this form + * + * @since 22.04 + */ + QList additionalActions() const; + protected: /// @cond PRIVATE explicit FormField(FormFieldPrivate &dd); diff --git a/core/generator.cpp b/core/generator.cpp index e24f33f27..980d7fb01 100644 --- a/core/generator.cpp +++ b/core/generator.cpp @@ -386,6 +386,10 @@ void Generator::opaqueAction(const BackendOpaqueAction * /*action*/) { } +void Generator::freeOpaqueActionContents(const BackendOpaqueAction & /*action*/) +{ +} + QVariant Generator::metaData(const QString &key, const QVariant &option) const { Q_D(const Generator); diff --git a/core/generator.h b/core/generator.h index 6172ceeb3..2a9667256 100644 --- a/core/generator.h +++ b/core/generator.h @@ -451,6 +451,13 @@ public: */ virtual void opaqueAction(const BackendOpaqueAction *action); + /** + * Frees the contents of the opaque action (if any); + * + * @since 22.04 + */ + virtual void freeOpaqueActionContents(const BackendOpaqueAction &action); + Q_SIGNALS: /** * This signal should be emitted whenever an error occurred in the generator. diff --git a/core/page.cpp b/core/page.cpp index aaab912cd..f79492bdd 100644 --- a/core/page.cpp +++ b/core/page.cpp @@ -582,6 +582,11 @@ void Page::setObjectRects(const QLinkedList &rects) m_rects << rects; } +const QLinkedList &Page::objectRects() const +{ + return m_rects; +} + void PagePrivate::setHighlight(int s_id, RegularAreaRect *rect, const QColor &color) { HighlightAreaRect *hr = new HighlightAreaRect(rect); diff --git a/core/page.h b/core/page.h index 6df8e4fe2..571f57573 100644 --- a/core/page.h +++ b/core/page.h @@ -283,6 +283,13 @@ public: */ void setObjectRects(const QLinkedList &rects); + /** + * Gets the list of object rects of the page. + * + * @since 22.04 + */ + const QLinkedList &objectRects() const; + /** * Sets the list of source reference objects @p rects. */ diff --git a/generators/poppler/generator_pdf.cpp b/generators/poppler/generator_pdf.cpp index aba7b93ae..65ddd9bd1 100644 --- a/generators/poppler/generator_pdf.cpp +++ b/generators/poppler/generator_pdf.cpp @@ -1008,6 +1008,11 @@ void PDFGenerator::opaqueAction(const Okular::BackendOpaqueAction *action) pdfdoc->optionalContentModel()->applyLink(const_cast(popplerLink)); } +void PDFGenerator::freeOpaqueActionContents(const Okular::BackendOpaqueAction &action) +{ + delete action.nativeId().value(); +} + bool PDFGenerator::isAllowed(Okular::Permission permission) const { bool b = true; diff --git a/generators/poppler/generator_pdf.h b/generators/poppler/generator_pdf.h index aea993dea..1b7bcc1a6 100644 --- a/generators/poppler/generator_pdf.h +++ b/generators/poppler/generator_pdf.h @@ -71,6 +71,7 @@ public: } QAbstractItemModel *layersModel() const override; void opaqueAction(const Okular::BackendOpaqueAction *action) override; + void freeOpaqueActionContents(const Okular::BackendOpaqueAction &action) override; // [INHERITED] document information bool isAllowed(Okular::Permission permission) const override;