Tornado HTTP traces should support multi-value headers

As reported in #46, `TornadoHttpPyctuator` doesn't properly handle
multi-value HTTP headers, which are supproted by
`tornado.httputil.HTTPHeaders` as multiple entries with the same header.

More than that, the value put in traces was a string instead of a
list-of-strings.

Note that `requests` doesn't seem to support sending a request with
multi-value headers, so the testing is limited to verifying
`TornadoHttpPyctuator._get_headers` only.

Author: michael.yak

pyctuator/httptrace/http_header_scrubber.py

@@ -1,7 +1,13 @@
 import re
 
 _keys_to_scrub = re.compile(
-    "^(.*[^A-Za-z])?key([^A-Za-z].*)?$|.*secret.*|.*password.*|.*token.*|.*authorization.*|.*authentication.*|.*cookie.*",
+    "^(.*[^A-Za-z])?key([^A-Za-z].*)?$|"
+    ".*secret.*|"
+    ".*password.*|"
+    ".*token.*|"
+    ".*authorization.*|"
+    ".*authentication.*|"
+    ".*cookie.*",
     re.IGNORECASE
 )
 

pyctuator/httptrace/http_tracer.py

@@ -22,13 +22,4 @@ def add_record(self, record: TraceRecord) -> None:
         self.traces_list.append(record)
 
     def _scrub_and_normalize_headers(self, headers: Mapping[str, List[str]]) -> Mapping[str, List[str]]:
-
-        processed_headers = {}
-        for k, values in headers.items():
-            if isinstance(values, list):
-                processed_headers[k] = [
-                    scrub_header_value(k, v) for v in values]
-            elif isinstance(values, str):
-                # this case happens for tornado headers, unsure why mypy is not catching this, maybe catch this earlier
-                processed_headers[k] = [scrub_header_value(k, values)]
-        return processed_headers
+        return {header: [scrub_header_value(header, value) for value in values] for (header, values) in headers.items()}

pyctuator/impl/tornado_pyctuator.py

@@ -3,8 +3,9 @@
 from datetime import datetime, timedelta
 from functools import partial
 from http import HTTPStatus
-from typing import Any, Optional, Callable
+from typing import Any, Optional, Callable, Mapping, List
 
+from tornado.httputil import HTTPHeaders
 from tornado.web import Application, RequestHandler
 
 from pyctuator.httptrace import TraceRecord, TraceRequest, TraceResponse
@@ -180,11 +181,11 @@ def _intercept_request_and_response(self, handler: RequestHandler) -> None:
             request=TraceRequest(
                 method=handler.request.method or "",
                 uri=handler.request.full_url(),
-                headers={k.lower(): v for k, v in handler.request.headers.items()}
+                headers=get_headers(handler.request.headers)
             ),
             response=TraceResponse(
                 status=handler.get_status(),
-                headers={k.lower(): [v] for k, v in handler._headers.items()}  # pylint: disable=protected-access
+                headers=get_headers(handler._headers)  # pylint: disable=protected-access
             ),
             timeTaken=int(handler.request.request_time() * 1000),
         )
@@ -200,3 +201,9 @@ def _custom_json_serializer(self, value: Any) -> Any:
         if isinstance(value, datetime):
             return str(value)
         return None
+
+
+def get_headers(headers: HTTPHeaders) -> Mapping[str, List[str]]:
+    """ Tornado's HTTPHeaders contains multiple entries of the same header name if multiple values were used, this
+    function groups headers by header name. See documentation of `tornado.httputil.HTTPHeaders` """
+    return {header.lower(): headers.get_list(header) for header in headers.keys()}

tests/httptrace/__init__.py

@@ -9,10 +9,10 @@
     ("authentication", "secret123"),
     ("COOKIE", "my-logged-in-session")
 ])
-def test_scrubbing(key, value):
+def test_scrubbing(key: str, value: str) -> None:
     assert scrub_header_value(key, value) == "******"
 
 
 @pytest.mark.parametrize("key,value", [("Host", "example.org"), ("Content-Length", "2000")])
-def test_non_scrubbing(key, value):
+def test_non_scrubbing(key: str, value: str) -> None:
     assert scrub_header_value(key, value) == value

tests/httptrace/test_http_header_scrubber.py

@@ -0,0 +1,13 @@
+from tornado.httputil import HTTPHeaders
+
+from pyctuator.impl.tornado_pyctuator import get_headers
+
+
+def test_get_headers() -> None:
+    tornado_headers = HTTPHeaders({"content-type": "text/html"})
+    tornado_headers.add("Set-Cookie", "A=B")
+    tornado_headers.add("Set-Cookie", "C=D")
+    assert get_headers(tornado_headers) == {
+        "content-type": ["text/html"],
+        "set-cookie": ["A=B", "C=D"]
+    }

tests/httptrace/test_tornado_pyctuator.py

@@ -288,7 +288,10 @@ def test_traces_endpoint(endpoints: Endpoints) -> None:
     # Create a request with header
     user_header = "my header test"
     authorization = "bearer 123"
-    requests.get(endpoints.root + "httptrace_test_url", headers={"User-Data": user_header, "authorization": authorization})
+    requests.get(
+        endpoints.root + "httptrace_test_url",
+        headers={"User-Data": user_header, "authorization": authorization}
+    )
 
     # Get the captured httptraces
     response = requests.get(endpoints.httptrace)
@@ -300,12 +303,12 @@ def test_traces_endpoint(endpoints: Endpoints) -> None:
     assert int(response.headers.get("Content-Length", -1)) > 0
 
     # Assert Response Secret is scrubbed
-    assert "******" == trace["response"]["headers"]["response-secret"][0]
+    assert trace["response"]["headers"]["response-secret"][0] == "******"
 
     # Assert Request Authorization is scrubbed
     auth_header = "Authorization" if "Authorization" in trace[
         "request"]["headers"] else "authorization"
-    assert "******" == trace["request"]["headers"][auth_header][0]
+    assert trace["request"]["headers"][auth_header][0] == "******"
     # Assert timestamp is formatted in ISO format
     datetime.fromisoformat(trace["timestamp"])