diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 36049e26b..6f06f4f32 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -3438,3 +3438,48 @@ def test_accept_language_ignore(self): def test_accept_language_invalid(self): response = self.fetch("/", headers={"Accept-Language": "fr-FR;q=-1"}) self.assertEqual(response.headers["Content-Language"], "en-US") + + +class ConvertHeaderValueTest(unittest.TestCase): + """Unit tests for ``RequestHandler._convert_header_value``.""" + + def _convert(self, value): + # ``_convert_header_value`` only uses ``self`` to look up the + # class-level regex, so any RequestHandler instance will do. + return RequestHandler._convert_header_value(self, value) + + def test_str_passthrough(self): + self.assertEqual(self._convert("plain"), "plain") + + def test_bytes_decoded_latin1(self): + # Non-ascii bytes should be passed through as latin1 so + # they round-trip identically. + self.assertEqual(self._convert(b"caf\xc3\xa9"), "caf\xc3\xa9") + + def test_int_converted(self): + self.assertEqual(self._convert(42), "42") + self.assertEqual(self._convert(0), "0") + self.assertEqual(self._convert(-1), "-1") + + def test_unsafe_value_message_includes_value(self): + # Regression test: ValueError used to be raised with the bad + # value as a second positional argument, so str(e) was the + # tuple repr "('Unsafe header value %r', 'foo\\r\\nbar')" and + # the offending value was unreachable from str(e). It must now + # be formatted into the message so callers can log it. + bad = "foo\r\nX-Inject: yes" + with self.assertRaises(ValueError) as cm: + self._convert(bad) + msg = str(cm.exception) + self.assertIn("Unsafe header value", msg) + # The bad value (or a recognisable chunk of it) must appear in + # the message, not be hiding in e.args as a separate element. + self.assertIn("foo", msg) + # And the exception should have exactly one args entry — the + # formatted message — not two. + self.assertEqual(len(cm.exception.args), 1) + + def test_empty_string_is_safe(self): + # An empty header value is allowed (mirrors the existing + # SetHeaderHandler behaviour in the integration test). + self.assertEqual(self._convert(""), "") diff --git a/tornado/web.py b/tornado/web.py index b572beac0..89d1959b9 100644 --- a/tornado/web.py +++ b/tornado/web.py @@ -422,7 +422,7 @@ def _convert_header_value(self, value: _HeaderTypes) -> str: # If \n is allowed into the header, it is possible to inject # additional headers or split the request. if RequestHandler._VALID_HEADER_CHARS.fullmatch(retval) is None: - raise ValueError("Unsafe header value %r", retval) + raise ValueError("Unsafe header value %r" % retval) return retval @overload