Repository navigation
gh-155907: Raise OSError and KeyboardInterrupt when reading marshal data #155909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
serhiy-storchaka
merged 6 commits into
python:main
from
serhiy-storchaka:marshal-read-errors
Sep 4, 2026
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2c5579a
gh-155907: Raise OSError and KeyboardInterrupt when reading marshal data
serhiy-storchaka fecb4a0
Merge branch 'main' into marshal-read-errors
serhiy-storchaka 1a321fb
Merge branch 'main' into marshal-read-errors
serhiy-storchaka ed7d607
Detect I/O errors and signals when writing marshalled data
serhiy-storchaka 9b05276
Address review comments
serhiy-storchaka 01413ca
Rename WFERR_ERROR_SET to WFERR_EXCEPTION_SET
serhiy-storchaka File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 7 additions & 0 deletions
7
Misc/NEWS.d/next/C_API/2026-08-16-20-48-08.gh-issue-155907.Vb3xTn.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,6 +106,7 @@ module marshal | |
| #define WFERR_NESTEDTOODEEP 2 | ||
| #define WFERR_NOMEMORY 3 | ||
| #define WFERR_CODE_NOT_ALLOWED 4 | ||
| #define WFERR_ERROR_SET 5 /* An exception has already been raised. */ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should rename the constant |
||
|
|
||
| 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_ERROR_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_ERROR_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)) { | ||
|
vstinner marked this conversation as resolved.
Resolved
|
||
| 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); | ||
|
vstinner marked this conversation as resolved.
Resolved
|
||
| 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; | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
marshal.load() and marshal.dump() error handling also changes and should be documented in Doc/library/marshal.rst.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they are not affected. They do not use
FILE*based C API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked again your change, you're right and I'm wrong. marshal.load() and marshal.dump() are not affected.