Merge pull request #5565 from radarhere/xml

Replaced xml.etree.ElementTree

.ci/install.sh

@@ -24,6 +24,7 @@ sudo apt-get -qq install libfreetype6-dev liblcms2-dev python3-tk\
 python3 -m pip install --upgrade pip
 PYTHONOPTIMIZE=0 python3 -m pip install cffi
 python3 -m pip install coverage
+python3 -m pip install defusedxml
 python3 -m pip install olefile
 python3 -m pip install -U pytest
 python3 -m pip install -U pytest-cov

.github/workflows/macos-install.sh

@@ -6,6 +6,7 @@ brew install libtiff libjpeg openjpeg libimagequant webp little-cms2 freetype op
 
 PYTHONOPTIMIZE=0 python3 -m pip install cffi
 python3 -m pip install coverage
+python3 -m pip install defusedxml
 python3 -m pip install olefile
 python3 -m pip install -U pytest
 python3 -m pip install -U pytest-cov

.github/workflows/test-windows.yml

@@ -51,8 +51,8 @@ jobs:
     - name: Print build system information
       run: python .github/workflows/system-info.py
 
-    - name: python -m pip install wheel pytest pytest-cov pytest-timeout
-      run: python -m pip install wheel pytest pytest-cov pytest-timeout
+    - name: python -m pip install wheel pytest pytest-cov pytest-timeout defusedxml
+      run: python -m pip install wheel pytest pytest-cov pytest-timeout defusedxml
 
     - name: Install dependencies
       id: install

Tests/test_file_jpeg.py

@@ -28,6 +28,11 @@ from .helper import (
     skip_unless_feature,
 )
 
+try:
+    import defusedxml.ElementTree as ElementTree
+except ImportError:
+    ElementTree = None
+
 TEST_FILE = "Tests/images/hopper.jpg"
 
 
@@ -825,26 +830,29 @@ class TestFileJpeg:
 
     def test_getxmp(self):
         with Image.open("Tests/images/xmp_test.jpg") as im:
-            xmp = im.getxmp()
+            if ElementTree is None:
+                with pytest.warns(UserWarning):
+                    assert im.getxmp() == {}
+            else:
+                xmp = im.getxmp()
 
-            assert isinstance(xmp, dict)
+                description = xmp["xmpmeta"]["RDF"]["Description"]
+                assert description["DerivedFrom"] == {
+                    "documentID": "8367D410E636EA95B7DE7EBA1C43A412",
+                    "originalDocumentID": "8367D410E636EA95B7DE7EBA1C43A412",
+                }
+                assert description["Look"]["Description"]["Group"]["Alt"]["li"] == {
+                    "lang": "x-default",
+                    "text": "Profiles",
+                }
+                assert description["ToneCurve"]["Seq"]["li"] == ["0, 0", "255, 255"]
 
-            description = xmp["xmpmeta"]["RDF"]["Description"]
-            assert description["DerivedFrom"] == {
-                "documentID": "8367D410E636EA95B7DE7EBA1C43A412",
-                "originalDocumentID": "8367D410E636EA95B7DE7EBA1C43A412",
-            }
-            assert description["Look"]["Description"]["Group"]["Alt"]["li"] == {
-                "lang": "x-default",
-                "text": "Profiles",
-            }
-            assert description["ToneCurve"]["Seq"]["li"] == ["0, 0", "255, 255"]
+                # Attribute
+                assert description["Version"] == "10.4"
 
-            # Attribute
-            assert description["Version"] == "10.4"
-
-        with Image.open("Tests/images/hopper.jpg") as im:
-            assert im.getxmp() == {}
+        if ElementTree is not None:
+            with Image.open("Tests/images/hopper.jpg") as im:
+                assert im.getxmp() == {}
 
 
 @pytest.mark.skipif(not is_win32(), reason="Windows only")

Tests/test_file_png.py

@@ -19,6 +19,11 @@ from .helper import (
     skip_unless_feature,
 )
 
+try:
+    import defusedxml.ElementTree as ElementTree
+except ImportError:
+    ElementTree = None
+
 # sample png stream
 
 TEST_PNG_FILE = "Tests/images/hopper.png"
@@ -651,15 +656,17 @@ class TestFilePng:
         with Image.open(out) as reloaded:
             assert len(reloaded.png.im_palette[1]) == 3
 
-    def test_xmp(self):
+    def test_getxmp(self):
         with Image.open("Tests/images/color_snakes.png") as im:
-            xmp = im.getxmp()
+            if ElementTree is None:
+                with pytest.warns(UserWarning):
+                    assert im.getxmp() == {}
+            else:
+                xmp = im.getxmp()
 
-            assert isinstance(xmp, dict)
-
-            description = xmp["xmpmeta"]["RDF"]["Description"]
-            assert description["PixelXDimension"] == "10"
-            assert description["subject"]["Seq"] is None
+                description = xmp["xmpmeta"]["RDF"]["Description"]
+                assert description["PixelXDimension"] == "10"
+                assert description["subject"]["Seq"] is None
 
     def test_exif(self):
         # With an EXIF chunk

Tests/test_file_tiff.py

@@ -16,6 +16,11 @@ from .helper import (
     is_win32,
 )
 
