Skip to content

h2: client: fix interop with strict servers and large response headers#3614

Closed
saghul wants to merge 1 commit into
warmcat:mainfrom
saghul:fix-h2-client-header-sid-authority
Closed

h2: client: fix interop with strict servers and large response headers#3614
saghul wants to merge 1 commit into
warmcat:mainfrom
saghul:fix-h2-client-header-sid-authority

Conversation

@saghul

@saghul saghul commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

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.

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.
@lws-team lws-team force-pushed the main branch 3 times, most recently from 3148e5e to 251d8a0 Compare June 20, 2026 05:13
@saghul

saghul commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Landed in main.

@saghul saghul closed this Jun 20, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant