Skip to content

feat(http2): configure header list size for tcp/uds#8673

Merged
aaronArinder merged 1 commit intodevfrom
aaron/http2-header-config-tcp-uds
Nov 26, 2025
Merged

feat(http2): configure header list size for tcp/uds#8673
aaronArinder merged 1 commit intodevfrom
aaron/http2-header-config-tcp-uds

Conversation

@aaronArinder
Copy link
Contributor


Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

  • PR description explains the motivation for the change and relevant context for reviewing
  • PR description links appropriate GitHub/Jira tickets (creating when necessary)
  • Changeset is included for user-facing changes
  • Changes are compatible1
  • Documentation2 completed
  • Performance impact assessed and acceptable
  • Metrics and logs are added3 and documented
  • Tests added and passing4
    • Unit tests
    • Integration tests
    • Manual tests, as necessary

Exceptions

Note any exceptions here

Notes

Footnotes

  1. It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this.

  2. Configuration is an important part of many changes. Where applicable please try to document configuration examples.

  3. A lot of (if not most) features benefit from built-in observability and debug-level logs. Please read this guidance on metrics best-practices.

  4. Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions.

@aaronArinder aaronArinder requested a review from a team November 24, 2025 22:01
@apollo-librarian
Copy link

apollo-librarian bot commented Nov 24, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 1 changed, 0 removed
* graphos/routing/(latest)/performance/caching/response-caching/customization.mdx

Build ID: dacd44e79706b137ce76a4ad
Build Logs: View logs

URL: https://www.apollographql.com/docs/deploy-preview/dacd44e79706b137ce76a4ad

@github-actions

This comment has been minimized.

@aaronArinder aaronArinder force-pushed the aaron/http2-header-config-tcp-uds branch from 202718e to a1cf753 Compare November 25, 2025 14:11
@aaronArinder aaronArinder requested a review from a team as a code owner November 25, 2025 15:09
if let Some(max_header_list_size) = opt_max_http2_headers_list_bytes {
// ByteSize has a .as_u64(), but doesn't have a as_u32(), to explain
// this funky `as` conversion
builder.max_header_list_size(max_header_list_size.as_u64() as u32);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it happen that the header size would be bigger than u32::MAX ? If so I would prefer using u32::try_from() instead of as cast. WDYT ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically yes, but practically I don't think so; there'd have to be ~4.3gb of headers for u32 (and almost 20k petabytes for usize), which would probably choke the router somewhere earlier; but, I've added a try_from() and a warning log just in case (because who knows what folks get up to)

}

if let Some(max_buf_size) = opt_max_http1_buf_size {
builder.max_buf_size(max_buf_size.as_u64() as usize);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here with the cast

@aaronArinder aaronArinder force-pushed the aaron/http2-header-config-tcp-uds branch from 54fb945 to e548b8b Compare November 26, 2025 16:15
@aaronArinder aaronArinder enabled auto-merge (squash) November 26, 2025 16:17
@aaronArinder aaronArinder merged commit fe00ac0 into dev Nov 26, 2025
15 checks passed
@aaronArinder aaronArinder deleted the aaron/http2-header-config-tcp-uds branch November 26, 2025 17:26
@yliu-intuit
Copy link

Hi @aaronArinder , the fix didn't work with our TCP connections. We continue to receive 431 when sending 10KB single header despite setting the config of http2_max_headers_list_bytes: "48KiB".

The issue is fixed when specifically check for HTTP2 connection and apply http2_only() on the builder.

This is caused by using the auto-detection builder in this PR. It happens as follows as a possible explanation.

  1. State Machine Initialization: When an HTTP/2 connection object is created, it must start with some rules for how much memory it allows the peer to use.

  2. The Default: The h2 crate defaults the SETTINGS_MAX_HEADER_LIST_SIZE to 16,384 bytes (16KB).

  3. The Update: If you configure a larger size (e.g., 800KB), the server must send a SETTINGS frame to the client to say "I actually accept 800KB."

  4. The Gap: In "Auto" mode, the server buffers the incoming bytes to detect the protocol. Once it detects "Oh, this is HTTP/2," it creates the H2 connection object. Critically, if it parses the incoming data before it has time to apply your configuration overrides and process the SETTINGS update, it uses the Initial Default (16KB).

I have a fix that force using http2_only() and it solves the problem. you can check here. https://github.com/yliu-intuit/router/pull/1/files

However, I am still looking into why #8631 works. #8631 is using the same logic as in this PR.

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.

4 participants