BUG: Fix XMP handling dropping indirect references (#3392)

Closes #3391.

According to table 29 of the PDF 2.0 specification, the Metadata stream
inside the catalog dictionary should be an indirect reference. The old
code would inline the content stream into the catalog dictionary, making
the file unreadable in some applications like some versions of Adobe
Acrobat.

Author: stefan6419846

pypdf/_writer.py

@@ -361,10 +361,23 @@ def xmp_metadata(self, value: Optional[XmpInformation]) -> None:
         if value is None:
             if "/Metadata" in self.root_object:
                 del self.root_object["/Metadata"]
+            return
+
+        metadata = self.root_object.get("/Metadata", None)
+        if not isinstance(metadata, IndirectObject):
+            if metadata is not None:
+                del self.root_object["/Metadata"]
+            metadata_stream = StreamObject()
+            stream_reference = self._add_object(metadata_stream)
+            self.root_object[NameObject("/Metadata")] = stream_reference
         else:
-            self.root_object[NameObject("/Metadata")] = value
+            metadata_stream = cast(StreamObject, metadata.get_object())
 
-        return self.root_object.xmp_metadata  # type: ignore
+        if isinstance(value, XmpInformation):
+            bytes_data = value.stream.get_data()
+        else:
+            bytes_data = value
+        metadata_stream.set_data(bytes_data)
 
     @property
     def with_as_usage(self) -> bool:

pypdf/generic/_data_structures.py

@@ -479,7 +479,7 @@ def __getitem__(self, key: Any) -> PdfObject:
     @property
     def xmp_metadata(self) -> Optional[XmpInformationProtocol]:
         """
-        Retrieve XMP (Extensible Metadata Platform) data relevant to the this
+        Retrieve XMP (Extensible Metadata Platform) data relevant to this
         object, if available.
 
         See Table 347 — Additional entries in a metadata stream dictionary.
@@ -497,11 +497,7 @@ def xmp_metadata(self) -> Optional[XmpInformationProtocol]:
             return None
         assert metadata is not None, "mypy"
         metadata = metadata.get_object()
-
-        if not isinstance(metadata, XmpInformation):
-            metadata = XmpInformation(metadata)
-            self[NameObject("/Metadata")] = metadata
-        return metadata
+        return XmpInformation(metadata)
 
     def write_to_stream(
         self, stream: StreamType, encryption_key: Union[None, str, bytes] = None

pypdf/xmp.py

@@ -22,7 +22,7 @@
 from xml.parsers.expat import ExpatError
 
 from ._protocols import XmpInformationProtocol
-from ._utils import StreamType, deprecate_no_replacement
+from ._utils import StreamType, deprecate_no_replacement, deprecate_with_replacement
 from .errors import PdfReadError
 from .generic import ContentStream, PdfObject
 
@@ -247,6 +247,11 @@ def __init__(self, stream: ContentStream) -> None:
     def write_to_stream(
         self, stream: StreamType, encryption_key: Union[None, str, bytes] = None
     ) -> None:
+        deprecate_with_replacement(
+            "XmpInformation.write_to_stream",
+            "PdfWriter.xmp_metadata",
+            "6.0.0"
+        )
         if encryption_key is not None:  # deprecated
             deprecate_no_replacement(
                 "the encryption_key parameter of write_to_stream", "5.0.0"

tests/test_xmp.py

@@ -9,6 +9,8 @@
 import pypdf.xmp
 from pypdf import PdfReader, PdfWriter
 from pypdf.errors import PdfReadError
+from pypdf.generic import NameObject, StreamObject
+from pypdf.xmp import XmpInformation
 
 from . import get_data_from_url
 
@@ -325,3 +327,91 @@ def test_dc_language__no_bag_container():
 
     assert reader.xmp_metadata is not None
     assert reader.xmp_metadata.dc_language == ["x-unknown"]
+
+
+def test_reading_does_not_destroy_root_object():
+    """Test for #3391."""
+    writer = PdfWriter(clone_from=RESOURCE_ROOT / "commented-xmp.pdf")
+    xmp = writer.xmp_metadata
+    assert xmp is not None
+    assert not isinstance(writer.root_object["/Metadata"], XmpInformation)
+    assert isinstance(writer.root_object["/Metadata"].get_object(), StreamObject)
+
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    assert b"\n/Metadata 27 0 R\n" in output_bytes
+
+
+def test_xmp_information__write_to_stream():
+    writer = PdfWriter(clone_from=RESOURCE_ROOT / "commented-xmp.pdf")
+    xmp = writer.xmp_metadata
+
+    output = BytesIO()
+    with pytest.warns(
+            DeprecationWarning,
+            match=(
+                r"^XmpInformation\.write_to_stream is deprecated and will be removed in pypdf 6\.0\.0\. "
+                r"Use PdfWriter\.xmp_metadata instead\.$"
+            )
+    ):
+        xmp.write_to_stream(output)
+    output_bytes = output.getvalue()
+    assert output_bytes.startswith(b"<<\n/Type /Metadata\n/Subtype /XML\n/Length 2786\n>>\nstream\n<?xpacket begin")
+
+
+def test_pdf_writer__xmp_metadata_setter():
+    # Clear existing metadata.
+    writer = PdfWriter(clone_from=RESOURCE_ROOT / "commented-xmp.pdf")
+    assert writer.xmp_metadata is not None
+    original_metadata = writer.xmp_metadata.stream.get_data()
+    writer.xmp_metadata = None
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    reader = PdfReader(BytesIO(output_bytes))
+    assert reader.xmp_metadata is None
+
+    # Attempt to clear again.
+    writer = PdfWriter(clone_from=reader)
+    assert writer.xmp_metadata is None
+    writer.xmp_metadata = None
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    reader = PdfReader(BytesIO(output_bytes))
+    assert reader.xmp_metadata is None
+
+    # Set new metadata from bytes.
+    writer = PdfWriter(clone_from=reader)
+    assert writer.xmp_metadata is None
+    writer.xmp_metadata = original_metadata
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    reader = PdfReader(BytesIO(output_bytes))
+    assert get_all_tiff(reader.xmp_metadata) == {"tiff:Artist": ["me"]}
+
+    # Set metadata from XmpInformation.
+    writer = PdfWriter(clone_from=reader)
+    xmp_metadata = writer.xmp_metadata
+    assert get_all_tiff(xmp_metadata) == {"tiff:Artist": ["me"]}
+    new_metadata = original_metadata.replace(b"<tiff:Artist>me</tiff:Artist>", b"<tiff:Artist>Foo Bar</tiff:Artist>")
+    xmp_metadata.stream.set_data(new_metadata)
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    reader = PdfReader(BytesIO(output_bytes))
+    assert get_all_tiff(reader.xmp_metadata) == {"tiff:Artist": ["Foo Bar"]}
+
+    # Fix metadata not being an IndirectObject before.
+    writer = PdfWriter(clone_from=RESOURCE_ROOT / "commented-xmp.pdf")
+    writer.root_object[NameObject("/Metadata")] = writer.root_object["/Metadata"].get_object()
+    assert "/XML" in str(writer.root_object)
+    writer.xmp_metadata = new_metadata
+    output = BytesIO()
+    writer.write(output)
+    output_bytes = output.getvalue()
+    reader = PdfReader(BytesIO(output_bytes))
+    assert get_all_tiff(reader.xmp_metadata) == {"tiff:Artist": ["Foo Bar"]}
+    assert "/XML" not in str(writer.root_object)