Skip to content

fix(client-certificates): avoid HTTP/2 crash when upstream TLS fails#41114

Draft
yury-s wants to merge 1 commit into
microsoft:mainfrom
yury-s:fix-41105
Draft

fix(client-certificates): avoid HTTP/2 crash when upstream TLS fails#41114
yury-s wants to merge 1 commit into
microsoft:mainfrom
yury-s:fix-41105

Conversation

@yury-s
Copy link
Copy Markdown
Member

@yury-s yury-s commented Jun 3, 2026

Summary

  • In the HTTP/2 503 error path of socksClientCertificatesInterceptor, cleanup was registered on the stream's 'end' event. For GET requests the browser sends END_STREAM with the HEADERS frame, so 'end' can fire and tear down the socket before the 503 DATA write completes — crashing the http2 session with a CHECK(is_write_in_progress()) assertion (SIGABRT).
  • Drop the 'end' listener and queue cleanup right after stream.end(), relying on setImmediate FIFO ordering to flush the DATA write first.
  • Adds a regression test exercising the h2 503 error path.

Note: the crash is a hard-to-hit race that depends on Node's internal http2 write accounting. I confirmed the faulty ordering (cleanup running before the response write finishes) but could not trigger the SIGABRT on Node 22.1–22.22 or 26; the fix removes the latent ordering hazard regardless.

Fixes #41105

When the upstream TLS handshake fails and the browser negotiated h2, the
proxy answers with a 503 over HTTP/2. For GET requests the browser sends
END_STREAM together with the HEADERS frame, so the stream 'end' event could
fire and run cleanup before the 503 DATA write completed. Destroying the
socket mid-write crashes the http2 session with an `is_write_in_progress()`
assertion (SIGABRT).

Drop the 'end' listener and queue cleanup right after stream.end(), relying
on setImmediate FIFO ordering to flush the DATA write first.

Fixes: microsoft#41105
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Test results for "MCP"

7230 passed, 1103 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Test results for "tests 1"

8 failed
❌ [chromium-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @chromium-ubuntu-22.04-arm-node20
❌ [chromium-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @chromium-ubuntu-22.04-node24
❌ [chromium-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/client-certificates.spec.ts:846 › browser › should return target connection errors when using http2 @chromium-ubuntu-22.04-node22
❌ [chromium-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @chromium-ubuntu-22.04-node22
❌ [firefox-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @firefox-ubuntu-22.04-node20
❌ [webkit-library] › library/client-certificates.spec.ts:346 › browser › should not intercept TLS for origins without a client certificate @webkit-ubuntu-22.04-node20
❌ [webkit-library] › library/client-certificates.spec.ts:863 › browser › should not crash on HTTP/2 when the TLS connection to the server fails @webkit-ubuntu-22.04-node20

2 flaky ⚠️ [chromium-library] › library/client-certificates.spec.ts:846 › browser › should return target connection errors when using http2 `@chromium-ubuntu-22.04-arm-node20`
⚠️ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@firefox-ubuntu-22.04-node20`

39549 passed, 775 skipped


Merge workflow run.

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.

HTTP/2 SIGABRT (is_write_in_progress assertion) in socksClientCertificatesInterceptor when TLS connection fails

1 participant