bpo-29649: Improve struct.pack_into() boundary error messages (#424)

andrewnester · serhiy-storchaka · commit f78b119364b5 · 2017-04-04T13:46:25.000+03:00
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
@@ -578,6 +578,26 @@ def test__sizeof__(self):
         self.check_sizeof('0s', 1)
         self.check_sizeof('0c', 0)
 
+    def test_boundary_error_message(self):
+        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'):
+            struct.pack_into('<I', byte_list, -2, 123)
+
+        with self.assertRaisesRegex(
+                struct.error,
+                'offset -11 out of range for 10-byte buffer'):
+            struct.pack_into('<B', byte_list, -11, 123)
 
 class UnpackIteratorTest(unittest.TestCase):
     """
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -303,6 +303,9 @@ Extension Modules
 Library
 -------
 
+- bpo-29649: Improve struct.pack_into() exception messages for problems
+  with the buffer size and offset.  Patch by Andrew Nester.
+
 - bpo-29654: Support If-Modified-Since HTTP header (browser cache).  Patch
   by Pierre Quentel.
 
diff --git a/Modules/_struct.c b/Modules/_struct.c
@@ -1931,14 +1931,40 @@ s_pack_into(PyObject *self, PyObject **args, Py_ssize_t nargs, PyObject *kwnames
     }
 
     /* Support negative offsets. */
-    if (offset < 0)
+    if (offset < 0) {
+         /* Check that negative offset is low enough to fit data */
+        if (offset + soself->s_size > 0) {
+            PyErr_Format(StructError,
+                         "no space to pack %zd bytes at offset %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,
+                         "offset %zd out of range for %zd-byte buffer",
+                         offset,
+                         buffer.len);
+            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",
-                     soself->s_size);
+                     "pack_into requires a buffer of at least %zd bytes for "
+                     "packing %zd bytes at offset %zd "
+                     "(actual buffer size is %zd)",
+                     soself->s_size + offset,
+                     soself->s_size,
+                     offset,
+                     buffer.len);
         PyBuffer_Release(&buffer);
         return NULL;
     }
