BUG: Fix cloning of objects that require constructor arguments

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 = []
@@ -415,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

tests/generic/test_data_structures.py

@@ -16,9 +16,11 @@
     ContentStream,
     Destination,
     DictionaryObject,
+    Fit,
     NameObject,
     NullObject,
     NumberObject,
+    OutlineItem,
     RectangleObject,
     StreamObject,
     StreamParsingBudget,
@@ -35,6 +37,34 @@
     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 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
+
+
 def test_dictionary_object__get_next_object_position() -> None:
     reader = PdfReader(RESOURCE_ROOT / "crazyones.pdf")
 

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

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"),
     [
@@ -2022,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()
@@ -2034,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)