https://github.com/python/cpython/commit/aaae15c35b5fba1246b8998e6586a76fc1541be7 commit: aaae15c35b5fba1246b8998e6586a76fc1541be7 branch: main author: Serhiy Storchaka <[email protected]> committer: serhiy-storchaka <[email protected]> date: 2026-09-04T16:44:57+03:00 summary:
gh-155907: Fix error handling in the marshal C API (GH-155909) Functions reading marshalled data from a FILE* now raise OSError for I/O errors and KeyboardInterrupt for interrupted reading, instead of EOFError. PyMarshal_WriteObjectToFile() and PyMarshal_WriteLongToFile() now detect I/O errors and interrupted writing instead of ignoring them. PyMarshal_WriteObjectToFile() now also sets the error indicator if the value cannot be marshalled. Co-authored-by: Claude Opus 5 (1M context) <[email protected]> files: A Misc/NEWS.d/next/C_API/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst M Doc/c-api/marshal.rst M Doc/whatsnew/3.16.rst M Lib/test/test_marshal.py M Modules/_testcapimodule.c M Python/marshal.c diff --git a/Doc/c-api/marshal.rst b/Doc/c-api/marshal.rst index 668a163b2df5a11..5bf4757f8ae158b 100644 --- a/Doc/c-api/marshal.rst +++ b/Doc/c-api/marshal.rst @@ -16,6 +16,20 @@ Numeric values are stored with the least significant byte first. The module supports several versions of the data format; see the :py:mod:`Python module documentation <marshal>` for details. +The following exceptions can be raised by these functions: +:exc:`ValueError` if the value cannot be marshalled, +:exc:`ValueError` or :exc:`TypeError` if the data is malformed, +:exc:`EOFError` if the end of the data is reached before the value is complete, +:exc:`OSError` if reading from or writing to a :c:expr:`FILE*` fails, +:exc:`KeyboardInterrupt` if reading or writing is interrupted by a signal, +and :exc:`MemoryError` if memory allocation fails. + +.. versionchanged:: next + Previously, in functions taking a :c:expr:`FILE*`, + the reading functions raised :exc:`EOFError` + instead of :exc:`OSError` and :exc:`KeyboardInterrupt`, + and the writing functions ignored I/O errors and interruptions. + .. c:macro:: Py_MARSHAL_VERSION The current format version. See :py:data:`marshal.version`. @@ -42,6 +56,8 @@ the :py:mod:`Python module documentation <marshal>` for details. Return a bytes object containing the marshalled representation of *value*. *version* indicates the file format. + On error, raises an exception and returns ``NULL``. + The following functions allow marshalled values to be read back in. @@ -52,8 +68,7 @@ The following functions allow marshalled values to be read back in. for reading. Only a 32-bit value can be read in using this function, regardless of the native size of :c:expr:`long`. - On error, sets the appropriate exception (:exc:`EOFError`) and returns - ``-1``. + On error, raises an exception and returns ``-1``. .. c:function:: int PyMarshal_ReadShortFromFile(FILE *file) @@ -62,8 +77,7 @@ The following functions allow marshalled values to be read back in. for reading. Only a 16-bit value can be read in using this function, regardless of the native size of :c:expr:`short`. - On error, sets the appropriate exception (:exc:`EOFError`) and returns - ``-1``. + On error, raises an exception and returns ``-1``. .. c:function:: PyObject* PyMarshal_ReadObjectFromFile(FILE *file) @@ -71,8 +85,7 @@ The following functions allow marshalled values to be read back in. Return a Python object from the data stream in a :c:expr:`FILE*` opened for reading. - On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError` - or :exc:`TypeError`) and returns ``NULL``. + On error, raises an exception and returns ``NULL``. .. c:function:: PyObject* PyMarshal_ReadLastObjectFromFile(FILE *file) @@ -85,8 +98,7 @@ The following functions allow marshalled values to be read back in. file. Only use this variant if you are certain that you won't be reading anything else from the file. - On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError` - or :exc:`TypeError`) and returns ``NULL``. + On error, raises an exception and returns ``NULL``. .. c:function:: PyObject* PyMarshal_ReadObjectFromString(const char *data, Py_ssize_t len) @@ -94,6 +106,5 @@ The following functions allow marshalled values to be read back in. Return a Python object from the data stream in a byte buffer containing *len* bytes pointed to by *data*. - On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError` - or :exc:`TypeError`) and returns ``NULL``. + On error, raises an exception and returns ``NULL``. diff --git a/Doc/whatsnew/3.16.rst b/Doc/whatsnew/3.16.rst index 3262acd87d6d49f..75541f1e106752e 100644 --- a/Doc/whatsnew/3.16.rst +++ b/Doc/whatsnew/3.16.rst @@ -956,6 +956,20 @@ Porting to Python 3.16 * :c:func:`PyType_ClearCache` is now a no-op as the type cache is now implemented per-type. It still returns the current version tag. +* Functions reading marshalled data from a :c:expr:`FILE*`, + such as :c:func:`PyMarshal_ReadObjectFromFile`, + now raise :exc:`OSError` for I/O errors + and :exc:`KeyboardInterrupt` for interrupted reading, + instead of :exc:`EOFError`. + (Contributed by Serhiy Storchaka in :gh:`155907`.) + +* :c:func:`PyMarshal_WriteLongToFile` and :c:func:`PyMarshal_WriteObjectToFile` + now set the error indicator for I/O errors and interrupted writing, + instead of ignoring them. + :c:func:`PyMarshal_WriteObjectToFile` now also sets the error indicator + if the value cannot be marshalled. + (Contributed by Serhiy Storchaka in :gh:`155907`.) + Deprecated C APIs ----------------- diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index b5bacadbfd381fc..c595e8cf14f1e15 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -793,6 +793,35 @@ def test_slice(self): @unittest.skipUnless(_testcapi, 'requires _testcapi') class CAPI_TestCase(unittest.TestCase, HelperMixin): + def test_read_from_file_error(self): + # A read error is reported as OSError, not EOFError. + # A directory cannot be read (on some platforms it cannot even + # be opened, which is reported as OSError as well). + os.mkdir(os_helper.TESTFN) + self.addCleanup(os_helper.rmdir, os_helper.TESTFN) + for func in (_testcapi.pymarshal_read_short_from_file, + _testcapi.pymarshal_read_long_from_file, + _testcapi.pymarshal_read_object_from_file, + _testcapi.pymarshal_read_last_object_from_file): + with self.subTest(func=func.__name__): + self.assertRaises(OSError, func, os_helper.TESTFN) + + @unittest.skipUnless(os.path.exists('/dev/full'), 'requires /dev/full') + def test_write_to_file_error(self): + # A write error is reported as OSError. + # The data is large enough to not fit in the stdio buffer, so that + # the error is detected before the file is closed. + obj = b'x' * 100000 + with self.assertRaises(OSError): + _testcapi.pymarshal_write_object_to_file(obj, '/dev/full', + marshal.version) + + def test_write_unmarshallable_to_file(self): + self.addCleanup(os_helper.unlink, os_helper.TESTFN) + with self.assertRaisesRegex(ValueError, 'unmarshallable object'): + _testcapi.pymarshal_write_object_to_file(object(), os_helper.TESTFN, + marshal.version) + def test_write_long_to_file(self): for v in range(marshal.version + 1): _testcapi.pymarshal_write_long_to_file(0x12345678, os_helper.TESTFN, v) diff --git a/Misc/NEWS.d/next/C_API/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst b/Misc/NEWS.d/next/C_API/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst new file mode 100644 index 000000000000000..215bb2d031c42ee --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst @@ -0,0 +1,7 @@ +:c:func:`PyMarshal_ReadObjectFromFile` and other functions reading marshalled +data from a :c:expr:`FILE*` now raise :exc:`OSError` for I/O errors and +:exc:`KeyboardInterrupt` for interrupted reading, instead of :exc:`EOFError`. +:c:func:`PyMarshal_WriteObjectToFile` and :c:func:`PyMarshal_WriteLongToFile` +now detect I/O errors and interrupted writing instead of ignoring them. +:c:func:`PyMarshal_WriteObjectToFile` now also sets the error indicator if the +value cannot be marshalled. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index f6009e7e73f249a..0312ee9066231c4 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1436,9 +1436,11 @@ pymarshal_write_long_to_file(PyObject* self, PyObject *args) } PyMarshal_WriteLongToFile(value, fp, version); - assert(!PyErr_Occurred()); fclose(fp); + if (PyErr_Occurred()) { + return NULL; + } Py_RETURN_NONE; } @@ -1460,9 +1462,11 @@ pymarshal_write_object_to_file(PyObject* self, PyObject *args) } PyMarshal_WriteObjectToFile(obj, fp, version); - assert(!PyErr_Occurred()); fclose(fp); + if (PyErr_Occurred()) { + return NULL; + } Py_RETURN_NONE; } diff --git a/Python/marshal.c b/Python/marshal.c index ef5a8d3840cd805..1897d700c055bd3 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -106,6 +106,7 @@ module marshal #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. */ typedef struct { FILE *fp; @@ -125,11 +126,32 @@ typedef struct { *(p)->ptr++ = (c); \ } while(0) +/* Report a failure of the underlying file. An earlier error is not + overwritten. */ +static void +w_file_error(WFILE *p) +{ + int saved_errno = errno; + if (p->error != WFERR_OK) { + return; + } + p->error = WFERR_EXCEPTION_SET; + if (PyErr_CheckSignals()) { + /* The signal handler has raised an exception. */ + return; + } + errno = saved_errno; + PyErr_SetFromErrno(PyExc_OSError); +} + static void w_flush(WFILE *p) { assert(p->fp != NULL); - fwrite(p->buf, 1, p->ptr - p->buf, p->fp); + size_t n = (size_t)(p->ptr - p->buf); + if (fwrite(p->buf, 1, n, p->fp) != n) { + w_file_error(p); + } p->ptr = p->buf; } @@ -182,7 +204,9 @@ w_string(const void *s, Py_ssize_t n, WFILE *p) } else { w_flush(p); - fwrite(s, 1, n, p->fp); + if (fwrite(s, 1, n, p->fp) != (size_t)n) { + w_file_error(p); + } } } else { @@ -782,11 +806,36 @@ 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. */ -/* Note that while the documentation states that this function - * can error, currently it never does. Setting an exception in - * this function should be regarded as an API-breaking change. - */ void PyMarshal_WriteLongToFile(long x, FILE *fp, int version) { @@ -800,6 +849,9 @@ 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); + } } void @@ -823,6 +875,9 @@ 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); + } } typedef struct { @@ -875,6 +930,14 @@ r_string(Py_ssize_t n, RFILE *p) if (!p->readable) { assert(p->fp != NULL); read = fread(p->buf, 1, n, p->fp); + if (read != n) { + assert(read < n); + int saved_errno = errno; + if (!PyErr_CheckSignals() && ferror(p->fp)) { + errno = saved_errno; + PyErr_SetFromErrno(PyExc_OSError); + } + } } else { PyObject *res, *mview; @@ -887,21 +950,26 @@ r_string(Py_ssize_t n, RFILE *p) return NULL; res = _PyObject_CallMethod(p->readable, &_Py_ID(readinto), "N", mview); - if (res != NULL) { - read = PyNumber_AsSsize_t(res, PyExc_ValueError); - Py_DECREF(res); + if (res == NULL) { + return NULL; + } + read = PyNumber_AsSsize_t(res, PyExc_ValueError); + Py_DECREF(res); + if (read == -1 && PyErr_Occurred()) { + return NULL; + } + if (read > n) { + PyErr_Format(PyExc_ValueError, + "read() returned too much data: " + "%zd bytes requested, %zd returned", + n, read); + return NULL; } } if (read != n) { if (!PyErr_Occurred()) { - if (read > n) - PyErr_Format(PyExc_ValueError, - "read() returned too much data: " - "%zd bytes requested, %zd returned", - n, read); - else - PyErr_SetString(PyExc_EOFError, - "EOF read where not expected"); + PyErr_SetString(PyExc_EOFError, + "EOF read where not expected"); } return NULL; } @@ -922,6 +990,15 @@ r_byte(RFILE *p) if (c != EOF) { return c; } + int saved_errno = errno; + if (PyErr_CheckSignals()) { + return EOF; + } + if (ferror(p->fp)) { + errno = saved_errno; + PyErr_SetFromErrno(PyExc_OSError); + return EOF; + } } else { const char *ptr = r_string(1, p); @@ -1850,8 +1927,18 @@ PyMarshal_ReadLastObjectFromFile(FILE *fp) if (filesize > 0 && filesize <= REASONABLE_FILE_LIMIT) { char* pBuf = (char *)PyMem_Malloc(filesize); if (pBuf != NULL) { + PyObject *v = NULL; size_t n = fread(pBuf, 1, (size_t)filesize, fp); - PyObject* v = PyMarshal_ReadObjectFromString(pBuf, n); + int saved_errno = errno; + if (!PyErr_CheckSignals()) { + if (ferror(fp)) { + errno = saved_errno; + PyErr_SetFromErrno(PyExc_OSError); + } + else { + v = PyMarshal_ReadObjectFromString(pBuf, n); + } + } PyMem_Free(pBuf); return v; } @@ -1938,24 +2025,7 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) } if (wf.error != WFERR_OK) { Py_XDECREF(wf.str); - switch (wf.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; - default: - case WFERR_UNMARSHALLABLE: - PyErr_SetString(PyExc_ValueError, - "unmarshallable object"); - break; - } + w_set_exception(&wf); return NULL; } return wf.str; _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
