Fix merge_headers() joining multiple Cookie headers with ", " - #1467
Fix merge_headers() joining multiple Cookie headers with ", "#1467kimgunnn wants to merge 3 commits into
Conversation
Cookies are separated by "; ", not commas. HTTP/2 clients may split the cookie header into multiple field lines, and joining them back with ", " produces a Cookie header that WSGI frameworks cannot parse, which breaks session authentication (e.g. Django login loops). The payload v2 path already joins event["cookies"] with "; "; this brings the v1 path in line. Fixes zappa#1466
6af9421 to
74b9e12
Compare
There was a problem hiding this comment.
The fix itself is correct and verified.
Requesting a few small changes to align with project conventions before merge:
- Add a lowercase
cookietest case. The new test uses"Cookie", but the real-world HTTP/2 shape (and the reproduction in #1466) is lowercase"cookie"— which is exactly what theh.lower()branch exists to handle, and it's currently untested. A second assertion or a parametrized case with{"multiValueHeaders": {"cookie": [...]}}would cover it.
Optional, non-blocking: the existing merge_headers tests live in tests/test_handler.py (test_merge_headers_no_multi_value etc.). Since the function lives in zappa/utilities.py, the new tests' placement in tests/test_utilities.py is reasonable — but consider moving the old ones alongside them in a follow-up so coverage of the function isn't split across two files.
|
coverage: 74.75% (-0.04%) from 74.785% — kimgunnn:fix/cookie-header-merge into zappa:master |
|
Good catch — added the lowercase case via subTest so both spellings are covered. Re: consolidating the existing merge_headers tests from test_handler.py, happy to do that in a follow-up PR. |
3 participants
Description
Fixes #1466
merge_headers()joins all multi-value headers with", ",including
Cookie— but cookies are separated by"; ", not commas.HTTP/2 clients such as Chrome may split the cookie header into multiple field lines,
which API Gateway payload v1 delivers via
multiValueHeaders.Zappa then reassembles them as
csrftoken=A, sessionid=B,which WSGI frameworks cannot parse —
sessionidis silently dropped and session auth breaks (e.g. Django login loops).The payload v2 path already joins
event["cookies"]with"; "(
zappa/wsgi.py), so this change brings v1 in line with v2.GitHub Issues
#1466