Merge pull request #4418 from asottile/printable_ids

Ensure that node ids are printable

changelog/4397.bugfix.rst

@@ -0,0 +1 @@
+Ensure that node ids are printable.

src/_pytest/compat.py

@@ -182,6 +182,18 @@ def get_default_arg_names(function):
     )
 
 
+_non_printable_ascii_translate_table = {
+    i: u"\\x{:02x}".format(i) for i in range(128) if i not in range(32, 127)
+}
+_non_printable_ascii_translate_table.update(
+    {ord("\t"): u"\\t", ord("\r"): u"\\r", ord("\n"): u"\\n"}
+)
+
+
+def _translate_non_printable(s):
+    return s.translate(_non_printable_ascii_translate_table)
+
+
 if _PY3:
     STRING_TYPES = bytes, str
     UNICODE_TYPES = six.text_type
@@ -221,9 +233,10 @@ if _PY3:
 
         """
         if isinstance(val, bytes):
-            return _bytes_to_ascii(val)
+            ret = _bytes_to_ascii(val)
         else:
-            return val.encode("unicode_escape").decode("ascii")
+            ret = val.encode("unicode_escape").decode("ascii")
+        return _translate_non_printable(ret)
 
 
 else:
@@ -241,11 +254,12 @@ else:
         """
         if isinstance(val, bytes):
             try:
-                return val.encode("ascii")
+                ret = val.decode("ascii")
             except UnicodeDecodeError:
-                return val.encode("string-escape")
+                ret = val.encode("string-escape").decode("ascii")
         else:
-            return val.encode("unicode-escape")
+            ret = val.encode("unicode-escape").decode("ascii")
+        return _translate_non_printable(ret)
 
 
 class _PytestWrapper(object):

src/_pytest/mark/structures.py

@@ -5,8 +5,10 @@ from functools import reduce
 from operator import attrgetter
 
 import attr
+import six
 from six.moves import map
 
+from ..compat import ascii_escaped
 from ..compat import getfslineno
 from ..compat import MappingMixin
 from ..compat import NOTSET
@@ -70,10 +72,13 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")):
         else:
             assert isinstance(marks, (tuple, list, set))
 
-        def param_extract_id(id=None):
-            return id
-
-        id_ = param_extract_id(**kw)
+        id_ = kw.pop("id", None)
+        if id_ is not None:
+            if not isinstance(id_, six.string_types):
+                raise TypeError(
+                    "Expected id to be a string, got {}: {!r}".format(type(id_), id_)
+                )
+            id_ = ascii_escaped(id_)
         return cls(values, marks, id_)
 
     @classmethod

testing/python/metafunc.py

@@ -5,6 +5,7 @@ import textwrap
 
 import attr
 import hypothesis
+import six
 from hypothesis import strategies
 
 import pytest
@@ -262,11 +263,8 @@ class TestMetafunc(object):
         from _pytest.python import _idval
 
         escaped = _idval(value, "a", 6, None, item=None, config=None)
-        assert isinstance(escaped, str)
-        if PY3:
-            escaped.encode("ascii")
-        else:
-            escaped.decode("ascii")
+        assert isinstance(escaped, six.text_type)
+        escaped.encode("ascii")
 
     def test_unicode_idval(self):
         """This tests that Unicode strings outside the ASCII character set get
@@ -382,6 +380,34 @@ class TestMetafunc(object):
             "\\xc3\\xb4-other",
         ]
 
+    def test_idmaker_non_printable_characters(self):
+        from _pytest.python import idmaker
+
+        result = idmaker(
+            ("s", "n"),
+            [
+                pytest.param("\x00", 1),
+                pytest.param("\x05", 2),
+                pytest.param(b"\x00", 3),
+                pytest.param(b"\x05", 4),
+                pytest.param("\t", 5),
+                pytest.param(b"\t", 6),
+            ],
+        )
+        assert result == ["\\x00-1", "\\x05-2", "\\x00-3", "\\x05-4", "\\t-5", "\\t-6"]
+
+    def test_idmaker_manual_ids_must_be_printable(self):
+        from _pytest.python import idmaker
+
+        result = idmaker(
+            ("s",),
+            [
+                pytest.param("x00", id="hello \x00"),
+                pytest.param("x05", id="hello \x05"),
+            ],
+        )
+        assert result == ["hello \\x00", "hello \\x05"]
+
     def test_idmaker_enum(self):
         from _pytest.python import idmaker
 

testing/test_mark.py

@@ -5,20 +5,21 @@ from __future__ import print_function
 import os
 import sys
 
+import six
+
+import pytest
+from _pytest.mark import EMPTY_PARAMETERSET_OPTION
+from _pytest.mark import MarkGenerator as Mark
+from _pytest.mark import ParameterSet
+from _pytest.mark import transfer_markers
+from _pytest.nodes import Collector
+from _pytest.nodes import Node
 from _pytest.warnings import SHOW_PYTEST_WARNINGS_ARG
 
 try:
     import mock
 except ImportError:
     import unittest.mock as mock
-import pytest
-from _pytest.mark import (
-    MarkGenerator as Mark,
-    ParameterSet,
-    transfer_markers,
-    EMPTY_PARAMETERSET_OPTION,
-)
-from _pytest.nodes import Node, Collector
 
 ignore_markinfo = pytest.mark.filterwarnings(
     "ignore:MarkInfo objects:pytest.RemovedInPytest4Warning"
@@ -1252,3 +1253,18 @@ def test_markers_from_parametrize(testdir):
 
     result = testdir.runpytest(SHOW_PYTEST_WARNINGS_ARG)
     result.assert_outcomes(passed=4)
+
+
+def test_pytest_param_id_requires_string():
+    with pytest.raises(TypeError) as excinfo:
+        pytest.param(id=True)
+    msg, = excinfo.value.args
+    if six.PY2:
+        assert msg == "Expected id to be a string, got <type 'bool'>: True"
+    else:
+        assert msg == "Expected id to be a string, got <class 'bool'>: True"
+
+
+@pytest.mark.parametrize("s", (None, "hello world"))
+def test_pytest_param_id_allows_none_or_string(s):
+    assert pytest.param(id=s)