From 439a9dd515e91ecfc6f9161bfa9263340d5c4652 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Wed, 23 Apr 2025 21:53:08 +0100 Subject: [PATCH 1/5] Commit --- Doc/library/struct.rst | 4 + Lib/test/test_struct.py | 2 +- ...-04-23-21-40-00.gh-issue-67766.0x8FFFF.rst | 1 + Modules/_struct.c | 79 +++++++++++-------- 4 files changed, 50 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-04-23-21-40-00.gh-issue-67766.0x8FFFF.rst diff --git a/Doc/library/struct.rst b/Doc/library/struct.rst index 3ea9e5ba071289..a57bd997ee0906 100644 --- a/Doc/library/struct.rst +++ b/Doc/library/struct.rst @@ -55,6 +55,10 @@ The module defines the following exception and functions: Exception raised on various occasions; argument is a string describing what is wrong. + .. versionchanged:: + Out of range errors are more descriptive specifying which argument resulted + in the exception. + .. function:: pack(format, v1, v2, ...) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index b99391e482ff70..750001d5871364 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -742,7 +742,7 @@ def test_error_msg(prefix, int_type, is_unsigned): else: max_ = 2 ** (size * 8 - 1) - 1 min_ = -2 ** (size * 8 - 1) - error_msg = f"'{int_type}' format requires {min_} <= number <= {max_}" + error_msg = f"'{int_type}' format requires {min_} <= arg 1 <= {max_}" for number in [int(-1e50), min_ - 1, max_ + 1, int(1e50)]: with self.subTest(format_str=fmt_str, number=number): with self.assertRaisesRegex(struct.error, error_msg): diff --git a/Misc/NEWS.d/next/Library/2025-04-23-21-40-00.gh-issue-67766.0x8FFFF.rst b/Misc/NEWS.d/next/Library/2025-04-23-21-40-00.gh-issue-67766.0x8FFFF.rst new file mode 100644 index 00000000000000..e2254e41cfae3c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-04-23-21-40-00.gh-issue-67766.0x8FFFF.rst @@ -0,0 +1 @@ +:func:`struct.pack` raises more informative errors when an argument is out of range. diff --git a/Modules/_struct.c b/Modules/_struct.c index f04805d9d6d1d7..ca6b2be41341e4 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -27,6 +27,7 @@ typedef struct { PyObject *PyStructType; PyObject *unpackiter_type; PyObject *StructError; + Py_ssize_t current_arg; } _structmodulestate; static inline _structmodulestate* @@ -290,7 +291,7 @@ get_size_t(_structmodulestate *state, PyObject *v, size_t *p) } -#define RANGE_ERROR(state, f, flag) return _range_error(state, f, flag) +#define RANGE_ERROR(state, f, flag, loc) return _range_error(state, f, flag, loc) /* Floating-point helpers */ @@ -347,7 +348,7 @@ unpack_double(const char *p, /* start of 8-byte string */ /* Helper to format the range error exceptions */ static int -_range_error(_structmodulestate *state, const formatdef *f, int is_unsigned) +_range_error(_structmodulestate *state, const formatdef *f, int is_unsigned, Py_ssize_t loc) { /* ulargest is the largest unsigned value with f->size bytes. * Note that the simpler: @@ -361,15 +362,17 @@ _range_error(_structmodulestate *state, const formatdef *f, int is_unsigned) assert(f->size >= 1 && f->size <= SIZEOF_SIZE_T); if (is_unsigned) PyErr_Format(state->StructError, - "'%c' format requires 0 <= number <= %zu", + "'%c' format requires 0 <= arg %zd <= %zu", f->format, + loc, ulargest); else { const Py_ssize_t largest = (Py_ssize_t)(ulargest >> 1); PyErr_Format(state->StructError, - "'%c' format requires %zd <= number <= %zd", + "'%c' format requires %zd <= arg %zd <= %zd", f->format, ~ largest, + loc, largest); } @@ -563,12 +566,12 @@ np_byte(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) long x; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } if (x < -128 || x > 127) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } *p = (char)x; return 0; @@ -580,12 +583,12 @@ np_ubyte(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) long x; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } if (x < 0 || x > 255) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } *(unsigned char *)p = (unsigned char)x; return 0; @@ -610,12 +613,12 @@ np_short(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) short y; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } if (x < SHRT_MIN || x > SHRT_MAX) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } y = (short)x; memcpy(p, &y, sizeof y); @@ -629,12 +632,12 @@ np_ushort(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned short y; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } if (x < 0 || x > USHRT_MAX) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } y = (unsigned short)x; memcpy(p, &y, sizeof y); @@ -648,13 +651,13 @@ np_int(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) int y; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } #if (SIZEOF_LONG > SIZEOF_INT) if ((x < ((long)INT_MIN)) || (x > ((long)INT_MAX))) - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); #endif y = (int)x; memcpy(p, &y, sizeof y); @@ -668,14 +671,14 @@ np_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned int y; if (get_ulong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } y = (unsigned int)x; #if (SIZEOF_LONG > SIZEOF_INT) if (x > ((unsigned long)UINT_MAX)) - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); #endif memcpy(p, &y, sizeof y); return 0; @@ -687,7 +690,7 @@ np_long(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) long x; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } @@ -701,7 +704,7 @@ np_ulong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned long x; if (get_ulong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } @@ -715,7 +718,7 @@ np_ssize_t(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_ssize_t x; if (get_ssize_t(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } @@ -729,7 +732,7 @@ np_size_t(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) size_t x; if (get_size_t(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } @@ -744,9 +747,10 @@ np_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) if (get_longlong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= number <= %lld", + "'%c' format requires %lld <= arg %zd <= %lld", f->format, LLONG_MIN, + state->current_arg, LLONG_MAX); } return -1; @@ -762,8 +766,9 @@ np_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f if (get_ulonglong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= number <= %llu", + "'%c' format requires 0 <= arg %zd <= %llu", f->format, + state->current_arg, ULLONG_MAX); } return -1; @@ -1065,17 +1070,17 @@ bp_int(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned char *q = (unsigned char *)p; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } i = f->size; if (i != SIZEOF_LONG) { if ((i == 2) && (x < -32768 || x > 32767)) - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); #if (SIZEOF_LONG != 4) else if ((i == 4) && (x < -2147483648L || x > 2147483647L)) - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); #endif } do { @@ -1093,7 +1098,7 @@ bp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned char *q = (unsigned char *)p; if (get_ulong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } @@ -1102,7 +1107,7 @@ bp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned long maxint = 1; maxint <<= (unsigned long)(i * 8); if (x >= maxint) - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } do { q[--i] = (unsigned char)(x & 0xffUL); @@ -1127,9 +1132,10 @@ bp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= number <= %lld", + "'%c' format requires %lld <= arg %zd <= %lld", f->format, LLONG_MIN, + state->current_arg, LLONG_MAX); return -1; } @@ -1152,8 +1158,9 @@ bp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= number <= %llu", + "'%c' format requires 0 <= arg %zd <= %llu", f->format, + state->current_arg, ULLONG_MAX); return -1; } @@ -1398,17 +1405,17 @@ lp_int(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned char *q = (unsigned char *)p; if (get_long(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); } return -1; } i = f->size; if (i != SIZEOF_LONG) { if ((i == 2) && (x < -32768 || x > 32767)) - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); #if (SIZEOF_LONG != 4) else if ((i == 4) && (x < -2147483648L || x > 2147483647L)) - RANGE_ERROR(state, f, 0); + RANGE_ERROR(state, f, 0, state->current_arg); #endif } do { @@ -1426,7 +1433,7 @@ lp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned char *q = (unsigned char *)p; if (get_ulong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } return -1; } @@ -1435,7 +1442,7 @@ lp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) unsigned long maxint = 1; maxint <<= (unsigned long)(i * 8); if (x >= maxint) - RANGE_ERROR(state, f, 1); + RANGE_ERROR(state, f, 1, state->current_arg); } do { *q++ = (unsigned char)(x & 0xffUL); @@ -1460,9 +1467,10 @@ lp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= number <= %lld", + "'%c' format requires %lld <= arg %zd <= %lld", f->format, LLONG_MIN, + state->current_arg, LLONG_MAX); return -1; } @@ -2193,6 +2201,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, char *res = buf + code->offset; Py_ssize_t j = code->repeat; while (j--) { + state->current_arg = i + 1; PyObject *v = args[i++]; if (e->format == 's') { Py_ssize_t n; From dfdd4b7ad576199c86a6744e84544a8ca2cbd6f9 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Wed, 23 Apr 2025 22:16:16 +0100 Subject: [PATCH 2/5] Clean up --- Doc/library/struct.rst | 2 +- Modules/_struct.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/struct.rst b/Doc/library/struct.rst index a57bd997ee0906..1861754b5e9aff 100644 --- a/Doc/library/struct.rst +++ b/Doc/library/struct.rst @@ -55,7 +55,7 @@ The module defines the following exception and functions: Exception raised on various occasions; argument is a string describing what is wrong. - .. versionchanged:: + .. versionchanged:: next Out of range errors are more descriptive specifying which argument resulted in the exception. diff --git a/Modules/_struct.c b/Modules/_struct.c index ca6b2be41341e4..0817a4f05ad845 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -2201,8 +2201,8 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, char *res = buf + code->offset; Py_ssize_t j = code->repeat; while (j--) { - state->current_arg = i + 1; PyObject *v = args[i++]; + state->current_arg = i; if (e->format == 's') { Py_ssize_t n; int isstring; From 551ae175f0b57abe5702c55c963e742dd14365c6 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Thu, 24 Apr 2025 18:16:52 +0100 Subject: [PATCH 3/5] Update error message --- Doc/library/struct.rst | 4 ---- Lib/test/test_struct.py | 2 +- Modules/_struct.c | 47 +++++++++++++++++++++-------------------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/Doc/library/struct.rst b/Doc/library/struct.rst index 1861754b5e9aff..3ea9e5ba071289 100644 --- a/Doc/library/struct.rst +++ b/Doc/library/struct.rst @@ -55,10 +55,6 @@ The module defines the following exception and functions: Exception raised on various occasions; argument is a string describing what is wrong. - .. versionchanged:: next - Out of range errors are more descriptive specifying which argument resulted - in the exception. - .. function:: pack(format, v1, v2, ...) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 750001d5871364..15fc1f9137b822 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -742,7 +742,7 @@ def test_error_msg(prefix, int_type, is_unsigned): else: max_ = 2 ** (size * 8 - 1) - 1 min_ = -2 ** (size * 8 - 1) - error_msg = f"'{int_type}' format requires {min_} <= arg 1 <= {max_}" + error_msg = f"'{int_type}' format requires {min_} <= number <= {max_} (at item 1)" for number in [int(-1e50), min_ - 1, max_ + 1, int(1e50)]: with self.subTest(format_str=fmt_str, number=number): with self.assertRaisesRegex(struct.error, error_msg): diff --git a/Modules/_struct.c b/Modules/_struct.c index 0817a4f05ad845..488c3749ea3e3a 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -362,18 +362,18 @@ _range_error(_structmodulestate *state, const formatdef *f, int is_unsigned, Py_ assert(f->size >= 1 && f->size <= SIZEOF_SIZE_T); if (is_unsigned) PyErr_Format(state->StructError, - "'%c' format requires 0 <= arg %zd <= %zu", + "'%c' format requires 0 <= number <= %zu (at item %zd)", f->format, - loc, - ulargest); + ulargest, + loc); else { const Py_ssize_t largest = (Py_ssize_t)(ulargest >> 1); PyErr_Format(state->StructError, - "'%c' format requires %zd <= arg %zd <= %zd", + "'%c' format requires %zd <= number <= %zd (at item %zd)", f->format, ~ largest, - loc, - largest); + largest, + loc); } return -1; @@ -747,11 +747,11 @@ np_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) if (get_longlong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= arg %zd <= %lld", + "'%c' format requires %lld <= number <= %lld (at item %zd)", f->format, LLONG_MIN, - state->current_arg, - LLONG_MAX); + LLONG_MAX, + state->current_arg); } return -1; } @@ -766,10 +766,10 @@ np_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f if (get_ulonglong(state, v, &x) < 0) { if (PyErr_ExceptionMatches(PyExc_OverflowError)) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= arg %zd <= %llu", + "'%c' format requires 0 <= number <= %llu (at item %zd)", f->format, - state->current_arg, - ULLONG_MAX); + ULLONG_MAX, + state->current_arg); } return -1; } @@ -1132,11 +1132,11 @@ bp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= arg %zd <= %lld", + "'%c' format requires %lld <= number <= %lld (at item %zd)", f->format, LLONG_MIN, - state->current_arg, - LLONG_MAX); + LLONG_MAX, + state->current_arg); return -1; } return res; @@ -1158,10 +1158,10 @@ bp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= arg %zd <= %llu", + "'%c' format requires 0 <= number <= %llu (at item %zd)", f->format, - state->current_arg, - ULLONG_MAX); + ULLONG_MAX, + state->current_arg); return -1; } return res; @@ -1467,11 +1467,11 @@ lp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires %lld <= arg %zd <= %lld", + "'%c' format requires %lld <= number <= %lld (at item %zd)", f->format, LLONG_MIN, - state->current_arg, - LLONG_MAX); + LLONG_MAX, + state->current_arg); return -1; } return res; @@ -1493,9 +1493,10 @@ lp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= number <= %llu", + "'%c' format requires 0 <= number <= %llu (at item %zd)", f->format, - ULLONG_MAX); + ULLONG_MAX, + state->current_arg); return -1; } return res; From 8f2c341a474ddbcc7db7f97d996262ca683ce893 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Thu, 24 Apr 2025 18:59:26 +0100 Subject: [PATCH 4/5] !fixup --- Modules/_struct.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_struct.c b/Modules/_struct.c index 488c3749ea3e3a..9649f9a7e03c47 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1493,10 +1493,9 @@ lp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res < 0) { PyErr_Format(state->StructError, - "'%c' format requires 0 <= number <= %llu (at item %zd)", + "'%c' format requires 0 <= number <= %llu", f->format, - ULLONG_MAX, - state->current_arg); + ULLONG_MAX); return -1; } return res; From 3393c378f12a913bd8fc5d29c7ffa557f272a7df Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Thu, 24 Apr 2025 19:15:57 +0100 Subject: [PATCH 5/5] Escape test --- Lib/test/test_struct.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 15fc1f9137b822..a25f2a82afaf7e 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -3,6 +3,7 @@ import array import gc import math +import re import operator import unittest import struct @@ -742,7 +743,7 @@ def test_error_msg(prefix, int_type, is_unsigned): else: max_ = 2 ** (size * 8 - 1) - 1 min_ = -2 ** (size * 8 - 1) - error_msg = f"'{int_type}' format requires {min_} <= number <= {max_} (at item 1)" + error_msg = re.escape(f"'{int_type}' format requires {min_} <= number <= {max_} (at item 1)") for number in [int(-1e50), min_ - 1, max_ + 1, int(1e50)]: with self.subTest(format_str=fmt_str, number=number): with self.assertRaisesRegex(struct.error, error_msg):