ROB: Deal with invalid annotations in extract_links (#3659)

Closes #3656.

Author: stefan6419846

pypdf/generic/_link.py

@@ -30,7 +30,8 @@
 
 from typing import TYPE_CHECKING, Optional, Union, cast
 
-from . import ArrayObject, DictionaryObject, IndirectObject, PdfObject, TextStringObject
+from .._utils import logger_warning
+from . import ArrayObject, DictionaryObject, IndirectObject, PdfObject, TextStringObject, is_null_or_none
 
 if TYPE_CHECKING:
     from .._page import PageObject
@@ -79,8 +80,23 @@ def extract_links(new_page: "PageObject", old_page: "PageObject") -> list[tuple[
     """Extracts links from two pages on the assumption that the two pages are
     the same. Produces one list of (new link, old link) tuples.
     """
-    new_links = [_build_link(link, new_page) for link in new_page.get("/Annots", [])]
-    old_links = [_build_link(link, old_page) for link in old_page.get("/Annots", [])]
+    new_annotations = new_page.get("/Annots", ArrayObject()).get_object()
+    old_annotations = old_page.get("/Annots", ArrayObject()).get_object()
+    if is_null_or_none(new_annotations):
+        new_annotations = ArrayObject()
+    if is_null_or_none(old_annotations):
+        old_annotations = ArrayObject()
+    if not isinstance(new_annotations, ArrayObject) or not isinstance(old_annotations, ArrayObject):
+        logger_warning(
+            f"Expected annotation arrays: {old_annotations} {new_annotations}. Ignoring annotations.",
+            __name__
+        )
+        return []
+    if len(new_annotations) != len(old_annotations):
+        logger_warning(f"Annotation sizes differ: {old_annotations} vs. {new_annotations}", __name__)
+
+    new_links = [_build_link(link, new_page) for link in new_annotations]
+    old_links = [_build_link(link, old_page) for link in old_annotations]
 
     return [
         (new_link, old_link) for (new_link, old_link)
@@ -110,7 +126,7 @@ def _build_link(indirect_object: IndirectObject, page: "PageObject") -> Optional
     return None  # Nothing to do here
 
 
-def _create_link(reference: PdfObject, source_pdf: "PdfReader")-> Optional[ReferenceLink]:
+def _create_link(reference: PdfObject, source_pdf: "PdfReader") -> Optional[ReferenceLink]:
     if isinstance(reference, TextStringObject):
         return NamedReferenceLink(reference, source_pdf)
     if isinstance(reference, ArrayObject):

tests/generic/test_link.py

@@ -0,0 +1,55 @@
+"""Test the pypdf.generic._link module."""
+from io import BytesIO
+
+import pytest
+
+from pypdf import PageObject, PdfReader, PdfWriter
+from pypdf.generic import ArrayObject, NameObject, NullObject, extract_links
+from tests import get_data_from_url
+
+
+@pytest.mark.enable_socket
+def test_extract_links__null_object_in_old_page():
+    url = "https://github.com/user-attachments/files/25507697/sample.pdf"
+    name = "issue3656.pdf"
+    reader = PdfReader(BytesIO(get_data_from_url(url=url, name=name)))
+
+    writer = PdfWriter()
+    writer.append(reader)
+
+
+def test_extract_links(caplog):
+    page1 = PageObject()
+    page2 = PageObject()
+
+    # No annotations.
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == []
+
+    # Only old annotations.
+    page1[NameObject("/Annots")] = NullObject()
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == []
+    caplog.clear()
+
+    page1[NameObject("/Annots")] = ArrayObject([NullObject()])
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == ["Annotation sizes differ: [] vs. [NullObject]"]
+    caplog.clear()
+
+    # Both old and new annotations.
+    page2[NameObject("/Annots")] = ArrayObject([NullObject()])
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == []  # Same size.
+    caplog.clear()
+
+    page2[NameObject("/Annots")] = NullObject()
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == ["Annotation sizes differ: [] vs. [NullObject]"]
+    caplog.clear()
+
+    # Only new annotations.
+    del page1[NameObject("/Annots")]
+    page2[NameObject("/Annots")] = ArrayObject([NullObject()])
+    assert extract_links(page1, page2) == []
+    assert caplog.messages == ["Annotation sizes differ: [NullObject] vs. []"]

tests/test_writer.py

@@ -2828,6 +2828,14 @@ def test_insert_filtered_annotations__annotations_are_no_list(caplog):
     writer.append(reader)
     font_file2 = reader.get_object(36).indirect_reference
     assert caplog.messages == [
+        (
+            f"Expected annotation arrays: {{'/FontFile2': {font_file2!r}, "
+            "'/Descent': -269, '/CapHeight': 714, '/FontWeight': "
+            "300, '/FontName': '/JQJGLF+OpenSans-Light', '/ItalicAngle': 0, '/StemV': "
+            "48, '/Type': '/FontDescriptor', '/FontBBox': [-521, -269, 1140, 1048], "
+            "'/FontFamily': 'Open Sans Light', '/Flags': 32, '/XHeight': 531, "
+            "'/Ascent': 1048, '/FontStretch': '/Normal'} []. Ignoring annotations."
+        ),
         (
             f"Expected list of annotations, got {{'/FontFile2': {font_file2!r}, "
             "'/Descent': -269, '/CapHeight': 714, '/FontWeight': 300, '/FontName': '/JQJGLF+OpenSans-Light', "