From 4bb78d53a3c2d1e275508a4a458bda9f767e714e Mon Sep 17 00:00:00 2001 From: nulano Date: Wed, 19 Aug 2020 23:43:43 +0200 Subject: [PATCH 1/3] give proper error message for invalid putpixel color type --- Tests/test_image_access.py | 30 ++++++++++++++++++++++++++++++ src/_imaging.c | 24 +++++++++++++----------- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/Tests/test_image_access.py b/Tests/test_image_access.py index 4c0dbb4cc..17cb615a5 100644 --- a/Tests/test_image_access.py +++ b/Tests/test_image_access.py @@ -328,6 +328,36 @@ class TestCffi(AccessTest): assert im.convert("RGB").getpixel((0, 0)) == (255, 0, 0) +class TestImagePutPixelError(AccessTest): + def test_putpixel_error_message(self): + for mode, reason, accept_tuple in [ + ("L", "color must be int or tuple", True), + ("LA", "color must be int or tuple", True), + ("RGB", "color must be int or tuple", True), + ("RGBA", "color must be int or tuple", True), + ("I", "color must be int", False), + ("I;16", "color must be int", False), + ("BGR;15", "color must be int", False), + ]: + im = hopper(mode) + + for v in ["foo", 1.0, None]: + with pytest.raises(TypeError, match=reason): + im.putpixel((0, 0), v) + + if not accept_tuple: + with pytest.raises(TypeError, match=reason): + im.putpixel((0, 0), (10,)) + + with pytest.raises(OverflowError): + im.putpixel((0, 0), 2 ** 80) + + for mode in ["BGR;15"]: + im = hopper(mode) + with pytest.raises(ValueError, match="unrecognized image mode"): + im.putpixel((0, 0), 0) + + class TestEmbeddable: @pytest.mark.skipif( not is_win32() or on_ci(), diff --git a/src/_imaging.c b/src/_imaging.c index 1ed5e8a42..d3159a494 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -521,12 +521,20 @@ getink(PyObject* color, Imaging im, char* ink) if (im->type == IMAGING_TYPE_UINT8 || im->type == IMAGING_TYPE_INT32 || im->type == IMAGING_TYPE_SPECIAL) { - if (PyLong_Check(color)) { - r = PyLong_AsLongLong(color); + if (PyLong_Check(color)) { + r = PyLong_AsLongLong(color); + if (r == -1 && PyErr_Occurred()) { + return NULL; + } rIsInt = 1; - } - if (r == -1 && PyErr_Occurred()) { - rIsInt = 0; + } else if (im->type == IMAGING_TYPE_UINT8) { + if (!PyTuple_Check(color)) { + PyErr_SetString(PyExc_TypeError, "color must be int or tuple"); + return NULL; + } + } else { + PyErr_SetString(PyExc_TypeError, "color must be int"); + return NULL; } } @@ -570,9 +578,6 @@ getink(PyObject* color, Imaging im, char* ink) return ink; case IMAGING_TYPE_INT32: /* signed integer */ - if (rIsInt != 1) { - return NULL; - } itmp = r; memcpy(ink, &itmp, sizeof(itmp)); return ink; @@ -587,9 +592,6 @@ getink(PyObject* color, Imaging im, char* ink) return ink; case IMAGING_TYPE_SPECIAL: if (strncmp(im->mode, "I;16", 4) == 0) { - if (rIsInt != 1) { - return NULL; - } ink[0] = (UINT8) r; ink[1] = (UINT8) (r >> 8); ink[2] = ink[3] = 0; From 76a51270fa94ab1bf7f4b9e4212ad2e339023c2b Mon Sep 17 00:00:00 2001 From: nulano Date: Thu, 20 Aug 2020 23:16:19 +0200 Subject: [PATCH 2/3] split up test --- Tests/test_image_access.py | 49 ++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/Tests/test_image_access.py b/Tests/test_image_access.py index 17cb615a5..a4050b4d8 100644 --- a/Tests/test_image_access.py +++ b/Tests/test_image_access.py @@ -329,33 +329,36 @@ class TestCffi(AccessTest): class TestImagePutPixelError(AccessTest): - def test_putpixel_error_message(self): - for mode, reason, accept_tuple in [ - ("L", "color must be int or tuple", True), - ("LA", "color must be int or tuple", True), - ("RGB", "color must be int or tuple", True), - ("RGBA", "color must be int or tuple", True), - ("I", "color must be int", False), - ("I;16", "color must be int", False), - ("BGR;15", "color must be int", False), - ]: - im = hopper(mode) + IMAGE_MODES1 = ["L", "LA", "RGB", "RGBA"] + INVALID_TYPES1 = ["foo", 1.0, None] - for v in ["foo", 1.0, None]: - with pytest.raises(TypeError, match=reason): - im.putpixel((0, 0), v) + @pytest.mark.parametrize("mode", IMAGE_MODES1) + def test_putpixel_type_error1(self, mode): + im = hopper(mode) + for v in self.INVALID_TYPES1: + with pytest.raises(TypeError, match="color must be int or tuple"): + im.putpixel((0, 0), v) - if not accept_tuple: - with pytest.raises(TypeError, match=reason): - im.putpixel((0, 0), (10,)) + IMAGE_MODES2 = ["I", "I;16", "BGR;15"] + INVALID_TYPES2 = [*INVALID_TYPES1, (10,)] - with pytest.raises(OverflowError): - im.putpixel((0, 0), 2 ** 80) + @pytest.mark.parametrize("mode", IMAGE_MODES2) + def test_putpixel_type_error2(self, mode): + im = hopper(mode) + for v in self.INVALID_TYPES2: + with pytest.raises(TypeError, match="color must be int"): + im.putpixel((0, 0), v) - for mode in ["BGR;15"]: - im = hopper(mode) - with pytest.raises(ValueError, match="unrecognized image mode"): - im.putpixel((0, 0), 0) + @pytest.mark.parametrize("mode", IMAGE_MODES1 + IMAGE_MODES2) + def test_putpixel_overflow_error(self, mode): + im = hopper(mode) + with pytest.raises(OverflowError): + im.putpixel((0, 0), 2 ** 80) + + def test_putpixel_unrecognized_mode(self): + im = hopper("BGR;15") + with pytest.raises(ValueError, match="unrecognized image mode"): + im.putpixel((0, 0), 0) class TestEmbeddable: From 4b2a0173d39a4e982c88fc1a43e5961260ff7731 Mon Sep 17 00:00:00 2001 From: nulano Date: Fri, 21 Aug 2020 13:05:36 +0100 Subject: [PATCH 3/3] group test cases Co-authored-by: Hugo van Kemenade --- Tests/test_image_access.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Tests/test_image_access.py b/Tests/test_image_access.py index a4050b4d8..6d336fb3c 100644 --- a/Tests/test_image_access.py +++ b/Tests/test_image_access.py @@ -330,7 +330,9 @@ class TestCffi(AccessTest): class TestImagePutPixelError(AccessTest): IMAGE_MODES1 = ["L", "LA", "RGB", "RGBA"] + IMAGE_MODES2 = ["I", "I;16", "BGR;15"] INVALID_TYPES1 = ["foo", 1.0, None] + INVALID_TYPES2 = [*INVALID_TYPES1, (10,)] @pytest.mark.parametrize("mode", IMAGE_MODES1) def test_putpixel_type_error1(self, mode): @@ -339,9 +341,6 @@ class TestImagePutPixelError(AccessTest): with pytest.raises(TypeError, match="color must be int or tuple"): im.putpixel((0, 0), v) - IMAGE_MODES2 = ["I", "I;16", "BGR;15"] - INVALID_TYPES2 = [*INVALID_TYPES1, (10,)] - @pytest.mark.parametrize("mode", IMAGE_MODES2) def test_putpixel_type_error2(self, mode): im = hopper(mode)