gh-155907: Raise OSError and KeyboardInterrupt when reading marshal data

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,17 +77,15 @@ 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)
 
    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,15 +98,13 @@ 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)
 
    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``.
 

Doc/whatsnew/3.16.rst

@@ -909,6 +909,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
 -----------------
 

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)

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.

Modules/_testcapimodule.c

@@ -1435,9 +1435,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;
 }
 
@@ -1459,9 +1461,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;
 }
 

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;