From f8912a73e07ba5f7821ca1d61284fc28f4c879fa Mon Sep 17 00:00:00 2001 From: homm Date: Wed, 27 Jul 2016 14:41:04 +0300 Subject: [PATCH 1/4] code style --- libImaging/PcxEncode.c | 47 +++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/libImaging/PcxEncode.c b/libImaging/PcxEncode.c index c73f49a24..b17a09184 100644 --- a/libImaging/PcxEncode.c +++ b/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]; @@ -103,22 +103,23 @@ 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 */ @@ -126,15 +127,17 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) 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; } } @@ -142,8 +145,7 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) state->LAST = this; state->count = 1; - state->x++; - + state->x += 1; } } @@ -152,15 +154,17 @@ 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; } } @@ -168,15 +172,16 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) return ptr - buf; } /* add the padding */ - for (i=0;ix < planes * bytes_per_line) { state->count = 1; state->LAST = state->buffer[state->x]; - state->x++; + state->x += 1; } } /* read next line */ From 978c37d699f24d5bdcadf7617e8313433ca0dfdb Mon Sep 17 00:00:00 2001 From: homm Date: Thu, 28 Jul 2016 05:29:24 +0300 Subject: [PATCH 2/4] add tests for different PCX encoding cases --- Tests/test_file_pcx.py | 80 +++++++++++++++++++++++++++++++++++++++++- libImaging/PcxEncode.c | 3 -- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/Tests/test_file_pcx.py b/Tests/test_file_pcx.py index db2a526a5..6b15b2fb6 100644 --- a/Tests/test_file_pcx.py +++ b/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 @@ class TestFilePcx(PillowTestCase): # 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_overflow(self, im): + _last = ImageFile.MAXBLOCK + ImageFile.MAXBLOCK = 1024 + 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_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_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_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_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_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_overflow(im) + if __name__ == '__main__': unittest.main() diff --git a/libImaging/PcxEncode.c b/libImaging/PcxEncode.c index b17a09184..1dd5836e2 100644 --- a/libImaging/PcxEncode.c +++ b/libImaging/PcxEncode.c @@ -168,9 +168,6 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) bytes -= 2; } } - if (bytes < padding) { - return ptr - buf; - } /* add the padding */ for (i = 0; i < padding; i++) { ptr[0] = 0; From 87b20389d85344f67db875b18886e6586048edbd Mon Sep 17 00:00:00 2001 From: homm Date: Thu, 28 Jul 2016 05:30:27 +0300 Subject: [PATCH 3/4] fix any errors --- libImaging/PcxEncode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libImaging/PcxEncode.c b/libImaging/PcxEncode.c index 1dd5836e2..163b09b13 100644 --- a/libImaging/PcxEncode.c +++ b/libImaging/PcxEncode.c @@ -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 @@ -180,7 +180,8 @@ ImagingPcxEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) state->LAST = state->buffer[state->x]; state->x += 1; } - } + } while (state->x < planes * bytes_per_line); + /* read next line */ state->state = FETCH; break; From 467f6cfcbb649a555455753c6719b8197ab29683 Mon Sep 17 00:00:00 2001 From: homm Date: Fri, 29 Jul 2016 12:47:36 +0300 Subject: [PATCH 4/4] rename test --- Tests/test_file_pcx.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Tests/test_file_pcx.py b/Tests/test_file_pcx.py index 6b15b2fb6..7621c1cc6 100644 --- a/Tests/test_file_pcx.py +++ b/Tests/test_file_pcx.py @@ -60,9 +60,9 @@ class TestFilePcx(PillowTestCase): px[x, 0] = x // 67 * 67 self._roundtrip(im) - def _test_overflow(self, im): + def _test_buffer_overflow(self, im, size=1024): _last = ImageFile.MAXBLOCK - ImageFile.MAXBLOCK = 1024 + ImageFile.MAXBLOCK = size try: self._roundtrip(im) finally: @@ -74,7 +74,7 @@ class TestFilePcx(PillowTestCase): for y in range(4): for x in range(256): px[x, y] = x % 128 - self._test_overflow(im) + self._test_buffer_overflow(im) def test_break_one_in_loop(self): im = Image.new('L', (256, 5)) @@ -82,7 +82,7 @@ class TestFilePcx(PillowTestCase): for y in range(5): for x in range(256): px[x, y] = x % 128 - self._test_overflow(im) + self._test_buffer_overflow(im) def test_break_many_in_loop(self): im = Image.new('L', (256, 5)) @@ -92,7 +92,7 @@ class TestFilePcx(PillowTestCase): px[x, y] = x % 128 for x in range(8): px[x, 4] = 16 - self._test_overflow(im) + self._test_buffer_overflow(im) def test_break_one_at_end(self): im = Image.new('L', (256, 5)) @@ -101,7 +101,7 @@ class TestFilePcx(PillowTestCase): for x in range(256): px[x, y] = x % 128 px[0, 3] = 128 + 64 - self._test_overflow(im) + self._test_buffer_overflow(im) def test_break_many_at_end(self): im = Image.new('L', (256, 5)) @@ -112,7 +112,7 @@ class TestFilePcx(PillowTestCase): for x in range(4): px[x * 2, 3] = 128 + 64 px[x + 256 - 4, 3] = 0 - self._test_overflow(im) + self._test_buffer_overflow(im) def test_break_padding(self): im = Image.new('L', (257, 5)) @@ -122,7 +122,7 @@ class TestFilePcx(PillowTestCase): px[x, y] = x % 128 for x in range(5): px[x, 3] = 0 - self._test_overflow(im) + self._test_buffer_overflow(im) if __name__ == '__main__':