bpo-37467: Fix PyErr_Display() for bytes filename (GH-14504) (GH-14515)

vstinner · web-flow · commit 8cbffc4d96d1 · 2019-07-01T17:41:38.000+02:00
Fix sys.excepthook() and PyErr_Display() if a filename is a bytes string. For example, for a SyntaxError exception where the filename attribute is a bytes string. Cleanup also test_sys: * Sort imports. * Rename numruns global var to INTERN_NUMRUNS. * Add DisplayHookTest and ExceptHookTest test case classes. * Don't save/restore sys.stdout and sys.displayhook using setUp()/tearDown(): do it in each test method. * Test error case (call hook with no argument) after the success case. (cherry picked from commit f9b7457)
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
@@ -1,82 +1,103 @@
-import unittest, test.support
+from test import support
 from test.support.script_helper import assert_python_ok, assert_python_failure
-import sys, io, os
+import builtins
+import codecs
+import gc
+import locale
+import operator
+import os
 import struct
 import subprocess
+import sys
+import sysconfig
+import test.support
 import textwrap
+import unittest
 import warnings
-import operator
-import codecs
-import gc
-import sysconfig
-import locale
-import threading
+
 
 # count the number of test runs, used to create unique
 # strings to intern in test_intern()
-numruns = 0
+INTERN_NUMRUNS = 0
 
 
-class SysModuleTest(unittest.TestCase):
+class DisplayHookTest(unittest.TestCase):
 
-    def setUp(self):
-        self.orig_stdout = sys.stdout
-        self.orig_stderr = sys.stderr
-        self.orig_displayhook = sys.displayhook
+    def test_original_displayhook(self):
+        dh = sys.__displayhook__
 
-    def tearDown(self):
-        sys.stdout = self.orig_stdout
-        sys.stderr = self.orig_stderr
-        sys.displayhook = self.orig_displayhook
-        test.support.reap_children()
+        with support.captured_stdout() as out:
+            dh(42)
 
-    def test_original_displayhook(self):
-        import builtins
-        out = io.StringIO()
-        sys.stdout = out
+        self.assertEqual(out.getvalue(), "42\n")
+        self.assertEqual(builtins._, 42)
 
-        dh = sys.__displayhook__
+        del builtins._
 
-        self.assertRaises(TypeError, dh)
-        if hasattr(builtins, "_"):
-            del builtins._
+        with support.captured_stdout() as out:
+            dh(None)
 
-        dh(None)
         self.assertEqual(out.getvalue(), "")
         self.assertTrue(not hasattr(builtins, "_"))
-        dh(42)
-        self.assertEqual(out.getvalue(), "42\n")
-        self.assertEqual(builtins._, 42)
 
-        del sys.stdout
-        self.assertRaises(RuntimeError, dh, 42)
+        # sys.displayhook() requires arguments
+        self.assertRaises(TypeError, dh)
+
+        stdout = sys.stdout
+        try:
+            del sys.stdout
+            self.assertRaises(RuntimeError, dh, 42)
+        finally:
+            sys.stdout = stdout
 
     def test_lost_displayhook(self):
-        del sys.displayhook
-        code = compile("42", "<string>", "single")
-        self.assertRaises(RuntimeError, eval, code)
+        displayhook = sys.displayhook
+        try:
+            del sys.displayhook
+            code = compile("42", "<string>", "single")
+            self.assertRaises(RuntimeError, eval, code)
+        finally:
+            sys.displayhook = displayhook
 
     def test_custom_displayhook(self):
         def baddisplayhook(obj):
             raise ValueError
-        sys.displayhook = baddisplayhook
-        code = compile("42", "<string>", "single")
-        self.assertRaises(ValueError, eval, code)
 
-    def test_original_excepthook(self):
-        err = io.StringIO()
-        sys.stderr = err
+        with support.swap_attr(sys, 'displayhook', baddisplayhook):
+            code = compile("42", "<string>", "single")
+            self.assertRaises(ValueError, eval, code)
+
 
-        eh = sys.__excepthook__
+class ExceptHookTest(unittest.TestCase):
 
-        self.assertRaises(TypeError, eh)
+    def test_original_excepthook(self):
         try:
             raise ValueError(42)
         except ValueError as exc:
-            eh(*sys.exc_info())
+            with support.captured_stderr() as err:
+                sys.__excepthook__(*sys.exc_info())
 
         self.assertTrue(err.getvalue().endswith("ValueError: 42\n"))
 
+        self.assertRaises(TypeError, sys.__excepthook__)
+
+    def test_excepthook_bytes_filename(self):
+        # bpo-37467: sys.excepthook() must not crash if a filename
+        # is a bytes string
+        with warnings.catch_warnings():
+            warnings.simplefilter('ignore', BytesWarning)
+
+            try:
+                raise SyntaxError("msg", (b"bytes_filename", 123, 0, "text"))
+            except SyntaxError as exc:
+                with support.captured_stderr() as err:
+                    sys.__excepthook__(*sys.exc_info())
+
+        err = err.getvalue()
+        self.assertIn("""  File "b'bytes_filename'", line 123\n""", err)
+        self.assertIn("""    text\n""", err)
+        self.assertTrue(err.endswith("SyntaxError: msg\n"))
+
     def test_excepthook(self):
         with test.support.captured_output("stderr") as stderr:
             sys.excepthook(1, '1', 1)
@@ -86,6 +107,12 @@ def test_excepthook(self):
     # FIXME: testing the code for a lost or replaced excepthook in
     # Python/pythonrun.c::PyErr_PrintEx() is tricky.
 
+
+class SysModuleTest(unittest.TestCase):
+
+    def tearDown(self):
+        test.support.reap_children()
+
     def test_exit(self):
         # call with two arguments
         self.assertRaises(TypeError, sys.exit, 42, 42)
@@ -502,10 +529,10 @@ def test_43581(self):
         self.assertEqual(sys.__stdout__.encoding, sys.__stderr__.encoding)
 
     def test_intern(self):
-        global numruns
-        numruns += 1
+        global INTERN_NUMRUNS
+        INTERN_NUMRUNS += 1
         self.assertRaises(TypeError, sys.intern)
-        s = "never interned before" + str(numruns)
+        s = "never interned before" + str(INTERN_NUMRUNS)
         self.assertTrue(sys.intern(s) is s)
         s2 = s.swapcase().swapcase()
         self.assertTrue(sys.intern(s2) is s)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-07-01-12-22-44.bpo-37467.u-XyEu.rst b/Misc/NEWS.d/next/Core and Builtins/2019-07-01-12-22-44.bpo-37467.u-XyEu.rst
@@ -0,0 +1,3 @@
+Fix :func:`sys.excepthook` and :c:func:`PyErr_Display` if a filename is a
+bytes string. For example, for a SyntaxError exception where the filename
+attribute is a bytes string.
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
@@ -750,7 +750,7 @@ print_exception(PyObject *f, PyObject *value)
             Py_DECREF(value);
             value = message;
 
-            line = PyUnicode_FromFormat("  File \"%U\", line %d\n",
+            line = PyUnicode_FromFormat("  File \"%S\", line %d\n",
                                           filename, lineno);
             Py_DECREF(filename);
             if (line != NULL) {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Fix :func:`sys.excepthook` and :c:func:`PyErr_Display` if a filename is a
	`2`	`+bytes string. For example, for a SyntaxError exception where the filename`
	`3`	`+attribute is a bytes string.`