From 78c62727d3de1dcf9ae443e30e38b1ef30edf08c Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 30 Sep 2017 13:32:43 +1000 Subject: [PATCH 1/3] Moved seek frame position check into ImageFile --- PIL/DcxImagePlugin.py | 5 +++-- PIL/FliImagePlugin.py | 4 +--- PIL/GifImagePlugin.py | 4 ++-- PIL/ImImagePlugin.py | 7 +------ PIL/ImageFile.py | 10 ++++++++++ PIL/MicImagePlugin.py | 5 +++-- PIL/MpoImagePlugin.py | 15 +++++++-------- PIL/PsdImagePlugin.py | 7 ++++--- PIL/SpiderImagePlugin.py | 4 ++-- PIL/TiffImagePlugin.py | 8 +++++--- Tests/test_file_tiff.py | 3 ++- 11 files changed, 40 insertions(+), 32 deletions(-) diff --git a/PIL/DcxImagePlugin.py b/PIL/DcxImagePlugin.py index 797b8a5a82a..0d920ad3c37 100644 --- a/PIL/DcxImagePlugin.py +++ b/PIL/DcxImagePlugin.py @@ -59,6 +59,7 @@ def _open(self): self._offset.append(offset) self.__fp = self.fp + self.frame = None self.seek(0) @property @@ -70,8 +71,8 @@ def is_animated(self): return len(self._offset) > 1 def seek(self, frame): - if frame >= len(self._offset): - raise EOFError("attempt to seek outside DCX directory") + if not self._seek_check(frame): + return self.frame = frame self.fp = self.__fp self.fp.seek(self._offset[frame]) diff --git a/PIL/FliImagePlugin.py b/PIL/FliImagePlugin.py index 733d2d8f193..2c190b6354f 100644 --- a/PIL/FliImagePlugin.py +++ b/PIL/FliImagePlugin.py @@ -118,12 +118,10 @@ def is_animated(self): return self.__framecount > 1 def seek(self, frame): - if frame == self.__frame: + if not self._seek_check(frame): return if frame < self.__frame: self._seek(0) - if frame >= self.__framecount: - raise EOFError("no more images in FLI file") for f in range(self.__frame + 1, frame + 1): self._seek(f) diff --git a/PIL/GifImagePlugin.py b/PIL/GifImagePlugin.py index fc118e60778..64764f99d50 100644 --- a/PIL/GifImagePlugin.py +++ b/PIL/GifImagePlugin.py @@ -48,7 +48,7 @@ class GifImageFile(ImageFile.ImageFile): format = "GIF" format_description = "Compuserve GIF" _close_exclusive_fp_after_loading = False - + global_palette = None def data(self): @@ -117,7 +117,7 @@ def is_animated(self): return self._is_animated def seek(self, frame): - if frame == self.__frame: + if not self._seek_check(frame): return if frame < self.__frame: self._seek(0) diff --git a/PIL/ImImagePlugin.py b/PIL/ImImagePlugin.py index 3371b303f02..3a1fdfbfcf0 100644 --- a/PIL/ImImagePlugin.py +++ b/PIL/ImImagePlugin.py @@ -270,11 +270,7 @@ def is_animated(self): return self.info[FRAMES] > 1 def seek(self, frame): - - if frame < 0 or frame >= self.info[FRAMES]: - raise EOFError("seek outside sequence") - - if self.frame == frame: + if not self._seek_check(frame): return self.frame = frame @@ -292,7 +288,6 @@ def seek(self, frame): self.tile = [("raw", (0, 0)+self.size, offs, (self.rawmode, 0, -1))] def tell(self): - return self.frame # diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 5e1719fdc47..051a61e6f05 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -275,6 +275,16 @@ def load_end(self): # def load_read(self, bytes): # pass + def _seek_check(self, frame): + if (frame < 0 or + # Only check upper limit on frames if additional seek operations + # are not required to do so + (not (hasattr(self, "_n_frames") and self._n_frames is None) and + frame >= self.n_frames)): + raise EOFError("attempt to seek outside sequence") + + return self.tell() != frame + class StubImageFile(ImageFile): """ diff --git a/PIL/MicImagePlugin.py b/PIL/MicImagePlugin.py index 1b7972061f2..1dbb6a588e1 100644 --- a/PIL/MicImagePlugin.py +++ b/PIL/MicImagePlugin.py @@ -65,7 +65,7 @@ def _open(self): raise SyntaxError("not an MIC file; no image entries") self.__fp = self.fp - self.frame = 0 + self.frame = None if len(self.images) > 1: self.category = Image.CONTAINER @@ -81,7 +81,8 @@ def is_animated(self): return len(self.images) > 1 def seek(self, frame): - + if not self._seek_check(frame): + return try: filename = self.images[frame] except IndexError: diff --git a/PIL/MpoImagePlugin.py b/PIL/MpoImagePlugin.py index b12307f006f..8ad27b919c6 100644 --- a/PIL/MpoImagePlugin.py +++ b/PIL/MpoImagePlugin.py @@ -72,14 +72,13 @@ def is_animated(self): return self.__framecount > 1 def seek(self, frame): - if frame < 0 or frame >= self.__framecount: - raise EOFError("no more images in MPO file") - else: - self.fp = self.__fp - self.offset = self.__mpoffsets[frame] - self.tile = [ - ("jpeg", (0, 0) + self.size, self.offset, (self.mode, "")) - ] + if not self._seek_check(frame): + return + self.fp = self.__fp + self.offset = self.__mpoffsets[frame] + self.tile = [ + ("jpeg", (0, 0) + self.size, self.offset, (self.mode, "")) + ] self.__frame = frame def tell(self): diff --git a/PIL/PsdImagePlugin.py b/PIL/PsdImagePlugin.py index 1e4051c295b..8603162e473 100644 --- a/PIL/PsdImagePlugin.py +++ b/PIL/PsdImagePlugin.py @@ -135,11 +135,12 @@ def is_animated(self): return len(self.layers) > 1 def seek(self, layer): - # seek to given layer (1..max) - if layer == self.frame: + if not self._seek_check(layer): return + + # seek to given layer (1..max) try: - if layer <= 0: + if layer == 0: raise IndexError name, mode, bbox, tile = self.layers[layer-1] self.mode = mode diff --git a/PIL/SpiderImagePlugin.py b/PIL/SpiderImagePlugin.py index a821fcf87eb..5e5dde5a675 100644 --- a/PIL/SpiderImagePlugin.py +++ b/PIL/SpiderImagePlugin.py @@ -174,8 +174,8 @@ def tell(self): def seek(self, frame): if self.istack == 0: raise EOFError("attempt to seek in a non-stack file") - if frame >= self._nimages: - raise EOFError("attempt to seek past end of file") + if not self._seek_check(frame): + return self.stkoffset = self.hdrlen + frame * (self.hdrlen + self.imgbytes) self.fp = self.__fp self.fp.seek(self.stkoffset) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index 81bb374ad21..07fb97d141e 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -555,8 +555,8 @@ def _setitem(self, tag, value, legacy_api): # Spec'd length == 1, Actual > 1, Warn and truncate. Formerly barfed. # No Spec, Actual length 1, Formerly (<4.2) returned a 1 element tuple. # Don't mess with the legacy api, since it's frozen. - if ((info.length == 1) or - (info.length is None and len(values) == 1 and not legacy_api)): + if ((info.length == 1) or + (info.length is None and len(values) == 1 and not legacy_api)): # Don't mess with the legacy api, since it's frozen. if legacy_api and self.tagtype[tag] in [5, 10]: # rationals values = values, @@ -980,7 +980,9 @@ def is_animated(self): def seek(self, frame): "Select a given frame as current image" - self._seek(max(frame, 0)) # Questionable backwards compatibility. + if not self._seek_check(frame): + return + self._seek(frame) # Create a new core image object on second and # subsequent frames in the image. Image may be # different size/mode. diff --git a/Tests/test_file_tiff.py b/Tests/test_file_tiff.py index 758d76fd3ca..f8402b47cc8 100644 --- a/Tests/test_file_tiff.py +++ b/Tests/test_file_tiff.py @@ -349,13 +349,14 @@ def test_load_double(self): def test_seek(self): filename = "Tests/images/pil136.tiff" im = Image.open(filename) - im.seek(-1) + im.seek(0) self.assertEqual(im.tell(), 0) def test_seek_eof(self): filename = "Tests/images/pil136.tiff" im = Image.open(filename) self.assertEqual(im.tell(), 0) + self.assertRaises(EOFError, im.seek, -1) self.assertRaises(EOFError, im.seek, 1) def test__limit_rational_int(self): From f61b70aa8f4156fe192fed9f704927caa3878d69 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 6 Sep 2017 13:19:33 +1000 Subject: [PATCH 2/3] Changed EOFError tests --- Tests/test_file_dcx.py | 15 +++++++-------- Tests/test_file_fli.py | 26 ++++++++++---------------- Tests/test_file_gif.py | 15 +++++++-------- Tests/test_file_im.py | 15 +++++++-------- Tests/test_file_mpo.py | 15 +++++++-------- Tests/test_file_tiff.py | 15 +++++++-------- 6 files changed, 45 insertions(+), 56 deletions(-) diff --git a/Tests/test_file_dcx.py b/Tests/test_file_dcx.py index 5e73fd10fed..28ebb91dc9e 100644 --- a/Tests/test_file_dcx.py +++ b/Tests/test_file_dcx.py @@ -42,15 +42,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open(TEST_FILE) - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_seek_too_far(self): # Arrange diff --git a/Tests/test_file_fli.py b/Tests/test_file_fli.py index 09dd8d8a7ca..142af3cec40 100644 --- a/Tests/test_file_fli.py +++ b/Tests/test_file_fli.py @@ -54,24 +54,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open(animated_test_file) - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) - - def test_seek_outside(self): - # Test negative seek - im = Image.open(static_test_file) - im.seek(-1) - self.assertEqual(im.tell(), 0) - # Test seek past end of file - self.assertRaises(EOFError, im.seek, 2) + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_seek_tell(self): im = Image.open(animated_test_file) @@ -91,6 +81,10 @@ def test_seek_tell(self): layer_number = im.tell() self.assertEqual(layer_number, 2) + im.seek(1) + layer_number = im.tell() + self.assertEqual(layer_number, 1) + if __name__ == '__main__': unittest.main() diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py index f5d244edfd6..0b52d7b449a 100644 --- a/Tests/test_file_gif.py +++ b/Tests/test_file_gif.py @@ -223,15 +223,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open(TEST_GIF) - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_dispose_none(self): img = Image.open("Tests/images/dispose_none.gif") diff --git a/Tests/test_file_im.py b/Tests/test_file_im.py index e4219703496..c9992476774 100644 --- a/Tests/test_file_im.py +++ b/Tests/test_file_im.py @@ -32,15 +32,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open(TEST_IM) - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_roundtrip(self): for mode in ["RGB", "P"]: diff --git a/Tests/test_file_mpo.py b/Tests/test_file_mpo.py index b4d0c696e3a..70bb9b10544 100644 --- a/Tests/test_file_mpo.py +++ b/Tests/test_file_mpo.py @@ -102,15 +102,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open("Tests/images/sugarshack.mpo") - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_image_grab(self): for test_file in test_files: diff --git a/Tests/test_file_tiff.py b/Tests/test_file_tiff.py index f8402b47cc8..3ad8b2bd100 100644 --- a/Tests/test_file_tiff.py +++ b/Tests/test_file_tiff.py @@ -250,15 +250,14 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open('Tests/images/multipage-lastframe.tif') - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - im.seek(n_frames) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_multipage(self): # issue #862 From c8b65f4efdaa850466255ff2b8d2001ec00394ed Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 6 Sep 2017 13:23:50 +1000 Subject: [PATCH 3/3] Added _min_frame property --- PIL/ImageFile.py | 6 ++++-- PIL/PsdImagePlugin.py | 5 ++--- Tests/test_file_psd.py | 23 ++++++++++------------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 051a61e6f05..b0d13a0fc55 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -78,6 +78,8 @@ class ImageFile(Image.Image): def __init__(self, fp=None, filename=None): Image.Image.__init__(self) + self._min_frame = 0 + self.tile = None self.readonly = 1 # until we know better @@ -276,11 +278,11 @@ def load_end(self): # pass def _seek_check(self, frame): - if (frame < 0 or + if (frame < self._min_frame or # Only check upper limit on frames if additional seek operations # are not required to do so (not (hasattr(self, "_n_frames") and self._n_frames is None) and - frame >= self.n_frames)): + frame >= self.n_frames+self._min_frame)): raise EOFError("attempt to seek outside sequence") return self.tell() != frame diff --git a/PIL/PsdImagePlugin.py b/PIL/PsdImagePlugin.py index 8603162e473..ee803271743 100644 --- a/PIL/PsdImagePlugin.py +++ b/PIL/PsdImagePlugin.py @@ -124,7 +124,8 @@ def _open(self): # keep the file open self._fp = self.fp - self.frame = 0 + self.frame = 1 + self._min_frame = 1 @property def n_frames(self): @@ -140,8 +141,6 @@ def seek(self, layer): # seek to given layer (1..max) try: - if layer == 0: - raise IndexError name, mode, bbox, tile = self.layers[layer-1] self.mode = mode self.tile = tile diff --git a/Tests/test_file_psd.py b/Tests/test_file_psd.py index 99bd0ea803d..afc69694d77 100644 --- a/Tests/test_file_psd.py +++ b/Tests/test_file_psd.py @@ -34,26 +34,23 @@ def test_n_frames(self): def test_eoferror(self): im = Image.open(test_file) + # PSD seek index starts at 1 rather than 0 + n_frames = im.n_frames+1 - n_frames = im.n_frames - while True: - n_frames -= 1 - try: - # PSD seek index starts at 1 rather than 0 - im.seek(n_frames+1) - break - except EOFError: - self.assertLess(im.tell(), n_frames) + # Test seeking past the last frame + self.assertRaises(EOFError, im.seek, n_frames) + self.assertLess(im.tell(), n_frames) + + # Test that seeking to the last frame does not raise an error + im.seek(n_frames-1) def test_seek_tell(self): im = Image.open(test_file) layer_number = im.tell() - self.assertEqual(layer_number, 0) + self.assertEqual(layer_number, 1) - im.seek(0) - layer_number = im.tell() - self.assertEqual(layer_number, 0) + self.assertRaises(EOFError, im.seek, 0) im.seek(1) layer_number = im.tell()