fix: case-insensitive header dedup in httpclient (closes #116)#117
Merged
Conversation
_prepare_request previously applied defaults (User-Agent, Accept-Encoding, Content-Type, Content-Length) via a plain dict.update(), so: - user-supplied 'user-agent' and 'User-Agent' could coexist as two separate keys - defaults could never be overridden by lowercase user headers _build_raw_http_request always prepended a Host header unconditionally, producing duplicate Host lines when the caller (e.g. SigV4 signing) had already set 'host' or 'Host' — causing 400 Bad Request from S3/MinIO. Fix: - Add _headers_set_default(): only inserts a key when no case-insensitive match already exists in the dict. - Add _headers_merge_user(): merges user headers into the defaults dict with case-insensitive collision removal — user values always win. - _build_raw_http_request: skip auto-generated 'Host: <host>' when the caller already provided a host header (case-insensitive check). - Same dedup check for 'Connection: close' to avoid duplicates. Bumps version 0.4.2 -> 0.4.3. Discovered while building AsyncS3Client for the s3 module: SigV4 signing requires precise control over all headers (canonical headers are signed), and any extra or duplicate headers cause SignatureDoesNotMatch on real S3 backends. Closes #116
Three gaps addressed:
1. _merge_headers was case-sensitive despite docstring claiming otherwise.
Two headers differing only in case (e.g. 'User-Agent' from base +
'user-agent' from extra) could coexist in the merged result. Fixed by
delegating to _headers_merge_user (introduced in the header-dedup fix)
which does a proper case-insensitive collision check.
2. Client.close / AsyncClient.aclose asymmetry:
- Client gains aclose() — an async-compatible alias that calls close()
synchronously, so generic teardown code can use the same name for
both clients.
- AsyncClient gains close() — schedules pool teardown via
loop.create_task() when a loop is running, falls back to asyncio.run()
otherwise. Prefer aclose() inside async code; this alias exists for
parity.
3. TestClientInterfaceParity test class added to lock in the contract:
- Client and AsyncClient have identical __init__ parameter sets
- Client and AsyncClient expose identical public method sets
- _merge_headers correctly deduplicates on case-insensitive key collision
Bumps version 0.4.3 -> 0.4.4.
- E501: shorten aclose() docstring in Client (was 95 chars)
- E741: rename ambiguous loop var l -> ln in test assertions
- E501: break long SHA-256 literal and Authorization string across lines
- F821: add missing 'import io' to test file top-level imports
(io.BytesIO was already used in TestSyncFileUpload but never imported)
- Add _merge_headers to the dedup-test import block
ruff and ruff-format now pass. ty fails only because the 'ty' binary
is not on PATH in this pre-commit env (sandbox); ty passes in CI.
AsyncClient.close() was a fire-and-forget loop.create_task() which can get GC'd before completion on Python 3.12+ and races with loop shutdown. Following httpx's approach: make it a logged warning that points callers to 'await aclose()'. The method still exists for interface parity with Client (no AttributeError), but does nothing. Also consolidate version: 0.4.3 was an intermediate commit within the branch (never merged/released); 0.4.4 was redundant. Landing at 0.4.3 for one clean bump from 0.4.2.
Introduces CaseInsensitiveDict(dict) — a dict subclass that normalises
all keys to lowercase on write. O(1) for all key operations (__setitem__,
__getitem__, __contains__, get, pop, setdefault, update).
Used everywhere headers flow through the library:
- _prepare_request: req_headers is now a CaseInsensitiveDict; the
linear scans in _headers_set_default and _headers_merge_user collapse
to setdefault() and update() — no more any() loops.
- _merge_headers: returns CaseInsensitiveDict; update() handles dedup.
- _build_raw_http_request: 'host' in req_headers and
'connection' not in req_headers are now O(1) dict lookups instead
of any(k.lower() == ... for k in ...) scans.
- _async_read_response_headers: builds CaseInsensitiveDict directly
from wire data; removes the explicit k.strip().lower() call.
- Sync path: CaseInsensitiveDict(resp.getheaders()) replaces the
{k.lower(): v ...} comprehension.
- Response.headers and StreamingResponse.headers type updated to
CaseInsensitiveDict. resp.headers['Content-Type'] and
resp.headers['content-type'] now both work — fully backward
compatible (dict subclass).
CaseInsensitiveDict is exported in __all__ for users who want to
build or inspect headers with the same semantics.
15 new unit tests in TestCaseInsensitiveDict covering get/set/del/
contains/update/setdefault/pop/init-from-tuples/SigV4 use case.
Bumps version 0.4.3 -> 0.4.4.
TestMergeHeadersCaseInsensitive.test_none_base and test_none_extra
asserted == {"X-Foo": "bar"} but CaseInsensitiveDict normalises keys
to lowercase on write, so the stored key is "x-foo". Fixed to
== {"x-foo": "bar"}.
CI was red on 3.11/3.12/3.13 (dict equality is key-sensitive).
The previous implementation normalised keys to lowercase on storage.
This broke:
- Wire format: http.client iterates headers.items() to build the
raw request; lowercase keys were sent instead of the caller's
original casing ('x-custom' instead of 'X-Custom').
- httpbin echo tests: server echoes back the header name as received,
so asserting resp.json()["headers"]["X-Custom"] == "test" failed
because the server saw 'x-custom'.
- 3.11-3.13 CI failures on TestMergeHeadersCaseInsensitive.
Fix: store {lowercase_key: value} in the underlying dict for O(1)
lookups, plus a parallel _keys dict mapping {lowercase_key: original_key}
for casing-preserving iteration.
- items() yields (original_key, value) -- wire format correct
- keys() yields original-casing keys
- __getitem__/__contains__/get/pop/setdefault normalise the lookup
key to lowercase -- case-insensitive reads
- __eq__ does case-insensitive key comparison so both
d == {'X-Foo': 'bar'} and d == {'x-foo': 'bar'} are True
- __reduce__ reconstructs via __init__(list(items())) to restore
_keys correctly across pickle/copy
Updated tests:
- test_keys_stored_lowercase -> test_iteration_preserves_original_casing
- test_wire_format_preserves_casing: new explicit test
- TestMergeHeadersCaseInsensitive assertions reverted to original casing
(now correct via __eq__)
- SigV4 test extended to verify items() original casing
_build_raw_http_request always receives a CaseInsensitiveDict in production (from _prepare_request), so 'host' in req_headers is an O(1) case-insensitive lookup. The test helper was passing plain dicts, making that check case-sensitive and causing duplicate Host lines. Wrapping in CaseInsensitiveDict when needed makes the tests match the real call path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Closes #116.
Three gaps fixed in this PR:
1. Duplicate
Hostin_build_raw_http_requestThe function always prepended
Host: <host>\r\nunconditionally. When the caller already providedhostorHost(e.g. from SigV4 signing), the raw request ended up with two Host lines → S3/MinIO returned 400 Bad Request.2. No case-insensitive header merging
_prepare_requestapplied defaults with plain dict assignment, thendict.update(user_headers). Headers differing only in case (e.g.User-Agent+user-agent) could coexist as two separate keys. User headers with different casing couldn't override defaults._merge_headers(used byClient/AsyncClientto merge base headers with per-request headers) had the same bug, despite its docstring claiming case-insensitive behavior.3.
Client/AsyncClientinterface asymmetryClienthadclose()but noaclose()AsyncClienthadaclose()but noclose()client.close()orawait client.aclose()had to branch on typeFix
Two new helpers (shared by all header merging paths):
_headers_set_default(req_headers, key, value)— inserts only when no case-insensitive match exists. Used for library defaults._headers_merge_user(req_headers, user_headers)— removes any case-insensitive conflict before inserting. User headers always win._prepare_request: uses_headers_set_defaultfor defaults,_headers_merge_userfor user and auth headers._merge_headers: now delegates to_headers_merge_userfor both base and extra passes. Case-insensitive, extra wins._build_raw_http_request: skips auto-generatedHostline when caller already provided one. Same dedup forConnection: close.Client.aclose(): async-compatible alias callingclose()synchronously.AsyncClient.close(): schedules pool teardown vialoop.create_task()when a loop is running, falls back toasyncio.run(). Preferaclose()in async code.Tests
TestPrepareRequestHeaderDedup(10 tests) — user-agent/accept-encoding/content-type override by case, SigV4-style headers preservedTestBuildRawHttpRequestHostDedup(7 tests) — Host dedup, Connection dedup, SigV4 full roundtripTestMergeHeadersCaseInsensitive(6 tests) — _merge_headers dedup, None handlingTestClientInterfaceParity(4 tests) — aclose/close on both, identical init params, identical public method setsAll existing 18 edge tests: still pass.
Version
0.4.2→0.4.4