h2: client: fix interop with strict servers and large response headers#3614
Closed
saghul wants to merge 1 commit into
Closed
h2: client: fix interop with strict servers and large response headers#3614saghul wants to merge 1 commit into
saghul wants to merge 1 commit into
Conversation
Three independent h2-client bugs surfaced testing fetch against real servers (txiki.js). Symptoms ranged from connections dropped right after ALPN to RST_STREAM(PROTOCOL_ERROR); they only bit a subset of servers, so they had gone unnoticed against lenient peers (nghttp2, Cloudflare). 1. lws_h2_client_handshake() unconditionally reassigned the stream's sid to highest_sid_opened + 2 at header-send time, overwriting the sid 1 the first client stream is already bound to (-> 3). Strict servers (Google's GFE) reject a first client stream that isn't sid 1 with PROTOCOL_ERROR. Only allocate a sid here when the stream doesn't already have one, and advance highest_sid_opened monotonically. 2. The client sent a Host header and no :authority pseudo-header (the :authority code was #if 0'd and the Host-skip guard commented out). h2 carries the authority in :authority (RFC 7540 8.1.2.3); GFE rejects a request with only Host as PROTOCOL_ERROR. Send :authority instead; it is accepted by all h2 servers. 3. The stock SETTINGS_MAX_HEADER_LIST_SIZE of 4096 is smaller than the response headers many real servers send (github.com, GFE, wikipedia), so their streams were GOAWAY'd with ENHANCE_YOUR_CALM. The public http2_settings[] override can't reach this index (the apply loop is bounded by LWS_H2_SETTINGS_LEN), so raise the stock default to 64KiB; actual header storage stays bounded by max_http_header_data.
3148e5e to
251d8a0
Compare
Contributor
Author
|
Landed in main. |
|
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.




Three independent h2-client bugs surfaced testing fetch against real
servers (txiki.js). Symptoms ranged from connections dropped right after
ALPN to RST_STREAM(PROTOCOL_ERROR); they only bit a subset of servers, so
they had gone unnoticed against lenient peers (nghttp2, Cloudflare).
lws_h2_client_handshake() unconditionally reassigned the stream's sid
to highest_sid_opened + 2 at header-send time, overwriting the sid 1
the first client stream is already bound to (-> 3). Strict servers
(Google's GFE) reject a first client stream that isn't sid 1 with
PROTOCOL_ERROR. Only allocate a sid here when the stream doesn't
already have one, and advance highest_sid_opened monotonically.
The client sent a Host header and no :authority pseudo-header (the
:authority code was #if 0'd and the Host-skip guard commented out).
h2 carries the authority in :authority (RFC 7540 8.1.2.3); GFE rejects
a request with only Host as PROTOCOL_ERROR. Send :authority instead;
it is accepted by all h2 servers.
The stock SETTINGS_MAX_HEADER_LIST_SIZE of 4096 is smaller than the
response headers many real servers send (github.com, GFE, wikipedia),
so their streams were GOAWAY'd with ENHANCE_YOUR_CALM. The public
http2_settings[] override can't reach this index (the apply loop is
bounded by LWS_H2_SETTINGS_LEN), so raise the stock default to 64KiB;
actual header storage stays bounded by max_http_header_data.