+try:
+    import defusedxml.ElementTree as ElementTree
+except ImportError:
+    ElementTree = None
+
 
 class TestFileTiff:
     def test_sanity(self, tmp_path):
@@ -643,15 +648,17 @@ class TestFileTiff:
         with Image.open(outfile) as reloaded:
             assert "icc_profile" not in reloaded.info
 
-    def test_xmp(self):
+    def test_getxmp(self):
         with Image.open("Tests/images/lab.tif") as im:
-            xmp = im.getxmp()
+            if ElementTree is None:
+                with pytest.warns(UserWarning):
+                    assert im.getxmp() == {}
+            else:
+                xmp = im.getxmp()
 
-            assert isinstance(xmp, dict)
-
-            description = xmp["xmpmeta"]["RDF"]["Description"]
-            assert description[0]["format"] == "image/tiff"
-            assert description[3]["BitsPerSample"]["Seq"]["li"] == ["8", "8", "8"]
+                description = xmp["xmpmeta"]["RDF"]["Description"]
+                assert description[0]["format"] == "image/tiff"
+                assert description[3]["BitsPerSample"]["Seq"]["li"] == ["8", "8", "8"]
 
     def test_close_on_load_exclusive(self, tmp_path):
         # similar to test_fd_leak, but runs on unixlike os

docs/releasenotes/8.3.0.rst

@@ -61,7 +61,17 @@ format, through the new ``bitmap_format`` argument::
 Security
 ========
 
-TODO
+Parsing XML
+^^^^^^^^^^^
+
+Pillow previously parsed XMP data using Python's ``xml`` module. However, this module
+is not secure.
+
+- :py:meth:`~PIL.Image.Image.getexif` has used ``xml`` to potentially retrieve
+  orientation data since Pillow 7.2.0. It has been refactored to use ``re`` instead.
+- :py:meth:`~PIL.JpegImagePlugin.JpegImageFile.getxmp` was added in Pillow 8.2.0. It
+  will now use ``defusedxml`` instead. If the dependency is not present, an empty
+  dictionary will be returned and a warning raised.
 
 Other Changes
 =============

requirements.txt

@@ -2,6 +2,7 @@
 black
 check-manifest
 coverage
+defusedxml
 markdown2
 olefile
 packaging

src/PIL/Image.py

@@ -31,14 +31,19 @@ import logging
 import math
 import numbers
 import os
+import re
 import struct
 import sys
 import tempfile
 import warnings
-import xml.etree.ElementTree
 from collections.abc import Callable, MutableMapping
 from pathlib import Path
 
+try:
+    import defusedxml.ElementTree as ElementTree
+except ImportError:
+    ElementTree = None
+
 # VERSION was removed in Pillow 6.0.0.
 # PILLOW_VERSION is deprecated and will be removed in a future release.
 # Use __version__ instead.
@@ -1358,8 +1363,12 @@ class Image:
                 return element.text
             return value
 
-        root = xml.etree.ElementTree.fromstring(xmp_tags)
-        return {get_name(root.tag): get_value(root)}
+        if ElementTree is None:
+            warnings.warn("XMP data cannot be read without defusedxml dependency")
+            return {}
+        else:
+            root = ElementTree.fromstring(xmp_tags)
+            return {get_name(root.tag): get_value(root)}
 
     def getexif(self):
         if self._exif is None:
@@ -1381,15 +1390,9 @@ class Image:
         if 0x0112 not in self._exif:
             xmp_tags = self.info.get("XML:com.adobe.xmp")
             if xmp_tags:
-                xmp = self._getxmp(xmp_tags)
-                if (
-                    "xmpmeta" in xmp
-                    and "RDF" in xmp["xmpmeta"]
-                    and "Description" in xmp["xmpmeta"]["RDF"]
-                ):
-                    description = xmp["xmpmeta"]["RDF"]["Description"]
-                    if "Orientation" in description:
-                        self._exif[0x0112] = int(description["Orientation"])
+                match = re.search(r'tiff:Orientation="([0-9])"', xmp_tags)
+                if match:
+                    self._exif[0x0112] = int(match[1])
 
         return self._exif
 

src/PIL/JpegImagePlugin.py

@@ -480,6 +480,7 @@ class JpegImageFile(ImageFile.ImageFile):
     def getxmp(self):
         """
         Returns a dictionary containing the XMP tags.
+        Requires defusedxml to be installed.
         :returns: XMP tags in a dictionary.
         """
 

src/PIL/PngImagePlugin.py

@@ -981,6 +981,7 @@ class PngImageFile(ImageFile.ImageFile):
     def getxmp(self):
         """
         Returns a dictionary containing the XMP tags.
+        Requires defusedxml to be installed.
         :returns: XMP tags in a dictionary.
         """
         return (

src/PIL/TiffImagePlugin.py

@@ -1112,6 +1112,7 @@ class TiffImageFile(ImageFile.ImageFile):
     def getxmp(self):
         """
         Returns a dictionary containing the XMP tags.
+        Requires defusedxml to be installed.
         :returns: XMP tags in a dictionary.
         """
         return self._getxmp(self.tag_v2[700]) if 700 in self.tag_v2 else {}