Fix headers overriding in RequestBase -> get_headers() (#601)

Author: MarshalX

packages/atproto_client/request.py

@@ -26,13 +26,40 @@ class Response:
     headers: t.Dict[str, t.Any]
 
 
+_NormalizedHeaders = t.Dict[str, t.Tuple[str, str]]
+
+
+def _normalize_headers(headers: t.Dict[str, str]) -> _NormalizedHeaders:
+    """Normalize headers by converting keys to lowercase.
+
+    Args:
+        headers: Headers dictionary with the original case.
+
+    Returns:
+        Dictionary with lowercase keys mapped to tuples of (original_key, value).
+    """
+    return {key.lower(): (key, value) for key, value in headers.items()}
+
+
+def _denormalize_headers(headers: _NormalizedHeaders) -> t.Dict[str, str]:
+    """Denormalize headers by converting keys back to their original case.
+
+    Args:
+        headers: Headers dictionary with lowercase keys mapped to tuples of (original_key, value).
+
+    Returns:
+        Dictionary with original keys and values.
+    """
+    return {original_key: value for _, (original_key, value) in headers.items()}
+
+
 def _convert_headers_to_dict(headers: httpx.Headers) -> t.Dict[str, str]:
-    """Convert custom case-insensitive multi-dict of HTTPX to pure dict with lowercased keys.
+    """Convert custom case-insensitive multi-dict of HTTPX to pure dict with lowercase keys.
 
     Note:
         Concatenate headers into a single comma separated value when a key occurs multiple times.
     """
-    return dict(headers.items())
+    return {key.lower(): value for key, value in headers.items()}
 
 
 def _parse_response(response: httpx.Response) -> Response:
@@ -100,15 +127,19 @@ def get_headers(self, additional_headers: t.Optional[t.Dict[str, str]] = None) -
         Returns:
             Headers for the request.
         """
-        headers = {**RequestBase._MANDATORY_HEADERS, **self._additional_headers}
+        # The order of calling `.update()` matters. It defines the priority of overriding.
+        headers_lower = {
+            **_normalize_headers(RequestBase._MANDATORY_HEADERS),
+            **_normalize_headers(self._additional_headers),
+        }
 
         for header_source in self._additional_header_sources:
-            headers.update(header_source())
+            headers_lower.update(_normalize_headers(header_source()))
 
         if additional_headers:
-            headers.update(additional_headers)
+            headers_lower.update(_normalize_headers(additional_headers))
 
-        return headers
+        return _denormalize_headers(headers_lower)
 
     def set_additional_headers(self, headers: t.Dict[str, str]) -> None:
         """Set additional headers for the request.

tests/test_atproto_client/client/test_request.py

@@ -0,0 +1,110 @@
+from atproto_client.request import RequestBase
+
+
+def test_get_headers_case_insensitivity() -> None:
+    """Test that get_headers handles case-insensitive header names correctly."""
+    req = RequestBase()
+
+    # Add a header with mixed case
+    req.add_additional_header('Content-Type', 'application/json')
+
+    # Try to override with a different case
+    headers = req.get_headers({'content-type': 'text/plain'})
+
+    # Check that the header was properly overridden
+    assert 'content-type' in headers
+    assert headers['content-type'] == 'text/plain'
+    assert 'Content-Type' not in headers  # No mixed case keys
+
+    # Check that there's only one content-type header (case-insensitive)
+    content_type_headers = [k for k in headers if k.lower() == 'content-type']
+    assert len(content_type_headers) == 1
+
+
+def test_add_additional_header_case_insensitivity() -> None:
+    """Test that add_additional_header handles case-insensitive header names correctly."""
+    req = RequestBase()
+
+    # Add a header with mixed case
+    req.add_additional_header('Content-Type', 'application/json')
+
+    # Add the same header with a different case
+    req.add_additional_header('content-type', 'text/plain')
+
+    # Get the headers
+    headers = req.get_headers()
+
+    # Check that the header was properly overridden
+    assert 'content-type' in headers
+    assert headers['content-type'] == 'text/plain'
+    assert 'Content-Type' not in headers  # No mixed case keys
+
+    # Check that there's only one content-type header (case-insensitive)
+    content_type_headers = [k for k in headers if k.lower() == 'content-type']
+    assert len(content_type_headers) == 1
+
+
+def test_set_additional_headers_case_insensitivity() -> None:
+    """Test set_additional_headers."""
+    req = RequestBase()
+
+    # Set headers with a mixed case
+    req.set_additional_headers(
+        {'Content-Type': 'application/json', 'AUTHORIZATION': 'Bearer token', 'accept': 'application/json'}
+    )
+
+    # Get the headers
+    headers = req.get_headers()
+
+    # Check that all headers are present
+    assert 'Content-Type' in headers
+    assert 'AUTHORIZATION' in headers
+    assert 'accept' in headers
+
+    # Check values
+    assert headers['Content-Type'] == 'application/json'
+    assert headers['AUTHORIZATION'] == 'Bearer token'
+    assert headers['accept'] == 'application/json'
+
+
+def test_headers_override_with_additional_headers() -> None:
+    """Test that additional headers properly override existing headers."""
+    req = RequestBase()
+
+    # Add some headers
+    req.add_additional_header('content-type', 'application/json')
+    req.add_additional_header('authorization', 'Bearer token1')
+
+    # Override with additional headers
+    headers = req.get_headers({'Content-Type': 'text/plain', 'AUTHORIZATION': 'Bearer token2'})
+
+    # Check that headers were properly overridden
+    assert headers['Content-Type'] == 'text/plain'
+    assert headers['AUTHORIZATION'] == 'Bearer token2'
+
+    # Check that there are no duplicate headers with different cases
+    assert len([k for k in headers if k.lower() == 'content-type']) == 1
+    assert len([k for k in headers if k.lower() == 'authorization']) == 1
+
+
+def test_headers_from_sources() -> None:
+    """Test that headers from sources are properly handled."""
+    req = RequestBase()
+
+    # Add a header source
+    req.add_additional_headers_source(lambda: {'Content-Type': 'application/json'})
+
+    # Add another header source with a different case
+    req.add_additional_headers_source(lambda: {'content-type': 'text/plain'})
+
+    # Get the headers
+    headers = req.get_headers()
+
+    # Check that the last source's value is used
+    assert headers['content-type'] == 'text/plain'
+
+    # Check that the first source's value is not used
+    assert 'Content-Type' not in headers
+
+    # Check that there are no duplicate headers with different cases
+    assert len([k for k in headers if k.lower() == 'content-type']) == 1