Merge pull request #3980 from kkopachev/exif-writing-fixes

Exif writing fixes: Rational boundaries and signed/unsigned types

Tests/test_file_tiff_metadata.py

@@ -2,7 +2,7 @@ import io
 import struct
 
 from PIL import Image, TiffImagePlugin, TiffTags
-from PIL.TiffImagePlugin import IFDRational, _limit_rational
+from PIL.TiffImagePlugin import IFDRational
 
 from .helper import PillowTestCase, hopper
 
@@ -139,14 +139,6 @@ class TestFileTiffMetadata(PillowTestCase):
         with Image.open(f) as loaded:
             reloaded = loaded.tag_v2.named()
 
-        for k, v in original.items():
-            if isinstance(v, IFDRational):
-                original[k] = IFDRational(*_limit_rational(v, 2 ** 31))
-            elif isinstance(v, tuple) and isinstance(v[0], IFDRational):
-                original[k] = tuple(
-                    IFDRational(*_limit_rational(elt, 2 ** 31)) for elt in v
-                )
-
         ignored = ["StripByteCounts", "RowsPerStrip", "PageNumber", "StripOffsets"]
 
         for tag, value in reloaded.items():
@@ -225,6 +217,90 @@ class TestFileTiffMetadata(PillowTestCase):
             self.assertEqual(0, reloaded.tag_v2[41988].numerator)
             self.assertEqual(0, reloaded.tag_v2[41988].denominator)
 
+    def test_ifd_unsigned_rational(self):
+        im = hopper()
+        info = TiffImagePlugin.ImageFileDirectory_v2()
+
+        max_long = 2 ** 32 - 1
+
+        # 4 bytes unsigned long
+        numerator = max_long
+
+        info[41493] = TiffImagePlugin.IFDRational(numerator, 1)
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(max_long, reloaded.tag_v2[41493].numerator)
+        self.assertEqual(1, reloaded.tag_v2[41493].denominator)
+
+        # out of bounds of 4 byte unsigned long
+        numerator = max_long + 1
+
+        info[41493] = TiffImagePlugin.IFDRational(numerator, 1)
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(max_long, reloaded.tag_v2[41493].numerator)
+        self.assertEqual(1, reloaded.tag_v2[41493].denominator)
+
+    def test_ifd_signed_rational(self):
+        im = hopper()
+        info = TiffImagePlugin.ImageFileDirectory_v2()
+
+        # pair of 4 byte signed longs
+        numerator = 2 ** 31 - 1
+        denominator = -(2 ** 31)
+
+        info[37380] = TiffImagePlugin.IFDRational(numerator, denominator)
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(numerator, reloaded.tag_v2[37380].numerator)
+        self.assertEqual(denominator, reloaded.tag_v2[37380].denominator)
+
+        numerator = -(2 ** 31)
+        denominator = 2 ** 31 - 1
+
+        info[37380] = TiffImagePlugin.IFDRational(numerator, denominator)
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(numerator, reloaded.tag_v2[37380].numerator)
+        self.assertEqual(denominator, reloaded.tag_v2[37380].denominator)
+
+        # out of bounds of 4 byte signed long
+        numerator = -(2 ** 31) - 1
+        denominator = 1
+
+        info[37380] = TiffImagePlugin.IFDRational(numerator, denominator)
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(2 ** 31 - 1, reloaded.tag_v2[37380].numerator)
+        self.assertEqual(-1, reloaded.tag_v2[37380].denominator)
+
+    def test_ifd_signed_long(self):
+        im = hopper()
+        info = TiffImagePlugin.ImageFileDirectory_v2()
+
+        info[37000] = -60000
+
+        out = self.tempfile("temp.tiff")
+        im.save(out, tiffinfo=info, compression="raw")
+
+        reloaded = Image.open(out)
+        self.assertEqual(reloaded.tag_v2[37000], -60000)
+
     def test_empty_values(self):
         data = io.BytesIO(
             b"II*\x00\x08\x00\x00\x00\x03\x00\x1a\x01\x05\x00\x00\x00\x00\x00"

src/PIL/TiffImagePlugin.py

@@ -263,6 +263,20 @@ def _limit_rational(val, max_val):
     return n_d[::-1] if inv else n_d
 
 
+def _limit_signed_rational(val, max_val, min_val):
+    frac = Fraction(val)
+    n_d = frac.numerator, frac.denominator
+
+    if min(n_d) < min_val:
+        n_d = _limit_rational(val, abs(min_val))
+
+    if max(n_d) > max_val:
+        val = Fraction(*n_d)
+        n_d = _limit_rational(val, max_val)
+
+    return n_d
+
+
 ##
 # Wrapper for TIFF IFDs.
 
@@ -520,12 +534,22 @@ class ImageFileDirectory_v2(MutableMapping):
             else:
                 self.tagtype[tag] = TiffTags.UNDEFINED
                 if all(isinstance(v, IFDRational) for v in values):
-                    self.tagtype[tag] = TiffTags.RATIONAL
+                    self.tagtype[tag] = (
+                        TiffTags.RATIONAL
+                        if all(v >= 0 for v in values)
+                        else TiffTags.SIGNED_RATIONAL
+                    )
                 elif all(isinstance(v, int) for v in values):
-                    if all(v < 2 ** 16 for v in values):
+                    if all(0 <= v < 2 ** 16 for v in values):
                         self.tagtype[tag] = TiffTags.SHORT
+                    elif all(-(2 ** 15) < v < 2 ** 15 for v in values):
+                        self.tagtype[tag] = TiffTags.SIGNED_SHORT
                     else:
-                        self.tagtype[tag] = TiffTags.LONG
+                        self.tagtype[tag] = (
+                            TiffTags.LONG
+                            if all(v >= 0 for v in values)
+                            else TiffTags.SIGNED_LONG
+                        )
                 elif all(isinstance(v, float) for v in values):
                     self.tagtype[tag] = TiffTags.DOUBLE
                 else:
@@ -666,7 +690,7 @@ class ImageFileDirectory_v2(MutableMapping):
     @_register_writer(5)
     def write_rational(self, *values):
         return b"".join(
-            self._pack("2L", *_limit_rational(frac, 2 ** 31)) for frac in values
+            self._pack("2L", *_limit_rational(frac, 2 ** 32 - 1)) for frac in values
         )
 
     @_register_loader(7, 1)
@@ -689,7 +713,8 @@ class ImageFileDirectory_v2(MutableMapping):
     @_register_writer(10)
     def write_signed_rational(self, *values):
         return b"".join(
-            self._pack("2L", *_limit_rational(frac, 2 ** 30)) for frac in values
+            self._pack("2l", *_limit_signed_rational(frac, 2 ** 31 - 1, -(2 ** 31)))
+            for frac in values
         )
 
     def _ensure_read(self, fp, size):