From 0a0b69b0eb4b4cfd51216616b83e56d8201d7b13 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Sun, 23 Feb 2020 23:48:51 -0700 Subject: [PATCH 1/2] bpo-31718: Fix io.IncrementalNewlineDecoder SystemErrors and segfaults Co-authored-by: Oren Milman --- Lib/test/test_io.py | 10 +++++- .../2020-02-23-23-48-15.bpo-31718.sXko5e.rst | 3 ++ Modules/_io/textio.c | 31 +++++++++++++------ 3 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-02-23-23-48-15.bpo-31718.sXko5e.rst diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index c27dfd96bc00dd..efe730834b59a6 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -3871,7 +3871,15 @@ def test_translate(self): self.assertEqual(decoder.decode(b"\r\r\n"), "\r\r\n") class CIncrementalNewlineDecoderTest(IncrementalNewlineDecoderTest): - pass + @support.cpython_only + def test_uninitialized(self): + uninitialized = self.IncrementalNewlineDecoder.__new__( + self.IncrementalNewlineDecoder) + self.assertRaises(ValueError, uninitialized.decode, b'bar') + self.assertRaises(ValueError, uninitialized.getstate) + self.assertRaises(ValueError, uninitialized.setstate, (b'foo', 0)) + self.assertRaises(ValueError, uninitialized.reset) + class PyIncrementalNewlineDecoderTest(IncrementalNewlineDecoderTest): pass diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-02-23-23-48-15.bpo-31718.sXko5e.rst b/Misc/NEWS.d/next/Core and Builtins/2020-02-23-23-48-15.bpo-31718.sXko5e.rst new file mode 100644 index 00000000000000..dd96c9e20d8759 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-02-23-23-48-15.bpo-31718.sXko5e.rst @@ -0,0 +1,3 @@ +Raise :exc:`ValueError` instead of :exc:`SystemError` when methods of +uninitialized :class:`io.IncrementalNewlineDecoder` objects are called. +Patch by Oren Milman. diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 3a9ce93a5eb5eb..1ef54af4ea30a7 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -248,19 +248,19 @@ _io_IncrementalNewlineDecoder___init___impl(nldecoder_object *self, PyObject *errors) /*[clinic end generated code: output=fbd04d443e764ec2 input=89db6b19c6b126bf]*/ { - self->decoder = decoder; - Py_INCREF(decoder); - if (errors == NULL) { - self->errors = _PyUnicode_FromId(&PyId_strict); + Py_XSETREF(self->errors, _PyUnicode_FromId(&PyId_strict)); if (self->errors == NULL) return -1; } else { - self->errors = errors; + Py_XSETREF(self->errors, errors); } Py_INCREF(self->errors); + Py_INCREF(decoder); + Py_XSETREF(self->decoder, decoder); + self->translate = translate ? 1 : 0; self->seennl = 0; self->pendingcr = 0; @@ -295,6 +295,13 @@ check_decoded(PyObject *decoded) return 0; } +#define CHECK_INITIALIZED_DECODER(self) \ + if (self->errors == NULL) { \ + PyErr_SetString(PyExc_ValueError, \ + "IncrementalNewlineDecoder.__init__() not called"); \ + return NULL; \ + } + #define SEEN_CR 1 #define SEEN_LF 2 #define SEEN_CRLF 4 @@ -308,11 +315,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *myself, Py_ssize_t output_len; nldecoder_object *self = (nldecoder_object *) myself; - if (self->decoder == NULL) { - PyErr_SetString(PyExc_ValueError, - "IncrementalNewlineDecoder.__init__ not called"); - return NULL; - } + CHECK_INITIALIZED_DECODER(self); /* decode input (with the eventual \r from a previous pass) */ if (self->decoder != Py_None) { @@ -526,6 +529,8 @@ _io_IncrementalNewlineDecoder_getstate_impl(nldecoder_object *self) PyObject *buffer; unsigned long long flag; + CHECK_INITIALIZED_DECODER(self); + if (self->decoder != Py_None) { PyObject *state = PyObject_CallMethodNoArgs(self->decoder, _PyIO_str_getstate); @@ -570,6 +575,8 @@ _io_IncrementalNewlineDecoder_setstate(nldecoder_object *self, PyObject *buffer; unsigned long long flag; + CHECK_INITIALIZED_DECODER(self); + if (!PyTuple_Check(state)) { PyErr_SetString(PyExc_TypeError, "state argument must be a tuple"); return NULL; @@ -598,6 +605,8 @@ static PyObject * _io_IncrementalNewlineDecoder_reset_impl(nldecoder_object *self) /*[clinic end generated code: output=32fa40c7462aa8ff input=728678ddaea776df]*/ { + CHECK_INITIALIZED_DECODER(self); + self->seennl = 0; self->pendingcr = 0; if (self->decoder != Py_None) @@ -609,6 +618,8 @@ _io_IncrementalNewlineDecoder_reset_impl(nldecoder_object *self) static PyObject * incrementalnewlinedecoder_newlines_get(nldecoder_object *self, void *context) { + CHECK_INITIALIZED_DECODER(self); + switch (self->seennl) { case SEEN_CR: return PyUnicode_FromString("\r"); From 0d064f6aeb4fac8b9440ec7faa793d0a7bf754a9 Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Thu, 16 Jul 2020 08:43:18 -0600 Subject: [PATCH 2/2] Simplify __init__() --- Modules/_io/textio.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index af145575db09a6..1eca876e393024 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -251,14 +251,13 @@ _io_IncrementalNewlineDecoder___init___impl(nldecoder_object *self, /*[clinic end generated code: output=fbd04d443e764ec2 input=89db6b19c6b126bf]*/ { if (errors == NULL) { - Py_XSETREF(self->errors, _PyUnicode_FromId(&PyId_strict)); - if (self->errors == NULL) + errors = _PyUnicode_FromId(&PyId_strict); + if (errors == NULL) { return -1; + } } - else { - Py_XSETREF(self->errors, errors); - } - Py_INCREF(self->errors); + Py_INCREF(errors); + Py_XSETREF(self->errors, errors); Py_INCREF(decoder); Py_XSETREF(self->decoder, decoder);