From 0a52ef22a785a6679b217234bfd9ccc785217042 Mon Sep 17 00:00:00 2001 From: Ak4shh Date: Mon, 26 Oct 2026 20:40:36 -0400 Subject: [PATCH 1/4] Fix cloning of objects that require constructor arguments DictionaryObject.clone() and DictionaryObject.replicate() created the target object via self.__class__(). That fails for every annotation class (Text, FreeText, Link, Polygon, ...), because their constructors require arguments, so PageObject.merge_page() raised TypeError whenever the merged page carried a markup annotation. Create the empty target through a helper that skips __init__ when the class cannot be instantiated without arguments. The clone is populated from the source object right afterwards, so the constructor arguments are not needed; the annotation constructors only store their arguments in the dictionary anyway. Fixes #3467. Signed-off-by: Ak4shh --- pypdf/generic/_data_structures.py | 35 +++++++++++++++++++++++++++++-- tests/test_annotations.py | 35 +++++++++++++++++++++++++++++++ tests/test_page.py | 26 +++++++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/pypdf/generic/_data_structures.py b/pypdf/generic/_data_structures.py index 24f27f8351..dd104de52b 100644 --- a/pypdf/generic/_data_structures.py +++ b/pypdf/generic/_data_structures.py @@ -28,11 +28,13 @@ __author__ = "Mathieu Fenniak" __author_email__ = "biziqe@mathieu.fenniak.net" +import inspect import logging import os import re import sys from collections.abc import Callable, Iterable, Sequence +from functools import cache from io import BytesIO from math import ceil from typing import ( @@ -302,14 +304,43 @@ def read_from_stream( return arr +@cache +def _requires_constructor_arguments(cls: type["DictionaryObject"]) -> bool: + """Report whether ``cls`` can only be instantiated with arguments.""" + parameters = inspect.signature(cls.__init__).parameters + return any( + parameter.default is inspect.Parameter.empty + and parameter.kind + in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.KEYWORD_ONLY, + ) + for parameter in list(parameters.values())[1:] + ) + + class DictionaryObject(dict[Any, Any], PdfObject): + def _new_for_clone(self) -> "DictionaryObject": + """Create a blank instance of this class to receive cloned entries. + + Most dictionary classes can be instantiated without arguments, but some + subclasses - the annotation dictionaries for instance - can only be + instantiated with arguments. Those arguments are not needed to receive a + clone, because every entry is copied from the source object afterwards, + so such classes are instantiated without calling ``__init__``. + """ + if _requires_constructor_arguments(self.__class__): + return self.__class__.__new__(self.__class__) + return self.__class__() + def replicate( self, pdf_dest: PdfWriterProtocol, ) -> "DictionaryObject": d__ = cast( "DictionaryObject", - self._reference_clone(self.__class__(), pdf_dest, False), + self._reference_clone(self._new_for_clone(), pdf_dest, False), ) for k, v in self.items(): d__[k.replicate(pdf_dest)] = ( @@ -335,7 +366,7 @@ def clone( visited: set[tuple[int, int]] = set() # (idnum, generation) d__ = cast( "DictionaryObject", - self._reference_clone(self.__class__(), pdf_dest, force_duplicate), + self._reference_clone(self._new_for_clone(), pdf_dest, force_duplicate), ) if ignore_fields is None: ignore_fields = [] diff --git a/tests/test_annotations.py b/tests/test_annotations.py index b157eaddbd..324638d710 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -683,3 +683,38 @@ def test_rectangle__default_border_color(): annotation = Rectangle(rect=(10, 10, 90, 90)) assert "/C" not in annotation assert "/IC" not in annotation + + +@pytest.mark.parametrize( + "annotation", + [ + Ellipse(rect=(0, 0, 100, 100)), + FreeText(rect=(0, 0, 100, 100), text="free text"), + Highlight( + rect=(0, 0, 100, 100), quad_points=ArrayObject([FloatObject(0)] * 8) + ), + Line(rect=(0, 0, 100, 100), p1=(0, 0), p2=(100, 100)), + Link(rect=(0, 0, 100, 100), url="https://example.com"), + Polygon(vertices=[(0, 0), (100, 100), (50, 50)]), + PolyLine(vertices=[(0, 0), (100, 100)]), + Popup(rect=(0, 0, 100, 100)), + Rectangle(rect=(0, 0, 100, 100)), + Text(rect=(0, 0, 100, 100), text="text"), + ], + ids=lambda annotation: type(annotation).__name__, +) +def test_clone_and_replicate_annotation(annotation): + """Annotations requiring constructor arguments can be cloned and replicated.""" + writer = PdfWriter() + + cloned = annotation.clone(writer, force_duplicate=True) + replicated = annotation.replicate(writer) + + for copy in (cloned, replicated): + assert type(copy) is type(annotation) + assert copy is not annotation + assert copy.items() == annotation.items() + + # The copy must not share its entries with the source annotation. + copy[NameObject("/Test")] = NumberObject(1) + assert "/Test" not in annotation diff --git a/tests/test_page.py b/tests/test_page.py index 82225301fa..048400882b 100644 --- a/tests/test_page.py +++ b/tests/test_page.py @@ -967,6 +967,32 @@ def test_merge_page_with_annotations(): assert len(page0.annotations) == 1 +def test_merge_page_with_annotations_of_merged_page(): + """Annotations of the page passed to merge_page are cloned as well.""" + writer = PdfWriter() + merged_writer = PdfWriter() + writer.add_blank_page(100, 100) + merged_writer.add_blank_page(100, 100) + + writer.add_annotation( + page_number=0, + annotation=Polygon(vertices=[(50, 550), (200, 650), (70, 750), (50, 700)]), + ) + + page = merged_writer.pages[0] + page.merge_page(writer.pages[0]) + + assert len(page.annotations) == 1 + assert page.annotations[0]["/Subtype"] == "/Polygon" + + buffer = BytesIO() + merged_writer.write(buffer) + + reread_page = PdfReader(buffer).pages[0] + assert len(reread_page.annotations) == 1 + assert reread_page.annotations[0]["/Subtype"] == "/Polygon" + + @pytest.mark.parametrize( ("quad_count", "transformation", "expected"), [ From c54b421367703f7945e6cde470b0003fee3930a7 Mon Sep 17 00:00:00 2001 From: Ak4shh Date: Fri, 30 Oct 2026 20:22:12 -0400 Subject: [PATCH 2/4] Clone linked objects requiring constructor arguments Linked entries such as /Next, /Prev, /N and /V were still instantiated via `__class__()`, failing for classes like OutlineItem. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Ak4shh --- pypdf/generic/_data_structures.py | 2 +- tests/generic/test_data_structures.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pypdf/generic/_data_structures.py b/pypdf/generic/_data_structures.py index dd104de52b..29afa2ef95 100644 --- a/pypdf/generic/_data_structures.py +++ b/pypdf/generic/_data_structures.py @@ -446,7 +446,7 @@ def _clone( clon = cast( "DictionaryObject", cur_obj._reference_clone( - cur_obj.__class__(), pdf_dest, force_duplicate + cur_obj._new_for_clone(), pdf_dest, force_duplicate ), ) # Check to see if we've previously processed our item diff --git a/tests/generic/test_data_structures.py b/tests/generic/test_data_structures.py index b850675c5a..fc4ab41c31 100644 --- a/tests/generic/test_data_structures.py +++ b/tests/generic/test_data_structures.py @@ -16,9 +16,11 @@ ContentStream, Destination, DictionaryObject, + Fit, NameObject, NullObject, NumberObject, + OutlineItem, RectangleObject, StreamObject, StreamParsingBudget, @@ -35,6 +37,31 @@ resource = None # type: ignore[assignment] +@pytest.mark.parametrize("link", ["/Next", "/Prev", "/N", "/V"]) +def test_dictionary_object__clone_linked_objects_with_constructor_arguments(link: str) -> None: + """Linked dictionaries requiring constructor arguments can be cloned.""" + source = PdfWriter() + destination = PdfWriter() + first = OutlineItem("First", NumberObject(0), Fit.fit()) + second = OutlineItem("Second", NumberObject(0), Fit.fit()) + source._add_object(first) + first[NameObject(link)] = source._add_object(second) + + cloned = first.clone(destination) + cloned_second = cloned[link] + + assert isinstance(cloned, OutlineItem) + assert isinstance(cloned_second, OutlineItem) + assert cloned is not first + assert cloned_second is not second + assert cloned.title == first.title + assert cloned_second.items() == second.items() + assert cloned.indirect_reference.pdf is destination + assert cloned_second.indirect_reference.pdf is destination + assert first[link] is second + assert second.indirect_reference.pdf is source + + def test_dictionary_object__get_next_object_position() -> None: reader = PdfReader(RESOURCE_ROOT / "crazyones.pdf") From 18da6d4d3b8f4d33ef01ded832c021aad4733599 Mon Sep 17 00:00:00 2001 From: Ak4shh Date: Fri, 30 Oct 2026 20:34:50 -0400 Subject: [PATCH 3/4] Narrow optional indirect references in clone test for mypy Co-Authored-By: Claude Opus 5.5 Signed-off-by: Ak4shh --- tests/generic/test_data_structures.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/generic/test_data_structures.py b/tests/generic/test_data_structures.py index fc4ab41c31..000a3c3d2c 100644 --- a/tests/generic/test_data_structures.py +++ b/tests/generic/test_data_structures.py @@ -56,9 +56,12 @@ def test_dictionary_object__clone_linked_objects_with_constructor_arguments(link assert cloned_second is not second assert cloned.title == first.title assert cloned_second.items() == second.items() + assert cloned.indirect_reference is not None assert cloned.indirect_reference.pdf is destination + assert cloned_second.indirect_reference is not None assert cloned_second.indirect_reference.pdf is destination assert first[link] is second + assert second.indirect_reference is not None assert second.indirect_reference.pdf is source From bc3d4708da75c9e2ed650d4afe1b7dab379118ef Mon Sep 17 00:00:00 2001 From: Ak4shh Date: Mon, 2 Nov 2026 19:07:22 -0500 Subject: [PATCH 4/4] TST: Remove obsolete annotation cloning xfail marker Signed-off-by: Ak4shh --- tests/test_page.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_page.py b/tests/test_page.py index 048400882b..6abcb5a37d 100644 --- a/tests/test_page.py +++ b/tests/test_page.py @@ -2048,7 +2048,8 @@ def test_merge_page__target_with_null_annotations() -> None: # ISO 32000-2:2020, Table 182: the four vertices in counterclockwise order. quad_points = ArrayObject(FloatObject(value) for value in (10, 10, 20, 10, 20, 20, 10, 20)) source.add_annotation(0, Highlight(rect=(10, 10, 20, 20), quad_points=quad_points)) - # Write and read the source, so the annotation is a plain dictionary and #3467 does not interfere. + # Write and read the source, so the annotation is a plain dictionary + # rather than the Highlight instance. source_data = BytesIO() source.write(source_data) writer = PdfWriter() @@ -2060,16 +2061,13 @@ def test_merge_page__target_with_null_annotations() -> None: assert len(target["/Annots"]) == 1 -@pytest.mark.xfail( - reason="#3467: Cloning annotations created with pypdf.annotations fails", strict=True -) def test_merge_page__annotation_added_by_writer() -> None: """ merge_page() copies an annotation that was added with PdfWriter.add_annotation() and not yet written. - The writer keeps the Highlight object itself, whose constructor requires - arguments, so cloning it with self.__class__() fails. + The writer keeps the Highlight object itself, so it is cloned directly + rather than read back as a plain dictionary first. """ source = PdfWriter() source.add_blank_page(100, 100)