From 21872ce1a1dce2796e80971cdbf838567d8fad73 Mon Sep 17 00:00:00 2001 From: andrewnester Date: Fri, 3 Mar 2017 15:38:01 +0300 Subject: [PATCH 1/5] bpo-29649: struct.pack_into check boundary error message didn't respect offset --- Lib/test/test_struct.py | 8 ++++++++ Misc/NEWS | 1 + Modules/_struct.c | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index cf1d56796636c8..21185dbb81655f 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -578,6 +578,14 @@ def test__sizeof__(self): self.check_sizeof('0s', 1) self.check_sizeof('0c', 0) + def test_boundary_error_message(self): + import ctypes + byte_list = ctypes.create_string_buffer(1) + try: + struct.pack_into('b', byte_list, 5, 1) + except struct.error as e: + self.assertEqual('pack_into requires a buffer of at least 6 bytes', str(e)) + class UnpackIteratorTest(unittest.TestCase): """ diff --git a/Misc/NEWS b/Misc/NEWS index 2e9cce6f12c63f..939caffb183edb 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -9,6 +9,7 @@ What's New in Python 3.7.0 alpha 1? Core and Builtins ----------------- +- bpo-29649: struct.pack_into check boundary error message didn't respect offset. - bpo-29949: Fix memory usage regression of set and frozenset object. diff --git a/Modules/_struct.c b/Modules/_struct.c index f66ee18bc49fd6..9d888dc3468e73 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1938,7 +1938,7 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames if (offset < 0 || (buffer.len - offset) < soself->s_size) { PyErr_Format(StructError, "pack_into requires a buffer of at least %zd bytes", - soself->s_size); + soself->s_size + (offset > 0 ? offset : 0)); PyBuffer_Release(&buffer); return NULL; } From e3fb912324850dc6c6fbfccd386fa1108636cb2f Mon Sep 17 00:00:00 2001 From: andrewnester Date: Fri, 3 Mar 2017 16:32:29 +0300 Subject: [PATCH 2/5] Code styles fixes --- Lib/test/test_struct.py | 10 +++++----- Misc/NEWS | 5 ++++- Modules/_struct.c | 7 +++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 21185dbb81655f..b39f9d98352894 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -579,12 +579,12 @@ def test__sizeof__(self): self.check_sizeof('0c', 0) def test_boundary_error_message(self): - import ctypes - byte_list = ctypes.create_string_buffer(1) - try: + byte_list = bytearray(1) + with self.assertRaisesRegex( + struct.error, + 'pack_into requires a buffer of at least 6 ' + 'bytes for packing 1 bytes at offset 5'): struct.pack_into('b', byte_list, 5, 1) - except struct.error as e: - self.assertEqual('pack_into requires a buffer of at least 6 bytes', str(e)) class UnpackIteratorTest(unittest.TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index 939caffb183edb..c3e4b0205a26ed 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -304,6 +304,9 @@ Extension Modules Library ------- +- bpo-29649: struct.pack_into() check boundary error message + didn't respect offset. Patch by Andrew Nester. + - bpo-29654: Support If-Modified-Since HTTP header (browser cache). Patch by Pierre Quentel. @@ -323,7 +326,7 @@ Library Now using them emits a deprecation warning. - bpo-27863: Fixed multiple crashes in ElementTree caused by race conditions - and wrong types. + and wrong types. - bpo-25996: Added support of file descriptors in os.scandir() on Unix. os.fwalk() is sped up by 2 times by using os.scandir(). diff --git a/Modules/_struct.c b/Modules/_struct.c index 9d888dc3468e73..1ed6d426acfc65 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1937,8 +1937,11 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames /* Check boundaries */ if (offset < 0 || (buffer.len - offset) < soself->s_size) { PyErr_Format(StructError, - "pack_into requires a buffer of at least %zd bytes", - soself->s_size + (offset > 0 ? offset : 0)); + "pack_into requires a buffer of at least %zd bytes for " + "packing %zd bytes at offset %zd)", + soself->s_size + (offset > 0 ? offset : 0), + soself->s_size, + offset); PyBuffer_Release(&buffer); return NULL; } From 11d21c0b85e5b11bded53b1ee1a7c0d058e478f8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Sun, 5 Mar 2017 19:31:25 +0300 Subject: [PATCH 3/5] Added new error messages for improper negative offset. --- Lib/test/test_struct.py | 16 +++++++++++++++- Misc/NEWS | 2 +- Modules/_struct.c | 35 ++++++++++++++++++++++++++++++----- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index b39f9d98352894..480d32943ac6b1 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -583,9 +583,23 @@ def test_boundary_error_message(self): with self.assertRaisesRegex( struct.error, 'pack_into requires a buffer of at least 6 ' - 'bytes for packing 1 bytes at offset 5'): + 'bytes for packing 1 bytes at offset 5 ' + '\(actual buffer size is 1\)'): struct.pack_into('b', byte_list, 5, 1) + def test_boundary_error_message_with_negative_offset(self): + byte_list = bytearray(10) + with self.assertRaisesRegex( + struct.error, + 'pack_into requires negative offset not higher than -4 ' + '\(actual offset is -2\)'): + struct.pack_into('s_size > 0) { + PyErr_Format(StructError, + "pack_into requires negative offset not higher " + "than %zd (actual offset is %zd)", + -soself->s_size, + offset); + PyBuffer_Release(&buffer); + return NULL; + } + + /* Check that negative offset is not crossing buffer boundary */ + if (offset + buffer.len < 0) { + PyErr_Format(StructError, + "pack_into requires negative offset not lower " + "than %zd (actual offset is %zd)", + -buffer.len, + offset); + PyBuffer_Release(&buffer); + return NULL; + } + offset += buffer.len; + } /* Check boundaries */ - if (offset < 0 || (buffer.len - offset) < soself->s_size) { + if ((buffer.len - offset) < soself->s_size) { PyErr_Format(StructError, "pack_into requires a buffer of at least %zd bytes for " - "packing %zd bytes at offset %zd)", - soself->s_size + (offset > 0 ? offset : 0), + "packing %zd bytes at offset %zd " + "(actual buffer size is %zd)", + soself->s_size + offset, soself->s_size, - offset); + offset, + buffer.len); PyBuffer_Release(&buffer); return NULL; } From 35b354f0158505e78ea496484634bf435f9d6d23 Mon Sep 17 00:00:00 2001 From: andrewnester Date: Sat, 11 Mar 2017 11:39:37 +0300 Subject: [PATCH 4/5] Changed error messages --- Lib/test/test_struct.py | 8 +++----- Misc/NEWS | 1 - Modules/_struct.c | 12 +++++------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 480d32943ac6b1..0f5d30ca44067f 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -584,21 +584,19 @@ def test_boundary_error_message(self): struct.error, 'pack_into requires a buffer of at least 6 ' 'bytes for packing 1 bytes at offset 5 ' - '\(actual buffer size is 1\)'): + r'\(actual buffer size is 1\)'): struct.pack_into('b', byte_list, 5, 1) def test_boundary_error_message_with_negative_offset(self): byte_list = bytearray(10) with self.assertRaisesRegex( struct.error, - 'pack_into requires negative offset not higher than -4 ' - '\(actual offset is -2\)'): + r'No space to pack 4 bytes at offset -2'): struct.pack_into('s_size > 0) { PyErr_Format(StructError, - "pack_into requires negative offset not higher " - "than %zd (actual offset is %zd)", - -soself->s_size, + "No space to pack %zd bytes at offset %zd", + soself->s_size, offset); PyBuffer_Release(&buffer); return NULL; @@ -1946,10 +1945,9 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames /* Check that negative offset is not crossing buffer boundary */ if (offset + buffer.len < 0) { PyErr_Format(StructError, - "pack_into requires negative offset not lower " - "than %zd (actual offset is %zd)", - -buffer.len, - offset); + "Offset %zd out of range for %zd-byte buffer", + offset, + buffer.len); PyBuffer_Release(&buffer); return NULL; } From 8870d3c2fa23e34fda333ecda81208ba9d964fcb Mon Sep 17 00:00:00 2001 From: andrewnester Date: Tue, 4 Apr 2017 12:13:09 +0300 Subject: [PATCH 5/5] Minor code style fixes --- Lib/test/test_struct.py | 18 +++++++++--------- Misc/NEWS | 6 +++--- Modules/_struct.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 0f5d30ca44067f..932ef4737852b9 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -579,24 +579,24 @@ def test__sizeof__(self): self.check_sizeof('0c', 0) def test_boundary_error_message(self): - byte_list = bytearray(1) - with self.assertRaisesRegex( - struct.error, - 'pack_into requires a buffer of at least 6 ' - 'bytes for packing 1 bytes at offset 5 ' - r'\(actual buffer size is 1\)'): - struct.pack_into('b', byte_list, 5, 1) + regex = ( + r'pack_into requires a buffer of at least 6 ' + r'bytes for packing 1 bytes at offset 5 ' + r'\(actual buffer size is 1\)' + ) + with self.assertRaisesRegex(struct.error, regex): + struct.pack_into('b', bytearray(1), 5, 1) def test_boundary_error_message_with_negative_offset(self): byte_list = bytearray(10) with self.assertRaisesRegex( struct.error, - r'No space to pack 4 bytes at offset -2'): + r'no space to pack 4 bytes at offset -2'): struct.pack_into('s_size > 0) { PyErr_Format(StructError, - "No space to pack %zd bytes at offset %zd", + "no space to pack %zd bytes at offset %zd", soself->s_size, offset); PyBuffer_Release(&buffer); @@ -1945,7 +1945,7 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames /* Check that negative offset is not crossing buffer boundary */ if (offset + buffer.len < 0) { PyErr_Format(StructError, - "Offset %zd out of range for %zd-byte buffer", + "offset %zd out of range for %zd-byte buffer", offset, buffer.len); PyBuffer_Release(&buffer);