From ff52f6bbbdc48e68f9438efe2fb58123aeb09b69 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 27 Aug 2017 20:18:37 +0300 Subject: [PATCH 1/7] init commit --- Lib/test/test_warnings/__init__.py | 27 +++++++++++++++++++++++++++ Python/_warnings.c | 12 ++++++++++++ 2 files changed, 39 insertions(+) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 69623c40f4677c..8bfee912dc12c9 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -794,6 +794,33 @@ def test_stderr_none(self): self.assertNotIn(b'Warning!', stderr) self.assertNotIn(b'Error', stderr) + def test_warn_explicit(self): + # Issue #31285: warn_explicit() should neither raise a SystemError nor + # cause an assertion failure, in case the return value of get_source() + # is invalid. + def _get_bad_loader(splitlines_ret_val): + class BadLoader: + def get_source(self, fullname): + class BadSource: + def splitlines(self): + return splitlines_ret_val + return BadSource() + return BadLoader() + self.assertRaises(TypeError, self.module.warn_explicit, + 'foo', UserWarning, 'bar', 42, + module_globals={'__loader__': _get_bad_loader(42), + '__name__': 'foobar'}) + show = self.module._showwarnmsg + try: + del self.module._showwarnmsg + self.assertRaises(TypeError, self.module.warn_explicit, + 'foo', ArithmeticError, 'bar', 1, + module_globals={ + '__loader__': _get_bad_loader([42]), + '__name__': 'foobar'}) + finally: + self.module._showwarnmsg = show + class WarningsDisplayTests(BaseTest): diff --git a/Python/_warnings.c b/Python/_warnings.c index add72e4ebb38af..8c47dd607a04bc 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -890,6 +890,12 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds) Py_DECREF(source); if (!source_list) return NULL; + if (!PyList_Check(source_list)) { + PyErr_SetString(PyExc_TypeError, + "loader get_source() return value is invalid"); + Py_DECREF(source_list); + return NULL; + } /* Get the source line. */ source_line = PyList_GetItem(source_list, lineno-1); @@ -897,6 +903,12 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds) Py_DECREF(source_list); return NULL; } + if (!PyUnicode_Check(source_line)) { + PyErr_SetString(PyExc_TypeError, + "loader get_source() return value is invalid"); + Py_DECREF(source_list); + return NULL; + } /* Handle the warning. */ returned = warn_explicit(category, message, filename, lineno, module, From 252647d48ec36386d51f0ce7b6f3940a40a3f315 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 27 Aug 2017 21:19:25 +0300 Subject: [PATCH 2/7] add a NEWS.d item --- .../Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst new file mode 100644 index 00000000000000..78aec0d8d082fa --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst @@ -0,0 +1,2 @@ +Fix an assertion failure in `warnings.warn_explicit`, when the return value +of the received loader's get_source() is invalid. Patch by Oren Milman. From d4dbe6dca56fa5dbef63f8449e805b87e774fe2c Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Mon, 28 Aug 2017 13:06:37 +0300 Subject: [PATCH 3/7] replace the C equivalent of source.splitlines() with str.splitlines(source) --- Lib/test/test_warnings/__init__.py | 6 +++--- Python/_warnings.c | 16 ++-------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 8bfee912dc12c9..b737eed8fd1d8b 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -801,19 +801,19 @@ def test_warn_explicit(self): def _get_bad_loader(splitlines_ret_val): class BadLoader: def get_source(self, fullname): - class BadSource: + class BadSource(str): def splitlines(self): return splitlines_ret_val return BadSource() return BadLoader() - self.assertRaises(TypeError, self.module.warn_explicit, + self.assertRaises(IndexError, self.module.warn_explicit, 'foo', UserWarning, 'bar', 42, module_globals={'__loader__': _get_bad_loader(42), '__name__': 'foobar'}) show = self.module._showwarnmsg try: del self.module._showwarnmsg - self.assertRaises(TypeError, self.module.warn_explicit, + self.assertRaises(IndexError, self.module.warn_explicit, 'foo', ArithmeticError, 'bar', 1, module_globals={ '__loader__': _get_bad_loader([42]), diff --git a/Python/_warnings.c b/Python/_warnings.c index 8c47dd607a04bc..a2a365563f2790 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -884,18 +884,12 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds) } /* Split the source into lines. */ - source_list = PyObject_CallMethodObjArgs(source, + source_list = PyObject_CallMethodObjArgs((PyObject *)&PyUnicode_Type, PyId_splitlines.object, - NULL); + source, NULL); Py_DECREF(source); if (!source_list) return NULL; - if (!PyList_Check(source_list)) { - PyErr_SetString(PyExc_TypeError, - "loader get_source() return value is invalid"); - Py_DECREF(source_list); - return NULL; - } /* Get the source line. */ source_line = PyList_GetItem(source_list, lineno-1); @@ -903,12 +897,6 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds) Py_DECREF(source_list); return NULL; } - if (!PyUnicode_Check(source_line)) { - PyErr_SetString(PyExc_TypeError, - "loader get_source() return value is invalid"); - Py_DECREF(source_list); - return NULL; - } /* Handle the warning. */ returned = warn_explicit(category, message, filename, lineno, module, From 9eb55b5a63ce139f4bdb6366d8a7955033f571eb Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Mon, 28 Aug 2017 13:11:36 +0300 Subject: [PATCH 4/7] fix NEWS.d item --- .../Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst index 78aec0d8d082fa..61f2c4eff65a71 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-21-18-30.bpo-31285.7lzaKV.rst @@ -1,2 +1,3 @@ Fix an assertion failure in `warnings.warn_explicit`, when the return value -of the received loader's get_source() is invalid. Patch by Oren Milman. +of the received loader's get_source() has a bad splitlines() method. Patch +by Oren Milman. From 5674174e3254c59c7d355a623f788ff0237215c6 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Wed, 30 Aug 2017 15:18:18 +0300 Subject: [PATCH 5/7] improve test method and its name, and change to PyUnicode_Splitlines() --- Lib/test/test_warnings/__init__.py | 31 ++++++++++++++++-------------- Python/_warnings.c | 4 +--- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index b737eed8fd1d8b..8d89d7313d7c48 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -794,30 +794,33 @@ def test_stderr_none(self): self.assertNotIn(b'Warning!', stderr) self.assertNotIn(b'Error', stderr) - def test_warn_explicit(self): - # Issue #31285: warn_explicit() should neither raise a SystemError nor - # cause an assertion failure, in case the return value of get_source() - # is invalid. + def test_issue31285(self): + # warn_explicit() should neither raise a SystemError nor cause an + # assertion failure, in case the return value of get_source() has a + # bad splitlines() method. def _get_bad_loader(splitlines_ret_val): class BadLoader: def get_source(self, fullname): class BadSource(str): def splitlines(self): return splitlines_ret_val - return BadSource() + return BadSource('spam') return BadLoader() - self.assertRaises(IndexError, self.module.warn_explicit, - 'foo', UserWarning, 'bar', 42, - module_globals={'__loader__': _get_bad_loader(42), - '__name__': 'foobar'}) + with support.captured_stderr() as stderr: + self.module.warn_explicit( + 'foo', UserWarning, 'bar', 1, + module_globals={'__loader__': _get_bad_loader(42), + '__name__': 'foobar'}) + self.assertIn('bar:1: UserWarning: foo', stderr.getvalue()) show = self.module._showwarnmsg try: del self.module._showwarnmsg - self.assertRaises(IndexError, self.module.warn_explicit, - 'foo', ArithmeticError, 'bar', 1, - module_globals={ - '__loader__': _get_bad_loader([42]), - '__name__': 'foobar'}) + with support.captured_stderr() as stderr: + self.module.warn_explicit( + 'foo', ArithmeticError, 'bar', 1, + module_globals={'__loader__': _get_bad_loader([42]), + '__name__': 'foobar'}) + self.assertIn('bar:1: ArithmeticError: foo', stderr.getvalue()) finally: self.module._showwarnmsg = show diff --git a/Python/_warnings.c b/Python/_warnings.c index a2a365563f2790..b678c51643b106 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -884,9 +884,7 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds) } /* Split the source into lines. */ - source_list = PyObject_CallMethodObjArgs((PyObject *)&PyUnicode_Type, - PyId_splitlines.object, - source, NULL); + source_list = PyUnicode_Splitlines(source, 0); Py_DECREF(source); if (!source_list) return NULL; From 6fe73a4cdd38a2126a95e9aa696d1f3d5f90c1c7 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 24 Sep 2017 14:57:33 +0300 Subject: [PATCH 6/7] remove redundant underscore --- Lib/test/test_warnings/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 583aefd5a8cf8b..28e201a057ecf3 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -798,7 +798,7 @@ def test_issue31285(self): # warn_explicit() should neither raise a SystemError nor cause an # assertion failure, in case the return value of get_source() has a # bad splitlines() method. - def _get_bad_loader(splitlines_ret_val): + def get_bad_loader(splitlines_ret_val): class BadLoader: def get_source(self, fullname): class BadSource(str): @@ -809,7 +809,7 @@ def splitlines(self): with support.captured_stderr() as stderr: self.module.warn_explicit( 'foo', UserWarning, 'bar', 1, - module_globals={'__loader__': _get_bad_loader(42), + module_globals={'__loader__': get_bad_loader(42), '__name__': 'foobar'}) self.assertIn('bar:1: UserWarning: foo', stderr.getvalue()) show = self.module._showwarnmsg @@ -818,7 +818,7 @@ def splitlines(self): with support.captured_stderr() as stderr: self.module.warn_explicit( 'foo', ArithmeticError, 'bar', 1, - module_globals={'__loader__': _get_bad_loader([42]), + module_globals={'__loader__': get_bad_loader([42]), '__name__': 'foobar'}) self.assertIn('bar:1: ArithmeticError: foo', stderr.getvalue()) finally: From 71da7df1c0e0627d1afb836f5edd9f5a8e8f6255 Mon Sep 17 00:00:00 2001 From: Oren Milman Date: Sun, 24 Sep 2017 17:32:21 +0300 Subject: [PATCH 7/7] fix the test, and make it clearer --- Lib/test/test_warnings/__init__.py | 36 +++++++++++++++++------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_warnings/__init__.py b/Lib/test/test_warnings/__init__.py index 28e201a057ecf3..f27ee6ef681fca 100644 --- a/Lib/test/test_warnings/__init__.py +++ b/Lib/test/test_warnings/__init__.py @@ -806,23 +806,29 @@ def splitlines(self): return splitlines_ret_val return BadSource('spam') return BadLoader() - with support.captured_stderr() as stderr: - self.module.warn_explicit( - 'foo', UserWarning, 'bar', 1, - module_globals={'__loader__': get_bad_loader(42), - '__name__': 'foobar'}) - self.assertIn('bar:1: UserWarning: foo', stderr.getvalue()) - show = self.module._showwarnmsg - try: - del self.module._showwarnmsg + + wmod = self.module + with original_warnings.catch_warnings(module=wmod): + wmod.filterwarnings('default', category=UserWarning) + with support.captured_stderr() as stderr: - self.module.warn_explicit( - 'foo', ArithmeticError, 'bar', 1, - module_globals={'__loader__': get_bad_loader([42]), + wmod.warn_explicit( + 'foo', UserWarning, 'bar', 1, + module_globals={'__loader__': get_bad_loader(42), '__name__': 'foobar'}) - self.assertIn('bar:1: ArithmeticError: foo', stderr.getvalue()) - finally: - self.module._showwarnmsg = show + self.assertIn('UserWarning: foo', stderr.getvalue()) + + show = wmod._showwarnmsg + try: + del wmod._showwarnmsg + with support.captured_stderr() as stderr: + wmod.warn_explicit( + 'eggs', UserWarning, 'bar', 1, + module_globals={'__loader__': get_bad_loader([42]), + '__name__': 'foobar'}) + self.assertIn('UserWarning: eggs', stderr.getvalue()) + finally: + wmod._showwarnmsg = show @support.cpython_only def test_issue31411(self):