[3.6] bpo-31243: Fixed PyArg_ParseTuple failure checks. (GH-3171) (#3233)

(cherry picked from commit ba7d736)

Author: orenmn

Lib/test/test_io.py

@@ -3191,6 +3191,26 @@ def _make_illegal_wrapper():
  t = _make_illegal_wrapper()
  self.assertRaises(TypeError, t.read)
 
+ # Issue 31243: calling read() while the return value of decoder's
+ # getstate() is invalid should neither crash the interpreter nor
+ # raise a SystemError.
+ def _make_very_illegal_wrapper(getstate_ret_val):
+ class BadDecoder:
+ def getstate(self):
+ return getstate_ret_val
+ def _get_bad_decoder(dummy):
+ return BadDecoder()
+ quopri = codecs.lookup("quopri")
+ with support.swap_attr(quopri, 'incrementaldecoder',
+ _get_bad_decoder):
+ return _make_illegal_wrapper()
+ t = _make_very_illegal_wrapper(42)
+ self.assertRaises(TypeError, t.read, 42)
+ t = _make_very_illegal_wrapper(())
+ self.assertRaises(TypeError, t.read, 42)
+ t = _make_very_illegal_wrapper((1, 2))
+ self.assertRaises(TypeError, t.read, 42)
+
  def _check_create_at_shutdown(self, **kwargs):
  # Issue #20037: creating a TextIOWrapper at shutdown
  # shouldn't crash the interpreter.

Misc/NEWS.d/next/Core and Builtins/2017-08-24-13-34-49.bpo-31243.dRJzqR.rst

@@ -0,0 +1,2 @@
+Fix a crash in some methods of `io.TextIOWrapper`, when the decoder's state
+is invalid. Patch by Oren Milman.

Modules/_io/textio.c

@@ -1457,15 +1457,23 @@ textiowrapper_read_chunk(textio *self, Py_ssize_t size_hint)
  /* Given this, we know there was a valid snapshot point
  * len(dec_buffer) bytes ago with decoder state (b'', dec_flags).
  */
- if (PyArg_ParseTuple(state, "OO", &dec_buffer, &dec_flags) < 0) {
+ if (!PyTuple_Check(state)) {
+ PyErr_SetString(PyExc_TypeError,
+ "illegal decoder state");
+ Py_DECREF(state);
+ return -1;
+ }
+ if (!PyArg_ParseTuple(state,
+ "OO;illegal decoder state", &dec_buffer, &dec_flags))
+ {
  Py_DECREF(state);
  return -1;
  }
 
  if (!PyBytes_Check(dec_buffer)) {
  PyErr_Format(PyExc_TypeError,
- "decoder getstate() should have returned a bytes "
- "object, not '%.200s'",
+ "illegal decoder state: the first item should be a "
+ "bytes object, not '%.200s'",
  Py_TYPE(dec_buffer)->tp_name);
  Py_DECREF(state);
  return -1;
@@ -2349,8 +2357,8 @@ _io_TextIOWrapper_tell_impl(textio *self)
  } \
  if (!PyBytes_Check(dec_buffer)) { \
  PyErr_Format(PyExc_TypeError, \
- "decoder getstate() should have returned a bytes " \
- "object, not '%.200s'", \
+ "illegal decoder state: the first item should be a " \
+ "bytes object, not '%.200s'", \
  Py_TYPE(dec_buffer)->tp_name); \
  Py_DECREF(_state); \
  goto fail; \

Modules/_testcapimodule.c

@@ -870,8 +870,9 @@ test_L_code(PyObject *self)
  PyTuple_SET_ITEM(tuple, 0, num);
 
  value = -1;
- if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0)
+ if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) {
  return NULL;
+ }
  if (value != 42)
  return raiseTestError("test_L_code",
  "L code returned wrong value for long 42");
@@ -884,8 +885,9 @@ test_L_code(PyObject *self)
  PyTuple_SET_ITEM(tuple, 0, num);
 
  value = -1;
- if (PyArg_ParseTuple(tuple, "L:test_L_code", &value) < 0)
+ if (!PyArg_ParseTuple(tuple, "L:test_L_code", &value)) {
  return NULL;
+ }
  if (value != 42)
  return raiseTestError("test_L_code",
  "L code returned wrong value for int 42");
@@ -1202,8 +1204,9 @@ test_k_code(PyObject *self)
  PyTuple_SET_ITEM(tuple, 0, num);
 
  value = 0;
- if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0)
+ if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) {
  return NULL;
+ }
  if (value != ULONG_MAX)
  return raiseTestError("test_k_code",
  "k code returned wrong value for long 0xFFF...FFF");
