-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
gh-127182: Fix io.StringIO.__setstate__
crash when None
is the first value
#127219
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Fix :meth:`!io.StringIO.__setstate__` crash, when :const:`None` was passed as | ||
the first value. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -908,23 +908,25 @@ _io_StringIO___setstate___impl(stringio *self, PyObject *state) | |
once by __init__. So we do not take any chance and replace object's | ||
buffer completely. */ | ||
{ | ||
PyObject *item; | ||
Py_UCS4 *buf; | ||
Py_ssize_t bufsize; | ||
|
||
item = PyTuple_GET_ITEM(state, 0); | ||
buf = PyUnicode_AsUCS4Copy(item); | ||
if (buf == NULL) | ||
return NULL; | ||
bufsize = PyUnicode_GET_LENGTH(item); | ||
PyObject *item = PyTuple_GET_ITEM(state, 0); | ||
if (PyUnicode_Check(item)) { | ||
Py_UCS4 *buf = PyUnicode_AsUCS4Copy(item); | ||
if (buf == NULL) | ||
return NULL; | ||
Py_ssize_t bufsize = PyUnicode_GET_LENGTH(item); | ||
|
||
if (resize_buffer(self, bufsize) < 0) { | ||
if (resize_buffer(self, bufsize) < 0) { | ||
PyMem_Free(buf); | ||
return NULL; | ||
} | ||
memcpy(self->buf, buf, bufsize * sizeof(Py_UCS4)); | ||
PyMem_Free(buf); | ||
return NULL; | ||
self->string_size = bufsize; | ||
} | ||
else { | ||
assert(item == Py_None); | ||
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. As a side note, should we encourage using 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. In this case, Py_IsNone() can be used ;-) 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. I would prefer to keep it as |
||
self->string_size = 0; | ||
} | ||
memcpy(self->buf, buf, bufsize * sizeof(Py_UCS4)); | ||
PyMem_Free(buf); | ||
self->string_size = bufsize; | ||
} | ||
|
||
/* Set carefully the position value. Alternatively, we could use the seek | ||
|
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.
It may be worth it to check item value at runtime, and fail with an assertion if it's not None.
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 don't think that it is, because
__init__
call above checks that. I don't think that it would be possible to trigger this error here.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.
Oh right, it makes sense. I didn't see the
_io_StringIO___init__()
call.