usockets(tls): FIN the TCP write side after SSL_shutdown completes on a half-open socket

[robobun]

What does this PR do?

Problem

us_internal_ssl_shutdown only takes the SSL_shutdown() path when SSL_RECEIVED_SHUTDOWN is already set (the !SSL_in_init && !RECEIVED_SHUTDOWN branch at the top handles the common case with a raw TCP half-close instead). That precondition is met for an allow_half_open TLS server socket whose peer has already half-closed: ssl_on_end sets SSL_RECEIVED_SHUTDOWN before dispatching 'end', and the ZERO_RETURN path sets it by receiving the peer's close_notify.

With RECEIVED_SHUTDOWN already set, SSL_shutdown() sends our close_notify and returns 1. The function then returned without calling us_internal_socket_raw_shutdown, so:

• no TCP FIN was ever sent to the peer (only the close_notify TLS record);
• the poll type stayed SOCKET, not SHUT_DOWN;
• on epoll, the half-open poll had already been changed to WRITABLE when the peer's FIN was handled, the writable dispatch drops it to 0 once drained, and EPOLLHUP (the only thing reported at events=0) needs both halves FIN'd, so the loop's is_shut_down close path never ran.

Every other exit from us_internal_ssl_shutdown already calls us_internal_socket_raw_shutdown; only the ret >= 0 path did not.

Reachable path

The only sockets that reach the SSL_shutdown() path after a peer half-close are UWS_HTTP_TLS sockets with allow_half_open set: node:https server CONNECT/Upgrade tunnels (upgradeToTunnelMode sets allow_half_open), where the JS 'end' handler's socket.end() routes through handle.end() → us_socket_shutdown. net.Socket's Duplex autoDestroy: true normally follows with handle.close(), which hid this; with autoDestroy disabled the socket never closes.

Fix

Call us_internal_socket_raw_shutdown(s) after the SSL_shutdown branch regardless of ret, matching the other exit paths. This sends the TCP FIN and moves the poll type to SHUT_DOWN.

Relation to #34487

Once the FIN is sent, epoll reports EPOLLHUP (both halves down) and the socket closes via the is_shut_down eof branch. kqueue and libuv still need the readable_ended re-arm from #34487 for the poll to re-derive eof at events=0; this PR is the TLS-side prerequisite that gets the FIN on the wire at all.

How did you verify your code works?

New test in test/js/node/http/node-http-connect.test.ts spawns an https CONNECT server with autoDestroy disabled, a TLS client that half-closes first (raw FIN, no close_notify first), and waits for the server socket's 'close':

# before
(fail) … https CONNECT socket.end() after peer FIN half-closes TCP so the socket can close
  ^ this test timed out after 5000ms.
# after
(pass) … https CONNECT socket.end() after peer FIN half-closes TCP so the socket can close

Gated to Linux because the close is observed via EPOLLHUP; kqueue/libuv coverage comes with #34487.

Also ran with no new failures vs main: test/js/node/tls/ (159/161), test/js/bun/net/socket.test.ts + test/js/node/net/ (249/268), and test/js/node/test/parallel/test-{tls,https}-*.js (207/210).

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/http/node-http-connect.test.ts

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0272f705-68cb-40fb-9941-794e46815c9a

📥 Commits

Reviewing files that changed from the base of the PR and between 0693416 and ce8d734.

📒 Files selected for processing (1)

• test/js/node/http/node-http-connect.test.ts

---

Walkthrough

Changes

The TLS shutdown path now sends the TCP FIN after TLS shutdown completion. A Linux-only HTTPS CONNECT regression test covers half-open TLS socket teardown and verifies event ordering and successful process exit.

TLS CONNECT shutdown

Layer / File(s)	Summary
TLS shutdown ordering packages/bun-usockets/src/crypto/openssl.c	Moves TCP raw shutdown until after the TLS shutdown reaches RECEIVED_SHUTDOWN.
CONNECT half-close regression test/js/node/http/node-http-connect.test.ts	Adds a Linux-only HTTPS CONNECT test using a half-open TLS-wrapped socket and validates shutdown ordering, logs, and process exit.

Possibly related PRs

• oven-sh/bun#34356: Also adjusts socket shutdown sequencing to allow buffered data to drain before peer FIN handling.

