Merge 467f6cf into 37841db

Tests/test_file_pcx.py

@@ -1,6 +1,6 @@
 from helper import unittest, PillowTestCase, hopper
 
-from PIL import Image, PcxImagePlugin
+from PIL import Image, ImageFile, PcxImagePlugin
 
 
 class TestFilePcx(PillowTestCase):
@@ -46,6 +46,84 @@ def test_pil184(self):
         # Make sure all pixels are either 0 or 255.
         self.assertEqual(im.histogram()[0] + im.histogram()[255], 447*144)
 
+    def test_1px_width(self):
+        im = Image.new('L', (1, 256))
+        px = im.load()
+        for y in range(256):
+            px[0, y] = y
+        self._roundtrip(im)
+
+    def test_large_count(self):
+        im = Image.new('L', (256, 1))
+        px = im.load()
+        for x in range(256):
+            px[x, 0] = x // 67 * 67
+        self._roundtrip(im)
+
+    def _test_buffer_overflow(self, im, size=1024):
+        _last = ImageFile.MAXBLOCK
+        ImageFile.MAXBLOCK = size
+        try:
+            self._roundtrip(im)
+        finally:
+            ImageFile.MAXBLOCK = _last
+
+    def test_break_in_count_overflow(self):
+        im = Image.new('L', (256, 5))
+        px = im.load()
+        for y in range(4):
+            for x in range(256):
+                px[x, y] = x % 128
+        self._test_buffer_overflow(im)
+
+    def test_break_one_in_loop(self):
+        im = Image.new('L', (256, 5))
+        px = im.load()
+        for y in range(5):
+            for x in range(256):
+                px[x, y] = x % 128
+        self._test_buffer_overflow(im)
+
+    def test_break_many_in_loop(self):
+        im = Image.new('L', (256, 5))
+        px = im.load()
+        for y in range(4):
+            for x in range(256):
+                px[x, y] = x % 128
+        for x in range(8):
+            px[x, 4] = 16
+        self._test_buffer_overflow(im)
+
+    def test_break_one_at_end(self):
+        im = Image.new('L', (256, 5))
+        px = im.load()
+        for y in range(5):
+            for x in range(256):
+                px[x, y] = x % 128
+        px[0, 3] = 128 + 64
+        self._test_buffer_overflow(im)
+
+    def test_break_many_at_end(self):
+        im = Image.new('L', (256, 5))
+        px = im.load()
+        for y in range(5):
+            for x in range(256):
+                px[x, y] = x % 128
+        for x in range(4):
+            px[x * 2, 3] = 128 + 64
+            px[x + 256 - 4, 3] = 0
+        self._test_buffer_overflow(im)
+
+    def test_break_padding(self):
+        im = Image.new('L', (257, 5))
+        px = im.load()
+        for y in range(5):
+            for x in range(257):
+                px[x, y] = x % 128
+        for x in range(5):
+            px[x, 3] = 0
+        self._test_buffer_overflow(im)
+
 
 if __name__ == '__main__':
     unittest.main()

libImaging/PcxEncode.c

@@ -75,7 +75,7 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
                            (UINT8*) im->image[state->y + state->yoff] +
                            state->xoff * im->pixelsize, state->xsize);
 
-            state->y++;
+            state->y += 1;
 
             state->count = 1;
             state->LAST = state->buffer[0];
@@ -91,7 +91,7 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
             /* when we arrive here, "count" contains the number of
                bytes having the value of "LAST" that we've already
                seen */
-            while (state->x < planes * bytes_per_line) {
+            do {
                 /* If we're encoding an odd width file, and we've
                    got more than one plane, we need to pad each
                    color row with padding bytes at the end. Since
@@ -103,47 +103,49 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
 
                     if (state->count == 63) {
                         /* this run is full; flush it */
-                        if (bytes < 2)
+                        if (bytes < 2) {
                             return ptr - buf;
-                        *ptr++ = 0xff;
-                        *ptr++ = state->LAST;
+                        }
+                        ptr[0] = 0xff;
+                        ptr[1] = state->LAST;
+                        ptr += 2;
                         bytes -= 2;
 
                         state->count = 0;
-
                     }
 
                     this = state->buffer[state->x];
 
                     if (this == state->LAST) {
                         /* extend the current run */
-                        state->x++;
-                        state->count++;
+                        state->x += 1;
+                        state->count += 1;
 
                     } else {
                         /* start a new run */
                         if (state->count == 1 && (state->LAST < 0xc0)) {
                             if (bytes < 1) {
                                 return ptr - buf;
                             }
-                            *ptr++ = state->LAST;
-                            bytes--;
+                            ptr[0] = state->LAST;
+                            ptr += 1;
+                            bytes -= 1;
                         } else {
                             if (state->count > 0) {
                                 if (bytes < 2) {
                                     return ptr - buf;
                                 }
-                                *ptr++ = 0xc0 | state->count;
-                                *ptr++ = state->LAST;
+                                ptr[0] = 0xc0 | state->count;
+                                ptr[1] = state->LAST;
+                                ptr += 2;
                                 bytes -= 2;
                             }
                         }
 
                         state->LAST = this;
                         state->count = 1;
 
-                        state->x++;
-
+                        state->x += 1;
                     }
                 }
 
@@ -152,33 +154,34 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
                     if (bytes < 1 + padding) {
                         return ptr - buf;
                     }
-                    *ptr++ = state->LAST;
-                    bytes--;
+                    ptr[0] = state->LAST;
+                    ptr += 1;
+                    bytes -= 1;
                 } else {
                     if (state->count > 0) {
                         if (bytes < 2 + padding) {
                             return ptr - buf;
                         }
-                        *ptr++ = 0xc0 | state->count;
-                        *ptr++ = state->LAST;
+                        ptr[0] = 0xc0 | state->count;
+                        ptr[1] = state->LAST;
+                        ptr += 2;
                         bytes -= 2;
                     }
                 }
-                if (bytes < padding) {
-                    return ptr - buf;
-                }
                 /* add the padding */
-                for (i=0;i<padding;i++){
-                    *ptr++=0;
-                    bytes--;
+                for (i = 0; i < padding; i++) {
+                    ptr[0] = 0;
+                    ptr += 1;
+                    bytes -= 1;
                 }
                 /* reset for the next color plane. */
                 if (state->x < planes * bytes_per_line) {
                     state->count = 1;
                     state->LAST = state->buffer[state->x];
-                    state->x++;
+                    state->x += 1;
                 }
-            }
+            } while (state->x < planes * bytes_per_line);
+
             /* read next line */
             state->state = FETCH;
             break;