8000 bpo-31271: fix an assertion failure in io.TextIOWrapper.write by orenmn · Pull Request #3201 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

bpo-31271: fix an assertion failure in io.TextIOWrapper.write #3201

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
merged 3 commits into from
Aug 25, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
init commit
  • Loading branch information
orenmn committed Aug 24, 2017
commit b89786b8fc80fe83bae323c2de42bf293f989f89
14 changes: 14 additions & 0 deletions Lib/test/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -3225,6 +3225,20 @@ def test_read_nonbytes(self):
t = self.TextIOWrapper(self.StringIO('a'))
self.assertRaises(TypeError, t.read)

def test_illegal_encoder(self):
# Issue 31271: Calling write() while the return value of encoder's
# encode() is invalid shouldn't cause an assertion failure.
class BadEncoder():
def encode(self, dummy):
return 42
def _get_bad_encoder(dummy):
return BadEncoder()
quopri = codecs.lookup("quopri")
with support.swap_attr(quopri, '_is_text_encoding', True), \
support.swap_attr(quopri, 'incrementalencoder', _get_bad_encoder):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is monkey-patching incrementalencoder needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understood your question.

  • if we remove it, then the assertion won't fail (even without the issue's patch).
  • if we just overwrite quopri.incrementalencoder, without restoring it, the assertion would fail (without the issue's patch), but I am not sure about the effects of not restoring it (on Python with the issue's patch), when Python keeps running.
  • can we reproduce the assertion failure without changing quopri.incrementalencoder? I don't know. I haven't looked for any other way, but I can do that, if you think it's worth it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if use an existing encoding that produces non-bytes? E.g. "rot13"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed. I would update the patch.

t = io.TextIOWrapper(io.BytesIO(b'foo'), encoding="quopri")
self.assertRaises(TypeError, t.write, 'bar')

def test_illegal_decoder(self):
# Issue #17106
# Bypass the early encoding check added in issue 20404
Expand Down
8 changes: 8 additions & 0 deletions Modules/_io/textio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1379,6 +1379,14 @@ _io_TextIOWrapper_write_impl(textio *self, PyObject *text)
Py_DECREF(text);
if (b == NULL)
return NULL;
if (!PyBytes_Check(b)) {
PyErr_Format(PyExc_TypeError,
"encode() should have returned a bytes object, not "
"'%.200s'",
Py_TYPE(b)->tp_name);
Py_DECREF(b);
return NULL;
}

if (self->pending_bytes == NULL) {
self->pending_bytes = PyList_New(0);
Expand Down
0