Suggested reviewers: cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the TLS shutdown fix and mentions the half-open socket behavior changed in this PR.
Description check	✅ Passed	The description matches the required template and includes both the change summary and verification details.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/http/node-http-connect.test.ts`:
- Line 609: Update the subprocess wait around proc.stdout.text(),
proc.stderr.text(), and proc.exited to race process completion against an
unref’d timeout that terminates proc; assert that the timeout branch did not
fire, while preserving collection of stdout, stderr, and exitCode on normal
completion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2eaca298-360f-4f99-bc7c-93d33d6da3a1

📥 Commits

Reviewing files that changed from the base of the PR and between 6618e7f and ceb36da.

📒 Files selected for processing (2)

• packages/bun-usockets/src/crypto/openssl.c
• test/js/node/http/node-http-connect.test.ts

[github-actions]

Found 1 issue this PR may fix:

1. Bun http.request() emits ECONNRESET after successful HTTP 101 Upgrade response #32222 - Server socket.end() on a TLS upgrade socket routes through us_internal_ssl_shutdown, and the missing TCP FIN would cause the peer to see ECONNRESET instead of a clean close, matching the reported symptom

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #32222

🤖 Generated with Claude Code

[claude]

No issues found — the one-line move of us_internal_socket_raw_shutdown out of the ret < 0 branch makes the ret >= 0 path consistent with every other exit from us_internal_ssl_shutdown, and the test nit from the earlier pass was addressed in 0693416. Deferring to a human because this is TLS shutdown ordering in usockets with epoll/kqueue lifecycle implications and ties into #34487; worth a quick look from someone who owns this layer.

Checked: the ret >= 0 path is only reachable with SSL_RECEIVED_SHUTDOWN already set (the !in_init && !RECEIVED_SHUTDOWN guard above returns early otherwise, and in_init returns at 2227), so FIN'ing the write side here cannot truncate readable data. Also confirmed the ret < 0 branch has no early return, so moving the call out preserves its behavior.

Extended reasoning...

Overview

The PR moves a single us_internal_socket_raw_shutdown(s) call in packages/bun-usockets/src/crypto/openssl.c from inside the if (ret < 0) block to after it, so that the TCP FIN is sent on the SSL_shutdown() >= 0 path as well. It adds a Linux-gated https CONNECT regression test that hangs without the fix and exits cleanly with it.

Security risks

This is TLS shutdown ordering. The change does not weaken any verification — it only adds a TCP-level FIN after the TLS close_notify has already been sent on a path where the peer's shutdown was already received. I traced the guards: this branch is only reached when SSL_RECEIVED_SHUTDOWN is set (or SSL_in_init, which returns earlier at line 2227), so there is no risk of FIN'ing the write side while application reads are still expected. No new attack surface.

Level of scrutiny

Medium-high. The diff is tiny and the reasoning in the PR description is thorough and checks out against the surrounding code, but this is native C in the TLS socket lifecycle with platform-specific event-loop coupling (EPOLLHUP vs kqueue/libuv), and it is one half of a two-PR fix alongside #34487. That combination is exactly where a maintainer familiar with usockets internals should confirm the model rather than a bot approving.

Other factors

• All other exit paths in us_internal_ssl_shutdown already call us_internal_socket_raw_shutdown; this closes the one gap, which argues strongly for correctness.
• The ret < 0 branch has no early return, so hoisting the call is behavior-preserving for the error path.
• The earlier inline nit (standalone expect(stderr).toBe("")) was addressed in 0693416 by folding into a combined .toEqual({...}) — confirmed in the current diff.
• PR author reports the tls/net/https parallel suites pass with no new failures.
• CodeRabbit suggested cirospaciari as reviewer, which matches ownership of this layer.

[robobun]

Self-review probed 34 angles across edge cases, blast radius, and coverage; none survived. The unconditional us_internal_socket_raw_shutdown after SSL_shutdown is reachable only with SSL_RECEIVED_SHUTDOWN already set (so FIN'ing cannot truncate anything the peer would still read), and the ret < 0 path it was hoisted from has no early return, so its behavior is preserved.

Diff is green. The new test passes on every Linux lane; no TLS/https/CONNECT failures anywhere.

CI on builds 74649 and 74670 reports only pre-existing/flaky failures unrelated to this diff: require-cache (darwin, pre-existing on main), http2-reset-flood (ASAN assertion, pre-existing on main), plus retried-then-passed flakes in es-module-lexer/spawn/repl-close/fs-promises-file-handle/compile-windows-metadata/cron/security-scanner/webview/filter-workspace/local-sql/no-orphans/20144. The two net-adjacent flakes (node-net.test.ts mimalloc page-count threshold at exactly 10, serve-protocols.test.ts HTTP/3 stream reset on darwin) both passed on retry and touch code paths this diff does not.

Ready for a maintainer to merge.

[robobun]

^{Updated 12:47 PM PT - Jul 17th, 2026}

❌ @robobun, your commit ce8d734 has 1 failures in Build #74670 (All Failures):

• test/cli/run/require-cache.test.ts - code 1 on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 34498

That installs a local version of the PR into your bun-34498 executable, so you can run:

bun-34498 --bun