Skip to content

Commit

Permalink
formal safety nits around formenvs
Browse files Browse the repository at this point in the history
1) For closing, attach pages to their direct parent only. If a formenv
exists, it will always be closed before the document and handle any open
pages, so handling them again on document level is needless (though it
wouldn't harm).
2) Keep a reference to the formenv on page level and capture it in the
finalizer at init time to make the implementation formally safe.
(Previously, the caller could interfere by tampering with
`pdf.formenv`.)
  • Loading branch information
mara004 committed Jun 12, 2023
1 parent 5e19859 commit 5f2bd3f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
11 changes: 7 additions & 4 deletions src/pypdfium2/_helpers/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ def __init__(self, input, password=None, autoclose=False):
self._input = input
self._password = password
self._autoclose = autoclose

self._data_holder = []
self._data_closer = []

# question: can we make attributes like formenv effectively immutable for the caller?
self.formenv = None

if isinstance(self._input, pdfium_c.FPDF_DOCUMENT):
Expand Down Expand Up @@ -370,12 +371,13 @@ def get_page(self, index):
raw_page = pdfium_c.FPDF_LoadPage(self, index)
if not raw_page:
raise PdfiumError("Failed to load page.")
page = PdfPage(raw_page, self)
self._add_kid(page)
page = PdfPage(raw_page, self, self.formenv)

if self.formenv:
pdfium_c.FORM_OnAfterLoadPage(page, self.formenv)
self.formenv._add_kid(page)
else:
self._add_kid(page)

return page

Expand All @@ -398,7 +400,8 @@ def new_page(self, width, height, index=None):
if index is None:
index = len(self)
raw_page = pdfium_c.FPDFPage_New(self, index, width, height)
page = PdfPage(raw_page, self)
page = PdfPage(raw_page, self, None)
# FIXME should we make the formenv distinction for new pages, too?
self._add_kid(page)
return page

Expand Down
17 changes: 9 additions & 8 deletions src/pypdfium2/_helpers/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,21 @@ class PdfPage (pdfium_i.AutoCloseable):
pdf (PdfDocument): Reference to the document this page belongs to.
"""

def __init__(self, raw, pdf):
self.raw, self.pdf = raw, pdf
super().__init__(PdfPage._close_impl, self.pdf)
def __init__(self, raw, pdf, formenv):
self.raw, self.pdf, self.formenv = raw, pdf, formenv
super().__init__(PdfPage._close_impl, self.formenv)


@staticmethod
def _close_impl(raw, pdf):
if pdf.formenv:
pdfium_c.FORM_OnBeforeClosePage(raw, pdf.formenv)
def _close_impl(raw, formenv):
if formenv:
pdfium_c.FORM_OnBeforeClosePage(raw, formenv)
pdfium_c.FPDF_ClosePage(raw)


@property
def parent(self): # AutoCloseable hook
# Might want to have this point to the direct parent, i. e. (self.pdf if formenv is None else self.formenv), but this might confuse callers expecting that parent be always pdf for pages.
return self.pdf


Expand Down Expand Up @@ -443,8 +444,8 @@ def render(
assert status == pdfium_c.FPDF_RENDER_DONE
pdfium_c.FPDF_RenderPage_Close(self)

if may_draw_forms and self.pdf.formenv:
pdfium_c.FPDF_FFLDraw(self.pdf.formenv, *render_args)
if may_draw_forms and self.formenv:
pdfium_c.FPDF_FFLDraw(self.formenv, *render_args)

return bitmap

Expand Down

0 comments on commit 5f2bd3f

Please sign in to comment.