Better label lowercase to avoid confusion

changelog.d/17894.misc

@@ -0,0 +1 @@
+Remove usage of internal header encoding API.

synapse/http/proxy.py

@@ -51,25 +51,17 @@ logger = logging.getLogger(__name__)
 # "Hop-by-hop" headers (as opposed to "end-to-end" headers) as defined by RFC2616
 # section 13.5.1 and referenced in RFC9110 section 7.6.1. These are meant to only be
 # consumed by the immediate recipient and not be forwarded on.
-HOP_BY_HOP_HEADERS = {
+HOP_BY_HOP_HEADERS_LOWERCASE = {
-    "Connection",
+    "connection",
-    "Keep-Alive",
+    "keep-alive",
-    "Proxy-Authenticate",
+    "proxy-authenticate",
-    "Proxy-Authorization",
+    "proxy-authorization",
-    "TE",
+    "te",
-    "Trailers",
+    "trailers",
-    "Transfer-Encoding",
+    "transfer-encoding",
-    "Upgrade",
+    "upgrade",
 }
-
+assert all(header.lower() == header for header in HOP_BY_HOP_HEADERS_LOWERCASE)
-if hasattr(Headers, "_canonicalNameCaps"):
-    # Twisted < 24.7.0rc1
-    _canonicalHeaderName = Headers()._canonicalNameCaps  # type: ignore[attr-defined]
-else:
-    # Twisted >= 24.7.0rc1
-    # But note that `_encodeName` still exists on prior versions,
-    # it just encodes differently
-    _canonicalHeaderName = Headers()._encodeName
 
 
 def parse_connection_header_value(
@@ -92,12 +84,12 @@ def parse_connection_header_value(
 
     Returns:
         The set of header names that should not be copied over from the remote response.
-        The keys are capitalized in canonical capitalization.
+        The keys are lowercased.
     """
     extra_headers_to_remove: Set[str] = set()
     if connection_header_value:
         extra_headers_to_remove = {
-            _canonicalHeaderName(connection_option.strip()).decode("ascii")
+            connection_option.decode("ascii").strip().lower()
             for connection_option in connection_header_value.split(b",")
         }
 
@@ -194,7 +186,7 @@ class ProxyResource(_AsyncResource):
 
         # The `Connection` header also defines which headers should not be copied over.
         connection_header = response_headers.getRawHeaders(b"connection")
-        extra_headers_to_remove = parse_connection_header_value(
+        extra_headers_to_remove_lowercase = parse_connection_header_value(
             connection_header[0] if connection_header else None
         )
 
@@ -202,10 +194,10 @@ class ProxyResource(_AsyncResource):
         for k, v in response_headers.getAllRawHeaders():
             # Do not copy over any hop-by-hop headers. These are meant to only be
             # consumed by the immediate recipient and not be forwarded on.
-            header_key = k.decode("ascii")
+            header_key_lowercase = k.decode("ascii").lower()
             if (
-                header_key in HOP_BY_HOP_HEADERS
+                header_key_lowercase in HOP_BY_HOP_HEADERS_LOWERCASE
-                or header_key in extra_headers_to_remove
+                or header_key_lowercase in extra_headers_to_remove_lowercase
             ):
                 continue
 

tests/http/test_matrixfederationclient.py

@@ -903,12 +903,18 @@ class FederationClientProxyTests(BaseMultiWorkerStreamTestCase):
                     headers=Headers(
                         {
                             "Content-Type": ["application/json"],
-                            "Connection": ["close, X-Foo, X-Bar"],
+                            # Define some hop-by-hop headers (try with varying casing to
+                            # make sure we still match-up the headers)
+                            "Connection": ["close, X-fOo, X-Bar", "X-baz"],
                             # Should be removed because it's defined in the `Connection` header
                             "X-Foo": ["foo"],
                             "X-Bar": ["bar"],
+                            # (not in canonical case)
+                            "x-baZ": ["baz"],
                             # Should be removed because it's a hop-by-hop header
                             "Proxy-Authorization": "abcdef",
+                            # Should be removed because it's a hop-by-hop header (not in canonical case)
+                            "transfer-EnCoDiNg": "abcdef",
                         }
                     ),
                 )

tests/http/test_proxy.py

@@ -30,19 +30,19 @@ from tests.unittest import TestCase
 class ProxyTests(TestCase):
     @parameterized.expand(
         [
-            [b"close, X-Foo, X-Bar", {"Close", "X-Foo", "X-Bar"}],
+            [b"close, X-Foo, X-Bar", {"close", "x-foo", "x-bar"}],
             # No whitespace
-            [b"close,X-Foo,X-Bar", {"Close", "X-Foo", "X-Bar"}],
+            [b"close,X-Foo,X-Bar", {"close", "x-foo", "x-bar"}],
             # More whitespace
-            [b"close,    X-Foo,      X-Bar", {"Close", "X-Foo", "X-Bar"}],
+            [b"close,    X-Foo,      X-Bar", {"close", "x-foo", "x-bar"}],
             # "close" directive in not the first position
-            [b"X-Foo, X-Bar, close", {"X-Foo", "X-Bar", "Close"}],
+            [b"X-Foo, X-Bar, close", {"x-foo", "x-bar", "close"}],
             # Normalizes header capitalization
-            [b"keep-alive, x-fOo, x-bAr", {"Keep-Alive", "X-Foo", "X-Bar"}],
+            [b"keep-alive, x-fOo, x-bAr", {"keep-alive", "x-foo", "x-bar"}],
             # Handles header names with whitespace
             [
                 b"keep-alive, x  foo, x bar",
-                {"Keep-Alive", "X  foo", "X bar"},
+                {"keep-alive", "x  foo", "x bar"},
             ],
         ]
     )