Improve performance of serializing headers (#10625)

Improve performance of serializing headers by moving the check for `\r`
and `\n` into the write loop instead of making a separate call to check
each disallowed character in the Python string.

Author: bdraco

CHANGES/10625.misc.rst

@@ -0,0 +1 @@
+Improved performance of serializing headers -- by :user:`bdraco`.

aiohttp/_http_writer.pyx

@@ -97,27 +97,34 @@ cdef inline int _write_str(Writer* writer, str s):
             return -1
 
 
-# --------------- _serialize_headers ----------------------
-
-cdef str to_str(object s):
+cdef inline int _write_str_raise_on_nlcr(Writer* writer, object s):
+    cdef Py_UCS4 ch
+    cdef str out_str
     if type(s) is str:
-        return <str>s
+        out_str = <str>s
     elif type(s) is _istr:
-        return PyObject_Str(s)
+        out_str = PyObject_Str(s)
     elif not isinstance(s, str):
         raise TypeError("Cannot serialize non-str key {!r}".format(s))
     else:
-        return str(s)
+        out_str = str(s)
+
+    for ch in out_str:
+        if ch == 0x0D or ch == 0x0A:
+            raise ValueError(
+                "Newline or carriage return detected in headers. "
+                "Potential header injection attack."
+            )
+        if _write_utf8(writer, ch) < 0:
+            return -1
 
 
+# --------------- _serialize_headers ----------------------
 
 def _serialize_headers(str status_line, headers):
     cdef Writer writer
     cdef object key
     cdef object val
-    cdef bytes ret
-    cdef str key_str
-    cdef str val_str
 
     _init_writer(&writer)
 
@@ -130,22 +137,13 @@ def _serialize_headers(str status_line, headers):
             raise
 
         for key, val in headers.items():
-            key_str = to_str(key)
-            val_str = to_str(val)
-
-            if "\r" in key_str or "\n" in key_str or "\r" in val_str or "\n" in val_str:
-                raise ValueError(
-                    "Newline or carriage return character detected in HTTP status message or "
-                    "header. This is a potential security issue."
-                )
-
-            if _write_str(&writer, key_str) < 0:
+            if _write_str_raise_on_nlcr(&writer, key) < 0:
                 raise
             if _write_byte(&writer, b':') < 0:
                 raise
             if _write_byte(&writer, b' ') < 0:
                 raise
-            if _write_str(&writer, val_str) < 0:
+            if _write_str_raise_on_nlcr(&writer, val) < 0:
                 raise
             if _write_byte(&writer, b'\r') < 0:
                 raise

tests/test_http_writer.py

@@ -8,8 +8,9 @@
 import pytest
 from multidict import CIMultiDict
 
-from aiohttp import ClientConnectionResetError, http
+from aiohttp import ClientConnectionResetError, hdrs, http
 from aiohttp.base_protocol import BaseProtocol
+from aiohttp.http_writer import _serialize_headers
 from aiohttp.test_utils import make_mocked_coro
 
 
@@ -603,3 +604,29 @@ async def test_set_eof_after_write_headers(
     msg.set_eof()
     await msg.write_eof()
     assert not transport.write.called
+
+
+@pytest.mark.parametrize(
+    "char",
+    [
+        "\n",
+        "\r",
+    ],
+)
+def test_serialize_headers_raises_on_new_line_or_carriage_return(char: str) -> None:
+    """Verify serialize_headers raises on cr or nl in the headers."""
+    status_line = "HTTP/1.1 200 OK"
+    headers = CIMultiDict(
+        {
+            hdrs.CONTENT_TYPE: f"text/plain{char}",
+        }
+    )
+
+    with pytest.raises(
+        ValueError,
+        match=(
+            "Newline or carriage return detected in headers. "
+            "Potential header injection attack."
+        ),
+    ):
+        _serialize_headers(status_line, headers)