Skip to content

Commit 2c5579a

Browse files
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) <noreply@anthropic.com>
1 parent 70fdc96 commit 2c5579a

4 files changed

Lines changed: 53 additions & 22 deletions

File tree

Doc/c-api/marshal.rst

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ The following functions allow marshalled values to be read back in.
5252
for reading. Only a 32-bit value can be read in using this function,
5353
regardless of the native size of :c:expr:`long`.
5454
55-
On error, sets the appropriate exception (:exc:`EOFError`) and returns
56-
``-1``.
55+
On error, sets the appropriate exception and returns ``-1``.
5756
5857
5958
.. c:function:: int PyMarshal_ReadShortFromFile(FILE *file)
@@ -62,17 +61,15 @@ The following functions allow marshalled values to be read back in.
6261
for reading. Only a 16-bit value can be read in using this function,
6362
regardless of the native size of :c:expr:`short`.
6463
65-
On error, sets the appropriate exception (:exc:`EOFError`) and returns
66-
``-1``.
64+
On error, sets the appropriate exception and returns ``-1``.
6765
6866
6967
.. c:function:: PyObject* PyMarshal_ReadObjectFromFile(FILE *file)
7068
7169
Return a Python object from the data stream in a :c:expr:`FILE*` opened for
7270
reading.
7371
74-
On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError`
75-
or :exc:`TypeError`) and returns ``NULL``.
72+
On error, sets the appropriate exception and returns ``NULL``.
7673
7774
7875
.. c:function:: PyObject* PyMarshal_ReadLastObjectFromFile(FILE *file)
@@ -85,15 +82,13 @@ The following functions allow marshalled values to be read back in.
8582
file. Only use this variant if you are certain that you won't be reading
8683
anything else from the file.
8784
88-
On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError`
89-
or :exc:`TypeError`) and returns ``NULL``.
85+
On error, sets the appropriate exception and returns ``NULL``.
9086
9187
9288
.. c:function:: PyObject* PyMarshal_ReadObjectFromString(const char *data, Py_ssize_t len)
9389
9490
Return a Python object from the data stream in a byte buffer
9591
containing *len* bytes pointed to by *data*.
9692
97-
On error, sets the appropriate exception (:exc:`EOFError`, :exc:`ValueError`
98-
or :exc:`TypeError`) and returns ``NULL``.
93+
On error, sets the appropriate exception and returns ``NULL``.
9994

Lib/test/test_marshal.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,19 @@ def test_slice(self):
784784
@unittest.skipUnless(_testcapi, 'requires _testcapi')
785785
class CAPI_TestCase(unittest.TestCase, HelperMixin):
786786

787+
def test_read_from_file_error(self):
788+
# A read error is reported as OSError, not EOFError.
789+
# A directory cannot be read (on some platforms it cannot even
790+
# be opened, which is reported as OSError as well).
791+
os.mkdir(os_helper.TESTFN)
792+
self.addCleanup(os_helper.rmdir, os_helper.TESTFN)
793+
for func in (_testcapi.pymarshal_read_short_from_file,
794+
_testcapi.pymarshal_read_long_from_file,
795+
_testcapi.pymarshal_read_object_from_file,
796+
_testcapi.pymarshal_read_last_object_from_file):
797+
with self.subTest(func=func.__name__):
798+
self.assertRaises(OSError, func, os_helper.TESTFN)
799+
787800
def test_write_long_to_file(self):
788801
for v in range(marshal.version + 1):
789802
_testcapi.pymarshal_write_long_to_file(0x12345678, os_helper.TESTFN, v)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:c:func:`PyMarshal_ReadObjectFromFile` and other functions reading marshalled
2+
data from a :c:expr:`FILE*` now raise :exc:`OSError` for I/O errors and
3+
:exc:`KeyboardInterrupt` for interrupted reading, instead of :exc:`EOFError`.

Python/marshal.c

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,12 @@ r_string(Py_ssize_t n, RFILE *p)
872872
if (!p->readable) {
873873
assert(p->fp != NULL);
874874
read = fread(p->buf, 1, n, p->fp);
875+
if (read != n) {
876+
assert(read < n);
877+
if (!PyErr_CheckSignals() && ferror(p->fp)) {
878+
PyErr_SetFromErrno(PyExc_OSError);
879+
}
880+
}
875881
}
876882
else {
877883
PyObject *res, *mview;
@@ -884,21 +890,23 @@ r_string(Py_ssize_t n, RFILE *p)
884890
return NULL;
885891

886892
res = _PyObject_CallMethod(p->readable, &_Py_ID(readinto), "N", mview);
887-
if (res != NULL) {
888-
read = PyNumber_AsSsize_t(res, PyExc_ValueError);
889-
Py_DECREF(res);
893+
if (res == NULL) {
894+
return NULL;
895+
}
896+
read = PyNumber_AsSsize_t(res, PyExc_ValueError);
897+
Py_DECREF(res);
898+
if (read > n) {
899+
PyErr_Format(PyExc_ValueError,
900+
"read() returned too much data: "
901+
"%zd bytes requested, %zd returned",
902+
n, read);
903+
return NULL;
890904
}
891905
}
892906
if (read != n) {
893907
if (!PyErr_Occurred()) {
894-
if (read > n)
895-
PyErr_Format(PyExc_ValueError,
896-
"read() returned too much data: "
897-
"%zd bytes requested, %zd returned",
898-
n, read);
899-
else
900-
PyErr_SetString(PyExc_EOFError,
901-
"EOF read where not expected");
908+
PyErr_SetString(PyExc_EOFError,
909+
"EOF read where not expected");
902910
}
903911
return NULL;
904912
}
@@ -919,6 +927,10 @@ r_byte(RFILE *p)
919927
if (c != EOF) {
920928
return c;
921929
}
930+
if (!PyErr_CheckSignals() && ferror(p->fp)) {
931+
PyErr_SetFromErrno(PyExc_OSError);
932+
return EOF;
933+
}
922934
}
923935
else {
924936
const char *ptr = r_string(1, p);
@@ -1841,8 +1853,16 @@ PyMarshal_ReadLastObjectFromFile(FILE *fp)
18411853
if (filesize > 0 && filesize <= REASONABLE_FILE_LIMIT) {
18421854
char* pBuf = (char *)PyMem_Malloc(filesize);
18431855
if (pBuf != NULL) {
1856+
PyObject *v = NULL;
18441857
size_t n = fread(pBuf, 1, (size_t)filesize, fp);
1845-
PyObject* v = PyMarshal_ReadObjectFromString(pBuf, n);
1858+
if (!PyErr_CheckSignals()) {
1859+
if (ferror(fp)) {
1860+
PyErr_SetFromErrno(PyExc_OSError);
1861+
}
1862+
else {
1863+
v = PyMarshal_ReadObjectFromString(pBuf, n);
1864+
}
1865+
}
18461866
PyMem_Free(pBuf);
18471867
return v;
18481868
}

0 commit comments

Comments
 (0)