Skip to content

http2: reject unsupported ALPN protocols#64493

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-http2-unsupported-alpn
Open

http2: reject unsupported ALPN protocols#64493
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-http2-unsupported-alpn

Conversation

@mcollina

Copy link
Copy Markdown
Member

Fixes: #26835

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Jul 14, 2026
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (892976d) to head (b5408ed).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64493      +/-   ##
==========================================
- Coverage   90.24%   90.24%   -0.01%     
==========================================
  Files         741      741              
  Lines      241424   241694     +270     
  Branches    45494    45556      +62     
==========================================
+ Hits       217883   218121     +238     
- Misses      15085    15111      +26     
- Partials     8456     8462       +6     
Files with missing lines Coverage Δ
lib/internal/http2/core.js 94.81% <100.00%> (+<0.01%) ⬆️

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollina mcollina requested review from ZYSzys, jasnell and trivikr and removed request for trivikr July 14, 2026 14:29
@mcollina mcollina force-pushed the fix-http2-unsupported-alpn branch from 9fbd282 to 0eee3d3 Compare July 14, 2026 14:30
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina force-pushed the fix-http2-unsupported-alpn branch from 0eee3d3 to b5408ed Compare July 14, 2026 14:31
@mcollina mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 14, 2026

@pimterry pimterry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The original issue was filed before Node v19, when Node would accept unknown ALPNs as false, and so this case was reachable unexpectedly with a normal HTTP/2 server. That was indeed bad.

Nowadays (as of #44031) we reject those at the TLS level, so this is only reachable for users who explicitly enable their own weird ALPNs (ALPNProtocols or ALPNCallback). Today the hax/13.37 example just fails to connect over TLS at all.

I think that invalidates the issue - it can just be closed. Any code which hits this today is clearly intending to do HTTP/2 on this connection and has opted into accepting this specific ALPN, so HTTP/2 is what we should do.

This is relevant because there's a surprising amount of weird ALPN in the wild:

  • Usage of the draft versions in old clients (spdy/*, h2-*). Until 2024 kube-ctl was still upgrading to spdy/3.1 for all streaming APIs, these are surprisingly widely used.
  • grpc/grpc-exp - old ALPN tokens for GRPC, now deprecated but still implemented in many places.
  • Private & internal APIs: e.g. all of Facebook's current app traffic uses h2-fb for HTTP/2, istio uses istio-h2 on internal mesh traffic. There will be other private HTTP/2 APIs doing similar things in Node today.
  • Workarounds for typos etc: e.g. the v19 ALPN changes broke https-proxy-agent (TooTallNate/proxy-agents#142) with Node servers, because it used http1.1 instead of http/1.1 for ALPN in millions of installs. There'll be others.

I see traffic with these frequently, especially grpc on mobile (I implemented ALPNCallback specifically to handle these). Any handling of these would be broken by this, with only quite awkward workarounds available (performServerHandshake + fully reimplement Http2Server's connection handling on top, compat API becomes completely unavailable).

Agree we should only support h2 & http/1.1 by default. I think that is the current state since v19.

Don't agree we should block users who explicitly configure alternate ALPNs on an H2 server - AFAICT there's no downside to letting people do this and it's perfectly valid, it's just unusual.

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

Labels

http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http2: cannot negotiate ALPN besides http/1.1

3 participants