Exif writing fixes: Rational boundaries and signed/unsigned types

Tests/test_file_tiff_metadata.py

@@ -2,7 +2,7 @@
 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
 
@@ -136,14 +136,6 @@ def test_write_metadata(self):
         original = img.tag_v2.named()
         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():
@@ -222,6 +214,78 @@ def test_exif_div_zero(self):
         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)
+
+        # pair of 4 byte signed longs
+        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)
+
+    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

@@ -279,6 +279,22 @@ def _limit_rational(val, max_val):
     return n_d[::-1] if inv else n_d
 
 
+def _limit_signed_rational(frac, max_val, min_val):
+    if frac >= 0:
+        return _limit_rational(frac, max_val)
+
+    max_abs = max(max_val, abs(min_val))
+
+    num, denom = _limit_rational(frac, max_abs)
+    if denom == max_abs or num == max_abs:
+        if (num < 0 or denom < 0) and num != denom:
+            num, denom = num * -1, denom * -1
+        else:
+            num, denom = _limit_rational(frac, max_val)
+
+    return num, denom
+
+
 def _libtiff_version():
     return Image.core.libtiff_version.split("\n")[0].split("Version ")[1]
 
@@ -553,12 +569,22 @@ def _setitem(self, tag, value, legacy_api):
             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:
@@ -705,7 +731,7 @@ def combine(a, b):
     @_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)
@@ -728,7 +754,8 @@ def combine(a, b):
     @_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):