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

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.

diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index c223f92ba6bb86f541f5b29b20741760c7e155df..8852aaef94374af19667c93e112af927c047f8c9 100644 (file)
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -1,81 +1,104 @@
-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 io
+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
+
 
 # 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__
 
-        self.assertRaises(TypeError, eh)
+class ExceptHookTest(unittest.TestCase):
+
+    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)
@@ -85,6 +108,12 @@ class SysModuleTest(unittest.TestCase):
     # 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)
@@ -492,10 +521,10 @@ class SysModuleTest(unittest.TestCase):
         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
new file mode 100644 (file)
index 0000000..5e80964
--- /dev/null
+++ 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
index 8f3ee19279d91289940f389996296c31d250b7a9..f1d946a0b0f83dbf7a15214c9a77a46b724cac63 100644 (file)
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -797,7 +797,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) {