@@ -1221,8 +1224,9 @@ test_k_code(PyObject *self)
  PyTuple_SET_ITEM(tuple, 0, num);
 
  value = 0;
- if (PyArg_ParseTuple(tuple, "k:test_k_code", &value) < 0)
+ if (!PyArg_ParseTuple(tuple, "k:test_k_code", &value)) {
  return NULL;
+ }
  if (value != (unsigned long)-0x42)
  return raiseTestError("test_k_code",
  "k code returned wrong value for long -0xFFF..000042");
@@ -1560,11 +1564,13 @@ test_s_code(PyObject *self)
  /* These two blocks used to raise a TypeError:
  * "argument must be string without null bytes, not str"
  */
- if (PyArg_ParseTuple(tuple, "s:test_s_code1", &value) < 0)
- return NULL;
+ if (!PyArg_ParseTuple(tuple, "s:test_s_code1", &value)) {
+ return NULL;
+ }
 
- if (PyArg_ParseTuple(tuple, "z:test_s_code2", &value) < 0)
- return NULL;
+ if (!PyArg_ParseTuple(tuple, "z:test_s_code2", &value)) {
+ return NULL;
+ }
 
  Py_DECREF(tuple);
  Py_RETURN_NONE;
@@ -1666,14 +1672,16 @@ test_u_code(PyObject *self)
  PyTuple_SET_ITEM(tuple, 0, obj);
 
  value = 0;
- if (PyArg_ParseTuple(tuple, "u:test_u_code", &value) < 0)
+ if (!PyArg_ParseTuple(tuple, "u:test_u_code", &value)) {
  return NULL;
+ }
  if (value != PyUnicode_AS_UNICODE(obj))
  return raiseTestError("test_u_code",
  "u code returned wrong value for u'test'");
  value = 0;
- if (PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len) < 0)
+ if (!PyArg_ParseTuple(tuple, "u#:test_u_code", &value, &len)) {
  return NULL;
+ }
  if (value != PyUnicode_AS_UNICODE(obj) ||
  len != PyUnicode_GET_SIZE(obj))
  return raiseTestError("test_u_code",
@@ -1706,8 +1714,9 @@ test_Z_code(PyObject *self)
  value2 = PyUnicode_AS_UNICODE(obj);
 
  /* Test Z for both values */
- if (PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2) < 0)
+ if (!PyArg_ParseTuple(tuple, "ZZ:test_Z_code", &value1, &value2)) {
  return NULL;
+ }
  if (value1 != PyUnicode_AS_UNICODE(obj))
  return raiseTestError("test_Z_code",
  "Z code returned wrong value for 'test'");
@@ -1721,9 +1730,11 @@ test_Z_code(PyObject *self)
  len2 = -1;
 
  /* Test Z# for both values */
- if (PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1,
- &value2, &len2) < 0)
+ if (!PyArg_ParseTuple(tuple, "Z#Z#:test_Z_code", &value1, &len1,
+ &value2, &len2))
+ {
  return NULL;
+ }
  if (value1 != PyUnicode_AS_UNICODE(obj) ||
  len1 != PyUnicode_GET_SIZE(obj))
  return raiseTestError("test_Z_code",
@@ -2028,17 +2039,19 @@ test_empty_argparse(PyObject *self)
  tuple = PyTuple_New(0);
  if (!tuple)
  return NULL;
- if ((result = PyArg_ParseTuple(tuple, "|:test_empty_argparse")) < 0)
+ if (!(result = PyArg_ParseTuple(tuple, "|:test_empty_argparse"))) {
  goto done;
+ }
  dict = PyDict_New();
  if (!dict)
  goto done;
  result = PyArg_ParseTupleAndKeywords(tuple, dict, "|:test_empty_argparse", kwlist);
  done:
  Py_DECREF(tuple);
  Py_XDECREF(dict);
- if (result < 0)
+ if (!result) {
  return NULL;
+ }
  else {
  Py_RETURN_NONE;
  }
@@ -3573,8 +3586,9 @@ test_raise_signal(PyObject* self, PyObject *args)
 {
  int signum, err;
 
- if (PyArg_ParseTuple(args, "i:raise_signal", &signum) < 0)
+ if (!PyArg_ParseTuple(args, "i:raise_signal", &signum)) {
  return NULL;
+ }
 
  err = raise(signum);
  if (err)