fix(proxy): concatenate HTTP/2 Cookie headers when proxying to HTTP/1.1 (RFC 9113 §8.2.3)#901
Open
MyLittleLuckyDog wants to merge 2 commits into
Open
Conversation
RFC 9113 §8.2.3 requires that multiple Cookie header fields received over HTTP/2 MUST be concatenated into a single octet string using the "; " delimiter before being passed into a non-HTTP/2 context. When a browser sends cookies over HTTP/2, it may split them into separate header fields for better HPACK compression. If these are forwarded as-is to an HTTP/1.1 upstream, many backends only read the first Cookie header, silently dropping session cookies and causing failures such as CSRF token verification errors (HTTP 422). Changes: - Add h2_to_h1_concat_cookie_headers() in proxy_h1.rs that merges multiple Cookie headers preserving insertion order - Call it during H2→H1 conversion in proxy_1to1() - Add PeerOptions.h2_to_h1_concat_cookies flag (default: true) to allow disabling if needed - Add 6 unit tests covering normal, edge, and no-op cases Fixes cloudflare#892 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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.
Summary
Implements Cookie header concatenation when proxying HTTP/2 requests to HTTP/1.1 upstreams, as required by RFC 9113 §8.2.3.
Fixes #892
Problem
When a browser sends cookies over HTTP/2, it may split them into separate
Cookieheader fields for better HPACK compression (permitted by RFC 9113 §8.2.3). Pingora currently forwards these as separate header lines to HTTP/1.1 upstreams. Many backends only read the firstCookieheader, causing session cookies to be silently dropped — leading to failures such as CSRF token verification errors (HTTP 422 on GitLab, for example).Changes
pingora-proxy/src/proxy_h1.rs: Addh2_to_h1_concat_cookie_headers()that merges multipleCookieheaders into one using"; "as delimiter, preserving insertion order. Called during H2→H1 conversion inproxy_1to1().pingora-core/src/upstreams/peer.rs: AddPeerOptions.h2_to_h1_concat_cookiesflag (default:true) to allow disabling the behavior if needed.Test plan
test_concat_multiple_cookies— 3 cookies merged correctlytest_concat_preserves_order— GitLab CSRF scenario (order preserved)test_concat_single_cookie_unchanged— single cookie untouchedtest_concat_no_cookies_is_noop— no cookies, no changestest_concat_two_cookies— 2 cookies merged, result is exactly 1 headertest_concat_cookies_with_semicolons— cookie values already containing;handled correctlycargo check -p pingora-proxy -p pingora-corepasses