From 2c5579a74acf03778d54aefe9c5b3e65fcba288e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 16 Aug 2026 20:48:40 +0300 Subject: [PATCH] gh-155907: Raise OSError and KeyboardInterrupt when reading marshal data Reading marshalled data from a FILE* did not check ferror() nor signals, so an I/O error or a Ctrl-C was reported as EOFError. Co-Authored-By: Claude Opus 5 (1M context) --- Doc/c-api/marshal.rst | 15 +++---- Lib/test/test_marshal.py | 13 ++++++ ...-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst | 3 ++ Python/marshal.c | 44 ++++++++++++++----- 4 files changed, 53 insertions(+), 22 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst diff --git a/Doc/c-api/marshal.rst b/Doc/c-api/marshal.rst index 668a163b2df5a11..8169d4a1e9bd380 100644 --- a/Doc/c-api/marshal.rst +++ b/Doc/c-api/marshal.rst @@ -52,8 +52,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, sets the appropriate exception and returns ``-1``. .. c:function:: int PyMarshal_ReadShortFromFile(FILE *file) @@ -62,8 +61,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, sets the appropriate exception and returns ``-1``. .. c:function:: PyObject* PyMarshal_ReadObjectFromFile(FILE *file) @@ -71,8 +69,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, sets the appropriate exception and returns ``NULL``. .. c:function:: PyObject* PyMarshal_ReadLastObjectFromFile(FILE *file) @@ -85,8 +82,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, sets the appropriate exception and returns ``NULL``. .. c:function:: PyObject* PyMarshal_ReadObjectFromString(const char *data, Py_ssize_t len) @@ -94,6 +90,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, sets the appropriate exception and returns ``NULL``. diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index 9c4d91c456dc5d9..7a8e88e33eceffd 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -784,6 +784,19 @@ 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) + 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/Core_and_Builtins/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst new file mode 100644 index 000000000000000..45250bb0056f8d4 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst @@ -0,0 +1,3 @@ +: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`. diff --git a/Python/marshal.c b/Python/marshal.c index 25353f6e6896249..7075e6c1563ad20 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -872,6 +872,12 @@ 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); + if (!PyErr_CheckSignals() && ferror(p->fp)) { + PyErr_SetFromErrno(PyExc_OSError); + } + } } else { PyObject *res, *mview; @@ -884,21 +890,23 @@ 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 > 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; } @@ -919,6 +927,10 @@ r_byte(RFILE *p) if (c != EOF) { return c; } + if (!PyErr_CheckSignals() && ferror(p->fp)) { + PyErr_SetFromErrno(PyExc_OSError); + return EOF; + } } else { const char *ptr = r_string(1, p); @@ -1841,8 +1853,16 @@ 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); + if (!PyErr_CheckSignals()) { + if (ferror(fp)) { + PyErr_SetFromErrno(PyExc_OSError); + } + else { + v = PyMarshal_ReadObjectFromString(pBuf, n); + } + } PyMem_Free(pBuf); return v; }