From 1b626f4d224ea3ab820a41e892f281c35d6d907f Mon Sep 17 00:00:00 2001 From: Konstantin Kopachev Date: Fri, 19 Jul 2019 14:12:16 -0700 Subject: [PATCH 1/6] Fix RATIONAL and SRATIONAL boundaries when writing IFDs --- Tests/README.rst | 2 +- Tests/test_file_tiff_metadata.py | 70 ++++++++++++++++++++++++++++---- src/PIL/TiffImagePlugin.py | 26 +++++++++--- 3 files changed, 83 insertions(+), 15 deletions(-) diff --git a/Tests/README.rst b/Tests/README.rst index d64a34bd7..da3297bce 100644 --- a/Tests/README.rst +++ b/Tests/README.rst @@ -4,7 +4,7 @@ Pillow Tests Test scripts are named ``test_xxx.py`` and use the ``unittest`` module. A base class and helper functions can be found in ``helper.py``. Dependencies ------------ +------------ Install:: diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index 8761e431e..8ee0aa64f 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/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 @@ -136,14 +136,6 @@ class TestFileTiffMetadata(PillowTestCase): 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,66 @@ 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) + + # 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_expty_values(self): data = io.BytesIO( b"II*\x00\x08\x00\x00\x00\x03\x00\x1a\x01\x05\x00\x00\x00\x00\x00" diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index dabcf8eb4..5cab83a2e 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/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,12 @@ 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): - self.tagtype[tag] = TiffTags.SHORT + self.tagtype[tag] = TiffTags.SHORT if all(v >= 0 for v in values) else 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 +721,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) @@ -728,7 +744,7 @@ 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): From 524933aa1da010d28d5a86db54ee4990d0e41e9e Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 12 Aug 2019 19:42:32 +1000 Subject: [PATCH 2/6] Lint fixes --- src/PIL/TiffImagePlugin.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index 5cab83a2e..71dc4fe74 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -569,12 +569,24 @@ class ImageFileDirectory_v2(MutableMapping): else: self.tagtype[tag] = TiffTags.UNDEFINED if all(isinstance(v, IFDRational) for v in values): - self.tagtype[tag] = TiffTags.RATIONAL if all(v >= 0 for v in values) else TiffTags.SIGNED_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): - self.tagtype[tag] = TiffTags.SHORT if all(v >= 0 for v in values) else TiffTags.SIGNED_SHORT + self.tagtype[tag] = ( + TiffTags.SHORT + if all(v >= 0 for v in values) + else TiffTags.SIGNED_SHORT + ) else: - self.tagtype[tag] = TiffTags.LONG if all(v >= 0 for v in values) else TiffTags.SIGNED_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: @@ -744,7 +756,8 @@ class ImageFileDirectory_v2(MutableMapping): @_register_writer(10) def write_signed_rational(self, *values): return b"".join( - self._pack("2l", *_limit_signed_rational(frac, 2 ** 31 - 1, -2 ** 31)) 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): From ddcfd259cf2619e32c9a27a66e8dddca3fa346a2 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Thu, 29 Aug 2019 19:36:46 +1000 Subject: [PATCH 3/6] Corrected short and long range checks --- Tests/test_file_tiff_metadata.py | 12 ++++++++++++ src/PIL/TiffImagePlugin.py | 10 ++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index 8ee0aa64f..535eadf1b 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/Tests/test_file_tiff_metadata.py @@ -274,6 +274,18 @@ class TestFileTiffMetadata(PillowTestCase): 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_expty_values(self): data = io.BytesIO( b"II*\x00\x08\x00\x00\x00\x03\x00\x1a\x01\x05\x00\x00\x00\x00\x00" diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index 71dc4fe74..2a3e68b34 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -575,12 +575,10 @@ class ImageFileDirectory_v2(MutableMapping): else TiffTags.SIGNED_RATIONAL ) elif all(isinstance(v, int) for v in values): - if all(v < 2 ** 16 for v in values): - self.tagtype[tag] = ( - TiffTags.SHORT - if all(v >= 0 for v in values) - else TiffTags.SIGNED_SHORT - ) + 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 From 74d53bcd15303bc0cba6506a5c69d17f1a2d74cd Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 31 Dec 2019 11:58:39 +1100 Subject: [PATCH 4/6] Lint fixes --- Tests/test_file_tiff_metadata.py | 4 ++-- src/PIL/TiffImagePlugin.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index 035a4fcc1..edfeeb465 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/Tests/test_file_tiff_metadata.py @@ -253,7 +253,7 @@ class TestFileTiffMetadata(PillowTestCase): # pair of 4 byte signed longs numerator = 2 ** 31 - 1 - denominator = -2 ** 31 + denominator = -(2 ** 31) info[37380] = TiffImagePlugin.IFDRational(numerator, denominator) @@ -265,7 +265,7 @@ class TestFileTiffMetadata(PillowTestCase): self.assertEqual(denominator, reloaded.tag_v2[37380].denominator) # pair of 4 byte signed longs - numerator = -2 ** 31 + numerator = -(2 ** 31) denominator = 2 ** 31 - 1 info[37380] = TiffImagePlugin.IFDRational(numerator, denominator) diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index f8993310e..e6a5fc63a 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -544,7 +544,7 @@ class ImageFileDirectory_v2(MutableMapping): elif all(isinstance(v, int) 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): + elif all(-(2 ** 15) < v < 2 ** 15 for v in values): self.tagtype[tag] = TiffTags.SIGNED_SHORT else: self.tagtype[tag] = ( @@ -715,7 +715,7 @@ class ImageFileDirectory_v2(MutableMapping): @_register_writer(10) def write_signed_rational(self, *values): return b"".join( - self._pack("2l", *_limit_signed_rational(frac, 2 ** 31 - 1, -2 ** 31)) + self._pack("2l", *_limit_signed_rational(frac, 2 ** 31 - 1, -(2 ** 31))) for frac in values ) From 69fc0413994f76a3ed87c76de414d9893e64dfb2 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 31 Dec 2019 19:11:03 +1100 Subject: [PATCH 5/6] Test out of bounds signed rational --- Tests/test_file_tiff_metadata.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index edfeeb465..4b8034cb2 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/Tests/test_file_tiff_metadata.py @@ -264,7 +264,6 @@ class TestFileTiffMetadata(PillowTestCase): 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 @@ -277,6 +276,19 @@ class TestFileTiffMetadata(PillowTestCase): 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, 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() From 8924054dd9ff1a13caea785bfd66210193de8a74 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 31 Dec 2019 19:12:33 +1100 Subject: [PATCH 6/6] Changed _limit_signed_rational --- Tests/test_file_tiff_metadata.py | 6 +++--- src/PIL/TiffImagePlugin.py | 20 +++++++++----------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Tests/test_file_tiff_metadata.py b/Tests/test_file_tiff_metadata.py index 4b8034cb2..f5407d49d 100644 --- a/Tests/test_file_tiff_metadata.py +++ b/Tests/test_file_tiff_metadata.py @@ -277,7 +277,7 @@ class TestFileTiffMetadata(PillowTestCase): self.assertEqual(denominator, reloaded.tag_v2[37380].denominator) # out of bounds of 4 byte signed long - numerator = -2 ** 31 - 1 + numerator = -(2 ** 31) - 1 denominator = 1 info[37380] = TiffImagePlugin.IFDRational(numerator, denominator) @@ -286,8 +286,8 @@ class TestFileTiffMetadata(PillowTestCase): im.save(out, tiffinfo=info, compression="raw") reloaded = Image.open(out) - self.assertEqual(- 2 ** 31, reloaded.tag_v2[37380].numerator) - self.assertEqual(1, reloaded.tag_v2[37380].denominator) + 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() diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index e6a5fc63a..74fb69516 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -263,20 +263,18 @@ 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) +def _limit_signed_rational(val, max_val, min_val): + frac = Fraction(val) + n_d = frac.numerator, frac.denominator - max_abs = max(max_val, abs(min_val)) + if min(n_d) < min_val: + n_d = _limit_rational(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) + if max(n_d) > max_val: + val = Fraction(*n_d) + n_d = _limit_rational(val, max_val) - return num, denom + return n_d ##