From f2f92d22d180ddcecab5bf9c0a4c0151c04d0980 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 25 Jan 2021 21:10:49 +1100 Subject: [PATCH 1/4] Do not use "use built-in mapper WIN32 only" --- src/PIL/ImageFile.py | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/PIL/ImageFile.py b/src/PIL/ImageFile.py index f2a55cb54..f58de95bd 100644 --- a/src/PIL/ImageFile.py +++ b/src/PIL/ImageFile.py @@ -192,24 +192,14 @@ class ImageFile(Image.Image): and args[0] in Image._MAPMODES ): try: - if hasattr(Image.core, "map"): - # use built-in mapper WIN32 only - self.map = Image.core.map(self.filename) - self.map.seek(offset) - self.im = self.map.readimage( - self.mode, self.size, args[1], args[2] - ) - else: - # use mmap, if possible - import mmap + # use mmap, if possible + import mmap - with open(self.filename) as fp: - self.map = mmap.mmap( - fp.fileno(), 0, access=mmap.ACCESS_READ - ) - self.im = Image.core.map_buffer( - self.map, self.size, decoder_name, offset, args - ) + with open(self.filename) as fp: + self.map = mmap.mmap(fp.fileno(), 0, access=mmap.ACCESS_READ) + self.im = Image.core.map_buffer( + self.map, self.size, decoder_name, offset, args + ) readonly = 1 # After trashing self.im, # we might need to reload the palette data. From 685e95118250e764ff50e6ed29d5ed96fc873b4a Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 25 Jan 2021 21:13:07 +1100 Subject: [PATCH 2/4] Removed unused C code --- src/_imaging.c | 5 - src/map.c | 260 ------------------------------------------------- 2 files changed, 265 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index 01dd22486..a5b12d325 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -3973,8 +3973,6 @@ PyPath_Create(ImagingObject *self, PyObject *args); extern PyObject * PyOutline_Create(ImagingObject *self, PyObject *args); -extern PyObject * -PyImaging_Mapper(PyObject *self, PyObject *args); extern PyObject * PyImaging_MapBuffer(PyObject *self, PyObject *args); @@ -4030,9 +4028,6 @@ static PyMethodDef functions[] = { /* Memory mapping */ #ifdef WITH_MAPPING -#ifdef _WIN32 - {"map", (PyCFunction)PyImaging_Mapper, 1}, -#endif {"map_buffer", (PyCFunction)PyImaging_MapBuffer, 1}, #endif diff --git a/src/map.c b/src/map.c index 2636a684b..c298bd148 100644 --- a/src/map.c +++ b/src/map.c @@ -28,269 +28,9 @@ PyImaging_CheckBuffer(PyObject *buffer); extern int PyImaging_GetBuffer(PyObject *buffer, Py_buffer *view); -/* -------------------------------------------------------------------- */ -/* Standard mapper */ - -typedef struct { - PyObject_HEAD char *base; - int size; - int offset; -#ifdef _WIN32 - HANDLE hFile; - HANDLE hMap; -#endif -} ImagingMapperObject; - -static PyTypeObject ImagingMapperType; - -ImagingMapperObject * -PyImaging_MapperNew(const char *filename, int readonly) { - ImagingMapperObject *mapper; - - if (PyType_Ready(&ImagingMapperType) < 0) { - return NULL; - } - - mapper = PyObject_New(ImagingMapperObject, &ImagingMapperType); - if (mapper == NULL) { - return NULL; - } - - mapper->base = NULL; - mapper->size = mapper->offset = 0; - -#ifdef _WIN32 - mapper->hFile = (HANDLE)-1; - mapper->hMap = (HANDLE)-1; - - /* FIXME: currently supports readonly mappings only */ - mapper->hFile = CreateFile( - filename, - GENERIC_READ, - FILE_SHARE_READ, - NULL, - OPEN_EXISTING, - FILE_ATTRIBUTE_NORMAL, - NULL); - if (mapper->hFile == (HANDLE)-1) { - PyErr_SetString(PyExc_OSError, "cannot open file"); - Py_DECREF(mapper); - return NULL; - } - - mapper->hMap = CreateFileMapping(mapper->hFile, NULL, PAGE_READONLY, 0, 0, NULL); - if (mapper->hMap == (HANDLE)-1) { - CloseHandle(mapper->hFile); - PyErr_SetString(PyExc_OSError, "cannot map file"); - Py_DECREF(mapper); - return NULL; - } - - mapper->base = (char *)MapViewOfFile(mapper->hMap, FILE_MAP_READ, 0, 0, 0); - - mapper->size = GetFileSize(mapper->hFile, 0); -#endif - - return mapper; -} - -static void -mapping_dealloc(ImagingMapperObject *mapper) { -#ifdef _WIN32 - if (mapper->base != 0) { - UnmapViewOfFile(mapper->base); - } - if (mapper->hMap != (HANDLE)-1) { - CloseHandle(mapper->hMap); - } - if (mapper->hFile != (HANDLE)-1) { - CloseHandle(mapper->hFile); - } - mapper->base = 0; - mapper->hMap = mapper->hFile = (HANDLE)-1; -#endif - PyObject_Del(mapper); -} - -/* -------------------------------------------------------------------- */ -/* standard file operations */ - -static PyObject * -mapping_read(ImagingMapperObject *mapper, PyObject *args) { - PyObject *buf; - - int size = -1; - if (!PyArg_ParseTuple(args, "|i", &size)) { - return NULL; - } - - /* check size */ - if (size < 0 || mapper->offset + size > mapper->size) { - size = mapper->size - mapper->offset; - } - if (size < 0) { - size = 0; - } - - buf = PyBytes_FromStringAndSize(NULL, size); - if (!buf) { - return NULL; - } - - if (size > 0) { - memcpy(PyBytes_AsString(buf), mapper->base + mapper->offset, size); - mapper->offset += size; - } - - return buf; -} - -static PyObject * -mapping_seek(ImagingMapperObject *mapper, PyObject *args) { - int offset; - int whence = 0; - if (!PyArg_ParseTuple(args, "i|i", &offset, &whence)) { - return NULL; - } - - switch (whence) { - case 0: /* SEEK_SET */ - mapper->offset = offset; - break; - case 1: /* SEEK_CUR */ - mapper->offset += offset; - break; - case 2: /* SEEK_END */ - mapper->offset = mapper->size + offset; - break; - default: - /* FIXME: raise ValueError? */ - break; - } - - Py_INCREF(Py_None); - return Py_None; -} - -/* -------------------------------------------------------------------- */ -/* map entire image */ - extern PyObject * PyImagingNew(Imaging im); -static void -ImagingDestroyMap(Imaging im) { - return; /* nothing to do! */ -} - -static PyObject * -mapping_readimage(ImagingMapperObject *mapper, PyObject *args) { - int y, size; - Imaging im; - - char *mode; - int xsize; - int ysize; - int stride; - int orientation; - if (!PyArg_ParseTuple( - args, "s(ii)ii", &mode, &xsize, &ysize, &stride, &orientation)) { - return NULL; - } - - if (stride <= 0) { - /* FIXME: maybe we should call ImagingNewPrologue instead */ - if (!strcmp(mode, "L") || !strcmp(mode, "P")) { - stride = xsize; - } else if (!strcmp(mode, "I;16") || !strcmp(mode, "I;16B")) { - stride = xsize * 2; - } else { - stride = xsize * 4; - } - } - - size = ysize * stride; - - if (mapper->offset + size > mapper->size) { - PyErr_SetString(PyExc_OSError, "image file truncated"); - return NULL; - } - - im = ImagingNewPrologue(mode, xsize, ysize); - if (!im) { - return NULL; - } - - /* setup file pointers */ - if (orientation > 0) { - for (y = 0; y < ysize; y++) { - im->image[y] = mapper->base + mapper->offset + y * stride; - } - } else { - for (y = 0; y < ysize; y++) { - im->image[ysize - y - 1] = mapper->base + mapper->offset + y * stride; - } - } - - im->destroy = ImagingDestroyMap; - - mapper->offset += size; - - return PyImagingNew(im); -} - -static struct PyMethodDef methods[] = { - /* standard file interface */ - {"read", (PyCFunction)mapping_read, 1}, - {"seek", (PyCFunction)mapping_seek, 1}, - /* extensions */ - {"readimage", (PyCFunction)mapping_readimage, 1}, - {NULL, NULL} /* sentinel */ -}; - -static PyTypeObject ImagingMapperType = { - PyVarObject_HEAD_INIT(NULL, 0) "ImagingMapper", /*tp_name*/ - sizeof(ImagingMapperObject), /*tp_size*/ - 0, /*tp_itemsize*/ - /* methods */ - (destructor)mapping_dealloc, /*tp_dealloc*/ - 0, /*tp_print*/ - 0, /*tp_getattr*/ - 0, /*tp_setattr*/ - 0, /*tp_compare*/ - 0, /*tp_repr*/ - 0, /*tp_as_number */ - 0, /*tp_as_sequence */ - 0, /*tp_as_mapping */ - 0, /*tp_hash*/ - 0, /*tp_call*/ - 0, /*tp_str*/ - 0, /*tp_getattro*/ - 0, /*tp_setattro*/ - 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT, /*tp_flags*/ - 0, /*tp_doc*/ - 0, /*tp_traverse*/ - 0, /*tp_clear*/ - 0, /*tp_richcompare*/ - 0, /*tp_weaklistoffset*/ - 0, /*tp_iter*/ - 0, /*tp_iternext*/ - methods, /*tp_methods*/ - 0, /*tp_members*/ - 0, /*tp_getset*/ -}; - -PyObject * -PyImaging_Mapper(PyObject *self, PyObject *args) { - char *filename; - if (!PyArg_ParseTuple(args, "s", &filename)) { - return NULL; - } - - return (PyObject *)PyImaging_MapperNew(filename, 1); -} - /* -------------------------------------------------------------------- */ /* Buffer mapper */ From e4b9f88de4378ae622b54998b186e93e68fab4c7 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 30 Jan 2021 12:59:45 +1100 Subject: [PATCH 3/4] Updated test now that Win32 uses map_buffer --- Tests/test_map.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Tests/test_map.py b/Tests/test_map.py index 2b65fb3f9..9131e6b7d 100644 --- a/Tests/test_map.py +++ b/Tests/test_map.py @@ -4,10 +4,6 @@ import pytest from PIL import Image -from .helper import is_win32 - -pytestmark = pytest.mark.skipif(is_win32(), reason="Win32 does not call map_buffer") - def test_overflow(): # There is the potential to overflow comparisons in map.c From 11cb3fba9c93275d78f4339973560938fc07bd9e Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 30 Jan 2021 13:01:42 +1100 Subject: [PATCH 4/4] Added test --- Tests/test_map.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Tests/test_map.py b/Tests/test_map.py index 9131e6b7d..752c5f268 100644 --- a/Tests/test_map.py +++ b/Tests/test_map.py @@ -23,6 +23,13 @@ def test_overflow(): Image.MAX_IMAGE_PIXELS = max_pixels +def test_tobytes(): + # Previously raised an access violation on Windows + with Image.open("Tests/images/l2rgb_read.bmp") as im: + with pytest.raises((ValueError, MemoryError, OSError)): + im.tobytes() + + @pytest.mark.skipif(sys.maxsize <= 2 ** 32, reason="Requires 64-bit system") def test_ysize(): numpy = pytest.importorskip("numpy", reason="NumPy not installed")