From bb393b7b39f80609ddc0f870f1005a2532149d85 Mon Sep 17 00:00:00 2001 From: Joris van der Geer Date: Fri, 21 Apr 2023 09:15:44 +1000 Subject: [PATCH 1/2] gh-67766 - show argument for struct.pack range errors --- Lib/test/test_struct.py | 2 +- Modules/_struct.c | 103 ++++++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 46 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 6b1f22f66fd157..3df0638e761fbf 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -733,7 +733,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/Modules/_struct.c b/Modules/_struct.c index 3db7b991acd0a1..1414da20f44434 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -24,6 +24,7 @@ typedef struct { PyObject *PyStructType; PyObject *unpackiter_type; PyObject *StructError; + Py_ssize_t current_arg; } _structmodulestate; static inline _structmodulestate* @@ -275,9 +276,6 @@ get_size_t(_structmodulestate *state, PyObject *v, size_t *p) } -#define RANGE_ERROR(state, f, flag) return _range_error(state, f, flag) - - /* Floating point helpers */ static PyObject * @@ -332,7 +330,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 argno) { /* ulargest is the largest unsigned value with f->size bytes. * Note that the simpler: @@ -346,15 +344,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, + argno, 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, + argno, largest); } @@ -529,12 +529,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); + return _range_error(state, f, 0, state->current_arg); } return -1; } if (x < -128 || x > 127) { - RANGE_ERROR(state, f, 0); + return _range_error(state, f, 0, state->current_arg); } *p = (char)x; return 0; @@ -546,12 +546,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } if (x < 0 || x > 255) { - RANGE_ERROR(state, f, 1); + return _range_error(state, f, 1, state->current_arg); } *(unsigned char *)p = (unsigned char)x; return 0; @@ -576,12 +576,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); + return _range_error(state, f, 0, state->current_arg); } return -1; } if (x < SHRT_MIN || x > SHRT_MAX) { - RANGE_ERROR(state, f, 0); + return _range_error(state, f, 0, state->current_arg); } y = (short)x; memcpy(p, (char *)&y, sizeof y); @@ -595,12 +595,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } if (x < 0 || x > USHRT_MAX) { - RANGE_ERROR(state, f, 1); + return _range_error(state, f, 1, state->current_arg); } y = (unsigned short)x; memcpy(p, (char *)&y, sizeof y); @@ -614,13 +614,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); + return _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); + return _range_error(state, f, 0, state->current_arg); #endif y = (int)x; memcpy(p, (char *)&y, sizeof y); @@ -634,14 +634,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); + return _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); + return _range_error(state, f, 1, state->current_arg); #endif memcpy(p, (char *)&y, sizeof y); return 0; @@ -653,7 +653,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); + return _range_error(state, f, 0, state->current_arg); } return -1; } @@ -667,7 +667,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } @@ -681,7 +681,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); + return _range_error(state, f, 0, state->current_arg); } return -1; } @@ -695,7 +695,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } @@ -710,9 +710,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; @@ -728,8 +729,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; @@ -940,17 +942,19 @@ 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); + return _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); + if ((i == 2) && (x < -32768 || x > 32767)) { + return _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); + else if ((i == 4) && (x < -2147483648L || x > 2147483647L)) { + return _range_error(state, f, 0, state->current_arg); + } #endif } do { @@ -968,7 +972,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } @@ -976,8 +980,9 @@ bp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) if (i != SIZEOF_LONG) { unsigned long maxint = 1; maxint <<= (unsigned long)(i * 8); - if (x >= maxint) - RANGE_ERROR(state, f, 1); + if (x >= maxint) { + return _range_error(state, f, 1, state->current_arg); + } } do { q[--i] = (unsigned char)(x & 0xffUL); @@ -1001,9 +1006,10 @@ bp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res == -1 && PyErr_Occurred()) { 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; } @@ -1025,8 +1031,9 @@ bp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res == -1 && PyErr_Occurred()) { 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; } @@ -1200,17 +1207,19 @@ 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); + return _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); + if ((i == 2) && (x < -32768 || x > 32767)) { + return _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); + else if ((i == 4) && (x < -2147483648L || x > 2147483647L)) { + return _range_error(state, f, 0, state->current_arg); + } #endif } do { @@ -1228,7 +1237,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); + return _range_error(state, f, 1, state->current_arg); } return -1; } @@ -1236,8 +1245,9 @@ lp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) if (i != SIZEOF_LONG) { unsigned long maxint = 1; maxint <<= (unsigned long)(i * 8); - if (x >= maxint) - RANGE_ERROR(state, f, 1); + if (x >= maxint) { + return _range_error(state, f, 1, state->current_arg); + } } do { *q++ = (unsigned char)(x & 0xffUL); @@ -1261,9 +1271,10 @@ lp_longlong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f) Py_DECREF(v); if (res == -1 && PyErr_Occurred()) { 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; } @@ -1285,8 +1296,9 @@ lp_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f Py_DECREF(v); if (res == -1 && PyErr_Occurred()) { 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; } @@ -1913,7 +1925,7 @@ Struct_iter_unpack(PyStructObject *self, PyObject *buffer) * * Takes a struct object, a tuple of arguments, and offset in that tuple of * argument for where to start processing the arguments for packing, and a - * character buffer for writing the packed string. The caller must insure + * character buffer for writing the packed string. The caller must ensure * that the buffer may contain the required length for packing the arguments. * 0 is returned on success, 1 is returned if there is an error. * @@ -1934,6 +1946,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; /* Provide context for errors - gh-67766 */ PyObject *v = args[i++]; if (e->format == 's') { Py_ssize_t n; From 78a04ab23c0a2dd07937674463e61ba710ca3abb Mon Sep 17 00:00:00 2001 From: Joris van der Geer Date: Fri, 21 Apr 2023 10:04:53 +1000 Subject: [PATCH 2/2] gh-67766 - add blurb --- .../next/Library/2023-04-21-09-59-57.gh-issue-67766.YrcQ2J.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2023-04-21-09-59-57.gh-issue-67766.YrcQ2J.rst diff --git a/Misc/NEWS.d/next/Library/2023-04-21-09-59-57.gh-issue-67766.YrcQ2J.rst b/Misc/NEWS.d/next/Library/2023-04-21-09-59-57.gh-issue-67766.YrcQ2J.rst new file mode 100644 index 00000000000000..ae85073e065de9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-21-09-59-57.gh-issue-67766.YrcQ2J.rst @@ -0,0 +1 @@ +Show argument number in struct.pack range error message