diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index c595e8cf14f1e1..f8c957fef1d40b 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -154,14 +154,16 @@ def test_no_allow_code(self): data = {'a': [({co, 0},)]} dump = marshal.dumps(data, allow_code=True) self.assertEqual(marshal.loads(dump, allow_code=True), data) - with self.assertRaises(ValueError): + with self.assertRaisesRegex(ValueError, + 'marshalling code objects is disallowed'): marshal.dumps(data, allow_code=False) with self.assertRaises(ValueError): marshal.loads(dump, allow_code=False) marshal.dump(data, io.BytesIO(), allow_code=True) self.assertEqual(marshal.load(io.BytesIO(dump), allow_code=True), data) - with self.assertRaises(ValueError): + with self.assertRaisesRegex(ValueError, + 'marshalling code objects is disallowed'): marshal.dump(data, io.BytesIO(), allow_code=False) with self.assertRaises(ValueError): marshal.load(io.BytesIO(dump), allow_code=False) @@ -339,16 +341,29 @@ def test_reference_loop_dict(self): self.assertIsInstance(b, dict) self.assertIs(b[None], b) + def check_reference_loop(self, a, typename, minversion, + oldmsg='object too deeply nested to marshal'): + # Only versions supporting references to the type detect the loop; + # older versions fail for a different reason. + for v in range(minversion): + with self.subTest(version=v): + with self.assertRaisesRegex(ValueError, oldmsg): + marshal.dumps(a, v) + for v in range(minversion, marshal.version + 1): + with self.subTest(version=v): + with self.assertRaisesRegex( + ValueError, + f'cannot marshal recursion {typename} objects'): + marshal.dumps(a, v) + def test_reference_loop_tuple(self): a = ([],) a[0].append(a) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop(a, 'tuple', 3) a = ({},) a[0][None] = a - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop(a, 'tuple', 3) def test_shared_reference_tuple(self): # A tuple referenced more than once still round-trips with the @@ -373,30 +388,28 @@ def f(): # so we need to break the loop manually. See gh-148722. self.addCleanup(a.clear) a.append(code) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, code, v) + self.check_reference_loop(code, 'code', 3) def test_reference_loop_slice(self): + oldmsg = 'marshalling slice objects requires version 5 or higher' a = slice([], None) a.start.append(a) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop(a, 'slice', 5, oldmsg) a = slice(None, []) a.stop.append(a) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop(a, 'slice', 5, oldmsg) a = slice(None, None, []) a.step.append(a) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop(a, 'slice', 5, oldmsg) def test_reference_loop_frozendict(self): a = frozendict({None: []}) a[None].append(a) - for v in range(marshal.version + 1): - self.assertRaises(ValueError, marshal.dumps, a, v) + self.check_reference_loop( + a, 'frozendict', 6, + 'marshalling frozendict objects requires version 6 or higher') def test_shared_reference_frozendict(self): # A frozendict referenced more than once must round-trip with the @@ -467,7 +480,9 @@ def test_exact_type_match(self): # Note: str subclasses are not tested because they get handled # by marshal's routines for objects supporting the buffer API. subtyp = type('subtyp', (typ,), {}) - self.assertRaises(ValueError, marshal.dumps, subtyp()) + with self.assertRaisesRegex(ValueError, + r'cannot marshal \S*subtyp objects'): + marshal.dumps(subtyp()) # Issue #1792 introduced a change in how marshal increases the size of its # internal buffer; this test ensures that the new code is exercised. @@ -570,9 +585,25 @@ def test_unmarshallable(self): ('code', code)) for name, arg in cases: with self.subTest(name, arg=arg): - with self.assertRaisesRegex(ValueError, "unmarshallable object"): + with self.assertRaisesRegex(ValueError, + "cannot marshal type objects"): marshal.dumps((arg, memoryview(b''))) + def test_error_in_set_item(self): + # Set items are sorted by their marshalled representation, and NaNs + # are only distinguished by identity, so they are compared as + # complex numbers. + nan = float('nan') + with self.assertRaisesRegex(TypeError, "'<' not supported"): + marshal.dumps({complex(nan, 0), complex(nan, 0)}) + + def test_error_in_buffer(self): + # The BufferError raised for a non-contiguous buffer is not replaced + # with a generic error. + step2 = slice(None, None, 2) + with self.assertRaises(BufferError): + marshal.dumps(memoryview(bytearray(b'abcdef'))[step2]) + LARGE_SIZE = 2**31 pointer_size = 8 if sys.maxsize > 0xFFFFFFFF else 4 @@ -583,8 +614,14 @@ def write(self, s): @unittest.skipIf(LARGE_SIZE > sys.maxsize, "test cannot run on 32-bit systems") class LargeValuesTestCase(unittest.TestCase): - def check_unmarshallable(self, data): - self.assertRaises(ValueError, marshal.dump, data, NullWriter()) + def check_unmarshallable(self, data, msg='object too large to marshal'): + with self.assertRaisesRegex(ValueError, msg): + marshal.dump(data, NullWriter()) + + @support.bigmemtest(size=LARGE_SIZE, memuse=4, dry_run=False) + def test_int(self, size): + # An int with more than SIZE32_MAX 15-bit digits. + self.check_unmarshallable(1 << (15 * size), 'int too large to marshal') @support.bigmemtest(size=LARGE_SIZE, memuse=2, dry_run=False) def test_bytes(self, size): @@ -717,7 +754,10 @@ def testFrozenDict(self): self.helper(dictobj) for version in range(6): - with self.assertRaises(ValueError): + with self.assertRaisesRegex( + ValueError, + 'marshalling frozendict objects requires ' + 'version 6 or higher'): marshal.dumps(dictobj, version) def testModule(self): @@ -786,7 +826,10 @@ def test_slice(self): self.helper(obj) for version in range(5): - with self.assertRaises(ValueError): + with self.assertRaisesRegex( + ValueError, + 'marshalling slice objects requires ' + 'version 5 or higher'): marshal.dumps(obj, version) @support.cpython_only @@ -818,7 +861,7 @@ def test_write_to_file_error(self): def test_write_unmarshallable_to_file(self): self.addCleanup(os_helper.unlink, os_helper.TESTFN) - with self.assertRaisesRegex(ValueError, 'unmarshallable object'): + with self.assertRaisesRegex(ValueError, 'cannot marshal object objects'): _testcapi.pymarshal_write_object_to_file(object(), os_helper.TESTFN, marshal.version) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-09-04-16-12-30.gh-issue-156942.Qk7Rw2.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-04-16-12-30.gh-issue-156942.Qk7Rw2.rst new file mode 100644 index 00000000000000..24bbeef0461efb --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-04-16-12-30.gh-issue-156942.Qk7Rw2.rst @@ -0,0 +1,4 @@ +:mod:`marshal` no longer replaces the exception raised while marshalling the +value with a generic ``ValueError("unmarshallable object")``. Error messages +now name the type of the unsupported object and, for the types supported only +by newer data formats, the required version. diff --git a/Python/marshal.c b/Python/marshal.c index 1897d700c055bd..53e84d8efbfc6d 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -102,11 +102,7 @@ module marshal // Error codes: #define WFERR_OK 0 -#define WFERR_UNMARSHALLABLE 1 -#define WFERR_NESTEDTOODEEP 2 -#define WFERR_NOMEMORY 3 -#define WFERR_CODE_NOT_ALLOWED 4 -#define WFERR_EXCEPTION_SET 5 /* An exception has already been raised. */ +#define WFERR_EXCEPTION_SET 1 /* An exception has been raised. */ typedef struct { FILE *fp; @@ -174,12 +170,14 @@ w_reserve(WFILE *p, Py_ssize_t needed) delta = size + 1024; delta = Py_MAX(delta, needed); if (delta > PY_SSIZE_T_MAX - size) { - p->error = WFERR_NOMEMORY; + PyErr_NoMemory(); + p->error = WFERR_EXCEPTION_SET; return 0; } size += delta; if (_PyBytes_Resize(&p->str, size) != 0) { p->end = p->ptr = p->buf = NULL; + p->error = WFERR_EXCEPTION_SET; return 0; } else { @@ -236,13 +234,15 @@ w_long(long x, WFILE *p) #define SIZE32_MAX 0x7FFFFFFF #if SIZEOF_SIZE_T > 4 -# define W_SIZE(n, p) do { \ - if ((n) > SIZE32_MAX) { \ - (p)->depth--; \ - (p)->error = WFERR_UNMARSHALLABLE; \ - return; \ - } \ - w_long((long)(n), p); \ +# define W_SIZE(n, p) do { \ + if ((n) > SIZE32_MAX) { \ + (p)->depth--; \ + PyErr_SetString(PyExc_ValueError, \ + "object too large to marshal"); \ + (p)->error = WFERR_EXCEPTION_SET; \ + return; \ + } \ + w_long((long)(n), p); \ } while(0) #else # define W_SIZE w_long @@ -295,7 +295,8 @@ _r_digits##bitsize(const uint ## bitsize ## _t *digits, Py_ssize_t n, \ } while (d != 0); \ if (l > SIZE32_MAX) { \ p->depth--; \ - p->error = WFERR_UNMARSHALLABLE; \ + PyErr_SetString(PyExc_ValueError, "int too large to marshal"); \ + p->error = WFERR_EXCEPTION_SET; \ return; \ } \ w_long((long)(negative ? -l : l), p); \ @@ -331,7 +332,7 @@ w_PyLong(const PyLongObject *ob, char flag, WFILE *p) if (PyLong_Export((PyObject *)ob, &long_export) < 0) { p->depth--; - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; return; } if (!long_export.digits) { @@ -384,7 +385,7 @@ w_float_bin(double v, WFILE *p) { char buf[8]; if (PyFloat_Pack8(v, buf, 1) < 0) { - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; return; } w_string(buf, 8, p); @@ -395,7 +396,7 @@ w_float_str(double v, WFILE *p) { char *buf = PyOS_double_to_string(v, 'g', 17, 0, NULL); if (!buf) { - p->error = WFERR_NOMEMORY; + p->error = WFERR_EXCEPTION_SET; return; } w_short_pstring(buf, strlen(buf), p); @@ -449,13 +450,14 @@ w_ref(PyObject *v, char *flag, WFILE *p) if (_Py_hashtable_set(p->hashtable, Py_NewRef(v), (void *)(uintptr_t)w) < 0) { Py_DECREF(v); + PyErr_NoMemory(); goto err; } *flag |= FLAG_REF; return 0; } err: - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; return 1; } @@ -495,7 +497,9 @@ w_object(PyObject *v, WFILE *p) p->depth++; if (p->depth > MAX_MARSHAL_STACK_DEPTH) { - p->error = WFERR_NESTEDTOODEEP; + PyErr_SetString(PyExc_ValueError, + "object too deeply nested to marshal"); + p->error = WFERR_EXCEPTION_SET; } else if (v == NULL) { w_byte(TYPE_NULL, p); @@ -598,7 +602,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p) utf8 = PyUnicode_AsEncodedString(v, "utf8", "surrogatepass"); if (utf8 == NULL) { p->depth--; - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; return; } if (p->version >= 3 && PyUnicode_CHECK_INTERNED(v)) @@ -638,7 +642,10 @@ w_complex_object(PyObject *v, char flag, WFILE *p) if (PyFrozenDict_CheckExact(v)) { if (p->version < 6) { w_byte(TYPE_UNKNOWN, p); - p->error = WFERR_UNMARSHALLABLE; + PyErr_Format(PyExc_ValueError, + "marshalling %T objects requires version 6 " + "or higher", v); + p->error = WFERR_EXCEPTION_SET; return; } @@ -675,7 +682,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p) // use an order equivalent to sorted(v, key=marshal.dumps): PyObject *pairs = PyList_New(n); if (pairs == NULL) { - p->error = WFERR_NOMEMORY; + p->error = WFERR_EXCEPTION_SET; return; } Py_ssize_t i = 0; @@ -684,25 +691,25 @@ w_complex_object(PyObject *v, char flag, WFILE *p) PyObject *dump = _PyMarshal_WriteObjectToString(value, p->version, p->allow_code); if (dump == NULL) { - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; Py_DECREF(value); break; } PyObject *pair = _PyTuple_FromPairSteal(dump, value); if (pair == NULL) { - p->error = WFERR_NOMEMORY; + p->error = WFERR_EXCEPTION_SET; break; } PyList_SET_ITEM(pairs, i++, pair); } Py_END_CRITICAL_SECTION(); - if (p->error == WFERR_UNMARSHALLABLE || p->error == WFERR_NOMEMORY) { + if (p->error != WFERR_OK) { Py_DECREF(pairs); return; } assert(i == n); if (PyList_Sort(pairs)) { - p->error = WFERR_NOMEMORY; + p->error = WFERR_EXCEPTION_SET; Py_DECREF(pairs); return; } @@ -715,13 +722,15 @@ w_complex_object(PyObject *v, char flag, WFILE *p) } else if (PyCode_Check(v)) { if (!p->allow_code) { - p->error = WFERR_CODE_NOT_ALLOWED; + PyErr_SetString(PyExc_ValueError, + "marshalling code objects is disallowed"); + p->error = WFERR_EXCEPTION_SET; return; } PyCodeObject *co = (PyCodeObject *)v; PyObject *co_code = _PyCode_GetCode(co); if (co_code == NULL) { - p->error = WFERR_NOMEMORY; + p->error = WFERR_EXCEPTION_SET; return; } W_TYPE(TYPE_CODE, p); @@ -750,7 +759,7 @@ w_complex_object(PyObject *v, char flag, WFILE *p) if (PyObject_GetBuffer(v, &view, PyBUF_SIMPLE) != 0) { w_byte(TYPE_UNKNOWN, p); p->depth--; - p->error = WFERR_UNMARSHALLABLE; + p->error = WFERR_EXCEPTION_SET; return; } W_TYPE(TYPE_STRING, p); @@ -760,7 +769,10 @@ w_complex_object(PyObject *v, char flag, WFILE *p) else if (PySlice_Check(v)) { if (p->version < 5) { w_byte(TYPE_UNKNOWN, p); - p->error = WFERR_UNMARSHALLABLE; + PyErr_Format(PyExc_ValueError, + "marshalling %T objects requires version 5 " + "or higher", v); + p->error = WFERR_EXCEPTION_SET; return; } PySliceObject *slice = (PySliceObject *)v; @@ -772,7 +784,8 @@ w_complex_object(PyObject *v, char flag, WFILE *p) } else { W_TYPE(TYPE_UNKNOWN, p); - p->error = WFERR_UNMARSHALLABLE; + PyErr_Format(PyExc_ValueError, "cannot marshal %T objects", v); + p->error = WFERR_EXCEPTION_SET; } } @@ -806,35 +819,6 @@ w_clear_refs(WFILE *wf) } } -/* Set the exception indicator according to the recorded error. */ -static void -w_set_exception(WFILE *p) -{ - assert(p->error != WFERR_OK); - switch (p->error) { - case WFERR_NOMEMORY: - PyErr_NoMemory(); - break; - case WFERR_NESTEDTOODEEP: - PyErr_SetString(PyExc_ValueError, - "object too deeply nested to marshal"); - break; - case WFERR_CODE_NOT_ALLOWED: - PyErr_SetString(PyExc_ValueError, - "marshalling code objects is disallowed"); - break; - case WFERR_EXCEPTION_SET: - /* An exception has already been raised. */ - assert(PyErr_Occurred()); - break; - default: - case WFERR_UNMARSHALLABLE: - PyErr_SetString(PyExc_ValueError, - "unmarshallable object"); - break; - } -} - /* version currently has no effect for writing ints. */ void PyMarshal_WriteLongToFile(long x, FILE *fp, int version) @@ -849,9 +833,7 @@ PyMarshal_WriteLongToFile(long x, FILE *fp, int version) wf.version = version; w_long(x, &wf); w_flush(&wf); - if (wf.error != WFERR_OK) { - w_set_exception(&wf); - } + assert(wf.error == WFERR_OK || PyErr_Occurred()); } void @@ -875,9 +857,7 @@ PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version) w_object(x, &wf); w_clear_refs(&wf); w_flush(&wf); - if (wf.error != WFERR_OK) { - w_set_exception(&wf); - } + assert(wf.error == WFERR_OK || PyErr_Occurred()); } typedef struct { @@ -2024,8 +2004,8 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) return NULL; } if (wf.error != WFERR_OK) { + assert(PyErr_Occurred()); Py_XDECREF(wf.str); - w_set_exception(&wf); return NULL; } return wf.str;