Repository navigation
Conversation
|
Status Reproduced on debug and release builds of main with the new tests in PR: #44394 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Walkthrough
ChangesSynchronous drain handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
The title identifies the TLS and drain-related problem, but it states that the pull request removes a drain listener. The changed implementation instead guards re-entrant drain handling during wrapped… | Rename the pull request to describe the implemented change, for example: "tls: ignore re-entrant drain during wrapped stream writes". |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is complete and relevant. It explains the problem, the fix, known limitations, scope, and verification results. It does not use the template headings exactly, but it provides the requi… Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, the fix, known limitations, scope, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections.
-
Full details: Title check
-
Explanation
The title identifies the TLS and drain-related problem, but it states that the pull request removes a drain listener. The changed implementation instead guards re-entrant drain handling during wrapped stream writes and ends.
Resolution
Rename the pull request to describe the implemented change, for example: "tls: ignore re-entrant drain during wrapped stream writes".
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Autopilot is currently an internal CodeRabbit preview.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked two things: the scopeguard's store to self.writing after write_or_end.call returns adds no new liveness exposure, since the existing error branch on the next line already dereferences self.handlers after the same JS call; and a 'drain' skipped while writing is set only matters when the TLS engine has parked bytes (renegotiation), where a Duplex that emits 'drain' solely from inside write() never delivered a later drain on main either.
Extended reasoning...
The change adds a writing Cell to UpgradedDuplex, sets it for the extent of call_write_or_end via scopeguard, and has the 'drain' host function return early while it is set, plus a node:test file with two cases. It touches the TLS-over-Duplex transport, a re-entrancy-sensitive path where JS runs inside native sends, but no auth or input-parsing surface. A verified finding was posted inline and another was withheld, so this is not an approval; the note records what else was examined.
There was a problem hiding this comment.
I reviewed the updated PR (including b5a402f, which wires raw/server close and error to reject as suggested earlier) and found no bugs. Because this changes re-entrancy semantics at the TLS-over-Duplex boundary and overlaps with the open #43529/#43877 designs, a human look at the chosen guard placement would still be worthwhile.
What was reviewed:
- Guard lifetime: the
scopeguardrestore writesself.writingafter the JSwrite()/end()call returns;DuplexUpgradeContextis only freed via the queuedEventState::Closetask (deinit), never synchronously inside that call, so the restore does not touch freed memory. - Nesting: the save/restore pattern keeps
writingtrue across nestedcall_write_or_endcalls and is per-instance, so a cross-wired peer's handshake writes are not suppressed. - Dropped 'drain': checked that no ciphertext is parked where only the listener could flush it —
handle_trafficdrains the write BIO on every send, and handshake-parked writes are retried fromon_handshakedirectly (not via the listener). - Test: no hardcoded ports, exact-bytes assertion, every failure/close path rejects; node:test format matches the sibling files in
test/js/node/tls/.
Extended reasoning...
The change adds a writing: Cell<bool> flag to UpgradedDuplex in src/runtime/socket/UpgradedDuplex.rs, set via a scopeguard for the duration of call_write_or_end and checked at the top of the on_writable host function so a 'drain' emitted synchronously from inside the Duplex's own write() is ignored, plus a new node:test-format test file covering a pre-handshake write and an HTTP/2 request over such a transport. It touches TLS transport plumbing (unsafe FFI host functions) but no crypto, auth, or input-parsing logic. The diff is small and self-contained, the prior inline nit was addressed by the second commit, and the bug hunt ran dry; deferral rather than approval is because the fix is a re-entrancy guard whose placement (listener only, versus the whole send) is a judgment call the author explicitly measured against alternatives and that overlaps with two other open PRs touching the same window.
|
Superseded by #44618, which consolidates the open TLS pull requests. This fix and its tests are in there, either as written, rewritten smaller, or merged with the other PRs that patched the same cause (see the "By area" list in that PR). Closing in favor of it. |
…ed it (#43877, #44394) UpgradedDuplex called duplex.write(chunk) with no callback and reported the SSL_write count, so write(cb) and end(data, cb) ran their callback while the ciphertext was still queued in the wrapped stream. A handler that exits or destroys in the callback truncated the response. Every duplex.write() and duplex.end() now gets one cached native callback and is counted. $write and $end report false while a call is outstanding, which parks the node:net callback as a partial write does. When the count reaches zero the existing writable path runs on the next tick, as node's JSStreamSocket defers finishWrite, so a stream that runs the callback inside write() cannot re-enter the sender that still holds its bytes. A write the stream fails is reported as node's errnoException(uv[`UV_${err.code}`] || UV_EPIPE, 'write'), also a tick later, so the stream still reports its own error. That callback takes the slot of the 'drain' listener on the wrapped stream, which node does not have. Its last job was a write parked during a renegotiation, which the end of the renegotiation now retries (#37094). A stream that emits 'drain' inside its own write() made the listener send the sender's bytes twice.
…ed it (#43877, #44394) UpgradedDuplex called duplex.write(chunk) with no callback and reported the SSL_write count, so write(cb) and end(data, cb) ran their callback while the ciphertext was still queued in the wrapped stream. A handler that exits or destroys in the callback truncated the response. Every duplex.write() and duplex.end() now gets one cached native callback and is counted. $write and $end report false while a call is outstanding, which parks the node:net callback as a partial write does. When the count reaches zero the existing writable path runs on the next tick, as node's JSStreamSocket defers finishWrite, so a stream that runs the callback inside write() cannot re-enter the sender that still holds its bytes. A write the stream fails is reported as node's errnoException(uv[`UV_${err.code}`] || UV_EPIPE, 'write'), also a tick later, so the stream still reports its own error. That callback takes the slot of the 'drain' listener on the wrapped stream, which node does not have. Its last job was a write parked during a renegotiation, which the end of the renegotiation now retries (#37094). A stream that emits 'drain' inside its own write() made the listener send the sender's bytes twice.
…ed it (#43877, #44394) UpgradedDuplex called duplex.write(chunk) with no callback and reported the SSL_write count, so write(cb) and end(data, cb) ran their callback while the ciphertext was still queued in the wrapped stream. A handler that exits or destroys in the callback truncated the response. Every duplex.write() and duplex.end() now gets one cached native callback and is counted. $write and $end report false while a call is outstanding, which parks the node:net callback as a partial write does. When the count reaches zero the existing writable path runs on the next tick, as node's JSStreamSocket defers finishWrite, so a stream that runs the callback inside write() cannot re-enter the sender that still holds its bytes. A write the stream fails is reported as node's errnoException(uv[`UV_${err.code}`] || UV_EPIPE, 'write'), also a tick later, so the stream still reports its own error. That callback takes the slot of the 'drain' listener on the wrapped stream, which node does not have. Its last job was a write parked during a renegotiation, which the end of the renegotiation now retries (#37094). A stream that emits 'drain' inside its own write() made the listener send the sender's bytes twice.
|
Reopened: in #44618 this was folded into #43877's mechanism (the write-done callback replaced the |
…ed it (#43877, #44394) UpgradedDuplex called duplex.write(chunk) with no callback and reported the SSL_write count, so write(cb) and end(data, cb) ran their callback while the ciphertext was still queued in the wrapped stream. A handler that exits or destroys in the callback truncated the response. Every duplex.write() and duplex.end() now gets one cached native callback and is counted. $write and $end report false while a call is outstanding, which parks the node:net callback as a partial write does. When the count reaches zero the existing writable path runs on the next tick, as node's JSStreamSocket defers finishWrite, so a stream that runs the callback inside write() cannot re-enter the sender that still holds its bytes. A write the stream fails is reported as node's errnoException(uv[`UV_${err.code}`] || UV_EPIPE, 'write'), also a tick later, so the stream still reports its own error. That callback takes the slot of the 'drain' listener on the wrapped stream, which node does not have. Its last job was a write parked during a renegotiation, which the end of the renegotiation now retries (#37094). A stream that emits 'drain' inside its own write() made the listener send the sender's bytes twice.
…ps, WebSocket, SQL) (#44618) ### What does this PR do? Consolidates the open TLS pull requests into one. Each was reproduced on `main` and, for `node:*` behavior, on Node v26.3.0 first. About a third are ported as written, the rest are rewritten smaller or merged into one fix where several PRs patched the same cause. One commit per fix, so it can be read commit by commit. Fixes #43520, fixes #31396, fixes #43635, fixes #37193, fixes #43846, fixes #17932, fixes #41061, fixes #36887, fixes #31810, fixes #35240, fixes #32234, fixes #44365, fixes #43807, fixes #42280, fixes #44517. Addresses #41856 (SNI and `servername`; not `checkServerIdentity` for SQL), #24845 (the spin is gone, shown with fault injection on Linux; not run on macOS), #19754 (node-fetch forwards the agent's TLS options; the Kubernetes client itself was not run). #### The ones that matter most | | On `main` | PRs | |---|---|---| | Client certificate disclosure | `https.request()` with a client certificate sends it to a server it then refuses (wrong name, `checkServerIdentity`, `destroy()` in `'secureConnect'`, `terminate()` in `handshake`). A server can force it with a junk record behind its Finished | #43946 | | False `authorized` | Over a Duplex, `secureConnect` with `authorized === true` for a peer that failed the key proof; `secureConnect` for a plaintext peer with `rejectUnauthorized: false` | #44422, #32929 | | Cleartext https | `https.createServer()` without a usable key/cert answers plain HTTP | #41672, #33539 | | Revoked client certificates | An https mTLS server never sees `crl`, so a revoked client is `authorized` | #41641 | | Pooled sockets | Requests with different client certificates or CAs share an `https.Agent` socket and session | #42498 | | Silent plaintext | `tls: [...]` given to `Bun.listen` / `Bun.connect` is plain TCP | #41490 | | Server weakened by a client knob | `NODE_TLS_REJECT_UNAUTHORIZED=0` turns off a server's client-certificate enforcement | #35245 | | Pins never checked | `WebSocket` never calls `tls.checkServerIdentity` and ignores `tls.serverName` | #41648 | | `verify-full` dropped | `PGSSLMODE=verify-*` is lost next to a `TLS_*` URL variable; `tls: true` sends no SNI | #44498 | | Crashes | use-after-free from `destroy()` in `ALPNCallback` over a Duplex; `abort()` on a late `setSession()`; SIGABRT in `fetch` with an https proxy from the environment and a `Bun.file()` body | #44462, #41671, #44458 | | Stream corruption | A TLS `write()` can lose 16 KiB it reported as written while another socket on the loop is stalled | #44529 | | Hangs and spins | 100% CPU on a failing `send()`; a fatal `SSL_write` leaves the socket open forever; `idleTimeout` never sheds a TLS client that ignores `close_notify` | #34510, #38176, #42336 | | Wrong certificate (regression since 1.3.14) | Connections accepted before `stop()` / `close()` get the default certificate and skip their entry's `requestCert` / `ca` | #42355 | | Quadratic Duplex / proxy tunnel | Reading one chunk over a Duplex, CPU: 8 MB 0.88 s → 0.14 s, 16 MB 3.18 s → 0.23 s, 32 MB 11.75 s → 0.39 s; `fetch` upload through CONNECT: 1.7 s → 0.18 s (debug build) | #44464 | #### By area - **fd engine, write path** (`openssl.c`, `socket.c`): #42352, #34510 + #38176 + #42336 as one change, #44529, #44458, #44192. A rejected `send()` ends the write side only and closes at the next writable event unless the peer's bytes are still queued (a 413 sent before a reset is still read). No new per-socket state. Also, on kqueue, **a FIN no longer ends a socket that waits in the low-priority queue** (`loop.c`): with more than 5 TLS handshakes at once, a client that ended right after its handshake could be reset and its server socket report `socket hang up`, because the eof that the sentinel read knote reports was acted on ahead of the unread Finished. That is on `main` too (the macOS entry for `node-tls-server.test.ts` in `test/flaky-tests.txt`: 7 of 48 recent builds of other branches), and this branch made it likelier (6 of 8 builds), since Finished now leaves in one segment with the close_notify. - **Error reporting, both engines**: #44422, #32929, #44516, #37094, #41272 + #42324 + #44223 as one change, #44021, #37472, #43946, #33630. One channel: a fatal error on an established session is reported, then **the engine closes the connection itself**, whatever the owner does with the report. `test/js/bun/net/tls-fatal-error-closes.test.ts` asserts closed-and-nothing-delivered for every owner (node:tls, `Bun.connect`, `Bun.listen`, `fetch` direct and through CONNECT, `Bun.serve`, `WebSocket` direct and through a proxy, Postgres, MySQL, Valkey, Duplex). - **Duplex engine** (`SSLWrapper`, `UpgradedDuplex`): #44462, #43529, #42332, #44464. #43877 + #44394 were in and are **out again**, see "Worth a look" 5. - **node:tls wrap lifecycle** (`net.ts`, `tls.ts`): #38007, #38058, #38028 + #38122 + #38076 as one change (six copies of the attach code become two helpers), #38311, #39008, #38154, #42340 + #42343 + #42339 + #42453 as one change, #43791, #42425, #44085, #42683, #39088, #39040, #40375, and what was still real of #36534. - **SNI, ALPN, server contexts**: #43080, #42050, #37195 + #43849 as one change (**one** SNI matcher for TCP and HTTP/3), #42355, #42285, #33253, part of #37896, part of #37013. A `tls.Server` has one `SSL_CTX`. - **Verification and options**: #44738, #41490, #37005 + the cwd pin of #40984, #31811, #43982, #33483 + #35245, #41810, #32235, #44441, #38092. - **node:tls API and CA store**: #41671, #38145, #32824, #43594, #39997, #41696, #33534, #34748, #42991, #42996, #42970. - **node:https, Agent, `ws`, node-fetch**: #41672 (https half), #41641, #38261, #42498, #44346, #35609, #31397, #42325. - **WebSocket client**: #41648, #37487 + #43048 as one change. - **SQL, Redis**: #33666, #41711, #44498, part of #42054. - **Tests only**: #41426, #40040, #44395, #44016, #37860, #40591, #44440, #41424. Found on the way and fixed here: an upload that a TLS 1.2 server interrupts with a renegotiation never completes on `main` (0 of 32 runs over `https.request`, `fetch`, `node:tls` and `Bun.connect`: the renegotiation ClientHello lands inside an application record that is still unsent, or the socket gets no `drain` again) and completes here, with two tests from robobun; the fix for #40653 (final flight and first write in one segment) stopped working whenever another TLS socket on the loop was stalled, on `main` too; the `tls.Server` prototype pinned the last server constructed and every `SSL_CTX` it owned; `Object.create(process.env).NODE_TLS_REJECT_UNAUTHORIZED = "0"` turned verification off process-wide once a `SHARE_ENV` worker existed; two debug panics when wrapping a shut-down or still-connecting socket; a `fetch` POST through a proxy sent its headers twice when the origin renegotiated; `BlockList` ignored IPv6 zone ids; a test now ties `root_certs.der` to `certdata.txt`. #### Behavior changes - **A server's `ca` without `requestCert` no longer asks for a client certificate** (`Bun.serve`, `Bun.listen`, HTTP/3, node:tls). It matches the docs and Node. On `main` such a server refused clients with no certificate but served any unrelated self-signed one, so it was never authentication. **Set `requestCert: true` to require a certificate.** A matrix test pins that `requestCert: true` still refuses no certificate and an untrusted one on 8 kinds of server, TLS 1.2 and 1.3, with `NODE_TLS_REJECT_UNAUTHORIZED` unset and `0`. - `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer relaxes a server. - `Bun.connect` / `Bun.listen` hear of a fatal TLS error after the handshake through `error(socket, err)`. With no `error` handler the socket just closes. - HTTP/3 server names match like TCP: `*.` covers exactly one label, case is ignored, a trailing dot is ignored, the last registration of a name wins. - `requestCert` on node:https is `=== true`, as in Node. - An array where a generated options dictionary is expected throws (`tls: []`, `jest.useFakeTimers([])`). - `key` / `cert` arrays serve every identity. A client that can use both gets ECDSA, where `main` served whichever pair came last. - `ecdhCurve` is forwarded by node:https, `ws` and node-fetch now, so a group BoringSSL lacks (`X448`) throws there as it already does in `tls.createServer`. - A wrapped socket's error is re-emitted on the TLS socket as in Node, so `raw.destroy(err)` with a listener on `raw` only is uncaught, as in Node. - `sql.options.tls` is always an object, never `true`. `RedisClient` sends SNI. - `tls: { secureContext }` alone asks for TLS on `Bun.listen` / `Bun.connect` (it was plain TCP), and a value that is not a `SecureContext` throws. The context is served as it is: the `requestCert` / `rejectUnauthorized` it was created with hold whatever the options next to it say, and `requestCert` in the options over a context that does not ask throws at `listen()`. - `tls.DEFAULT_CIPHERS` reaches every client once assigned (`fetch`, `WebSocket`, `Bun.connect`, `RedisClient`, `Bun.SQL`, `S3Client`, proxy tunnels) and servers again. A list that selects no cipher throws `ERR_SSL_NO_CIPHER_MATCH` at the assignment. `fetch.preconnect()` dials nothing after an assignment. - The warning for an unreadable `NODE_EXTRA_CA_CERTS` is Node's one line, without the `warn:` prefix. - `BUN_CONFIG_WS_CLOSE_TIMEOUT` (default 30 s): how long a `WebSocket` client waits for the server to close the connection after the closing handshake. #### Worth a look in review 1. **#44529**: the kernel-refused remainder of a TLS write moves from the loop's one slot onto the connection (in the existing rare struct), so the write BIO never refuses a sealed record. Nothing is allocated on an unstalled path (200 writes: 0 appends, same `send()` count as `main`), memory with 16 stalled writers is lower than on `main` (276 KB vs 340 KB, which `main` holds inside BoringSSL's buffers), `us_socket_t` stays 80 bytes. It needs a bound on how long a deferred close waits, or a peer that stops reading pins the fd past `destroy()`: `US_SSL_CLOSE_AFTER_SPILL_TIMEOUT` is a fixed 10 s, not re-armed on progress. Separate commits, but the fix that keeps the client certificate off the wire beside a stalled socket builds on them. 2. **The default name check of node:tls also runs inside the handshake**, so a wrong-name server gets no client certificate on TLS 1.2 either. JS still runs it after every successful handshake, so a difference between the two matchers can only refuse. Error objects are byte-identical. 3. **#44441** widens trust by design: a self-issued leaf whose `keyUsage` lacks `keyCertSign` (`dotnet dev-certs`) is its own anchor when the store holds a byte-identical copy. No BoringSSL change. Expired pin, same subject with another key, wrong EKU and a pinned intermediate are tested to fail. 4. **#32235** only adds Ed25519 and ECDSA P-521 to the verify list. A captured ClientHello shows `main`'s list with the two inserted; `rsa_pkcs1_sha1` stays. 5. **A stream that a TLS socket wraps, when that TLS socket closes.** An earlier state of this branch lost data here while CI was green (found by #44709's report): with the peer closing first, 4 of 8 MiB arrived with TLS in TLS, 4 of 32 MiB on the http2 `emit("connection")` path, and a `write()` with no `'error'` listener ended the process. Three Node-parity changes only hold together: destroying the wrapped stream at the close (#38028 + #38122 + #38076, #38154) is safe only if every write has really completed (#43877), which in turn needs Node's handling of the peer's close_notify, which needs half-open sockets that the GC can collect. So: - #43877 + #44394 are reverted and reopened. A write over a stream completes once the stream has taken the ciphertext, as on `main`. - Until the verdict on the peer lets the session through, the application cannot have written over it. There the wrapped stream is destroyed as in Node, with the sessions below it. That keeps the release of the connection after a failed handshake, a rejected certificate and an early `destroy()`. The same for an http2 socket the application never got, and for `resetAndDestroy()`. - After that it is `main`'s teardown: a `net.Socket` only gets the engine's `end()`, closes at its peer's FIN, keeps its own timeout and reports its own errors. Any other stream is destroyed with the TLS socket. The regular suites cannot see any of this (999 files were green on every broken variant), so it was steered by eleven seeded differential fuzzers run on this build, `main`, Node v26.3.0 and the earlier state: close, `end()`, `destroy()`, `destroySoon()`, resets, hung and half-open peers, paused writers, timeouts, two and three sessions deep, over TCP and over Duplexes, before, at and after the handshake, and http2 requests. See "How did you verify". #### Known limits - `fetch` with a `checkServerIdentity` function still sends the client certificate (not the request) to a server the function refuses. On TLS 1.2 any verdict a JS callback gives is too late, as in Node. - `addContext()` / `SNICallback` still do not apply to a server-side socket on the stream engine (`emit("connection", duplex)`, TLS in TLS, unflushed writes, named pipes), as on `main`. - A CA bundled in a pfx extends an explicit `ca` only, for `ws` / node-fetch / `WebSocket`: the native `ca` can only replace the default store, and that store keeps `SSL_CERT_FILE` / `SSL_CERT_DIR`. - P-521 leaves work on TLS 1.3 only. TLS 1.2 needs secp521r1 in every ClientHello (`it.todo`). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch(url, { protocol: "http3" })` is `HTTP3Unsupported`, as with an explicit `ciphers`. - `addCACert()` by hand does not extend the chains of a context with several identities. - A throwing `ALPNCallback` sends `no_application_protocol` on both engines. Node sends nothing and its client sees `ECONNRESET`. - TLS in TLS, peer FIN while the outer handshake runs: the inner socket gets one `write EPIPE`, where Node gives `ECONNRESET` (`main` gives it no error at all). - `@SECLEVEL` in `ciphers` is dropped by the `ws` / node-fetch shims, which used to ignore `ciphers`. node:tls keeps throwing `ERR_SSL_INVALID_COMMAND`. - Beside a stalled TLS socket only the first record (16 KiB) of the first write leaves with the handshake flight. The rest goes record by record, which is what bounds the memory of stalled writers. - After a fatal error on an established session the socket emits `'error'` and then `'close'`. Node emits `'error'` and leaves the socket open. - A paused reader whose own write the kernel rejects loses what it had not read yet, with an `EPIPE`, as on Node. `main` reports no error there and delivers it. - On `main` too: a `Bun.listen` socket without `allowHalfOpen` that has unsent ciphertext when the client's `shutdown()` arrives loses that ciphertext (32 KiB), and over plain TCP `end()` with the peer still sending is a close over unread input, so a reset. - Differences from both `main` and Node that the differential runs below found and that stay, all with a peer that aborts: `ECONNRESET` instead of a clean `'end'` after the socket's own `'finish'` when the peer destroyed with unread data; under TLS 1.2, a zero-length `write()` followed by `destroy()` in `'secureConnection'` leaves the client without `'secureConnect'` (a plain `destroy()` there matches Node); a TLS 1.2 client that destroys in `'secureConnect'` gets no `'session'`; a `ClientRequest` whose handshake fails with an alert emits `'error'` and `'close'` but no `'finish'` (`writableFinished` is true). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch.preconnect()` opens nothing: `fetch()` then uses a context of its own, and a socket warmed under the default one would never be picked up. - A TLS `send()` that the kernel refuses outside a `write()` call (the drain of unsent ciphertext) is reported with the close, as `read EPIPE` / `read ECONNRESET`. Node says `write EPIPE`. `main` does not report it at all. - Once the application has a session over a `net.Socket` (TLS in TLS, http2 `emit("connection")`), a peer that never sends its FIN holds that socket after the TLS socket closed, as on `main`. Node destroys it. Two tests of #38154 are `todo` for this. Closing it any earlier (at its `'finish'`, say) makes the kernel drop what it has not sent yet as soon as the peer's close_notify arrives. - Plaintext that was queued on a socket before it was wrapped (STARTTLS with a backlog) is dropped when the TLS socket is destroyed, or its handshake fails, before the session is accepted. Node drops it too, except on `destroySoon()`. `main` sends it. - Over a stream that is no `net.Socket`, `end()` can still cut what that stream has buffered, and there is no backpressure, both as on `main` (#43877). - `tls.secureContext` (the undocumented door node:tls uses) is not read by a Windows named pipe listener, which builds its context from the options. On `upgradeTLS({ isServer: true })` the options next to it are the policy, as with Node's `SetVerifyMode`. - `selectServerName()` rebuilds the name tree per ClientHello for injected sockets of a server with `addContext()` entries: 0.4 µs for 1 entry, 3.7 µs for 10, 41 µs for 100, against 631–1111 µs for a handshake. #### Not included Left open, because they need a decision or are not TLS: #43877 + #44394 (see "Worth a look" 5; #43874 stays open with them), #38548, #38591 (both shrink who is trusted), #41589 (`verify-full` vs `NODE_TLS_REJECT_UNAUTHORIZED=0`), #37197, #41706, #43216, #33487, #33545, #36707, #32435, #37255, #28691, #40275, #30314 (features), #38120 (needs the BoringSSL fork, as did #33517, which the stale bot has closed since), #38529 (needs a Windows measurement), #34342, #38232, #43089, #44454, #40451, #42710, #44527, #38088, #38093, #41898. #37896, #42054 and #37013 stay open for the halves not taken. `http.createServer({ key, cert })` keeps serving TLS on purpose. One open question: `tls: {}` (an object that names no TLS option) is plain TCP on `Bun.listen` / `Bun.connect`, here and on `main`. It is the same trap as `tls: []`, but changing it changes a Bun default, so it is left alone. ### How did you verify your code works? - Every new test fails on `main` for the stated reason and passes here, except guards that pin existing behavior, each shown to fail when its clause is removed. `node:*` tests also pass on Node v26.3.0; the few that cannot say which Node version has the behavior. - 212 test files that touch TLS, sockets, http, http2, fetch, WebSocket, SQL, Valkey and workers: 6154 pass, 2 fail. Both are seen on `main` too: `serve.test.ts` "root range port" (the box runs as root), and `worker_threads.test.ts` "terminate(): nothing of the worker's runs after the request", which is flaky there and passed in the run below. - 58 of those files the way the ASAN lane runs them (LeakSanitizer + `BUN_JSC_validateExceptionChecks`): 58 files, 48 of them with leak checking, 4132 pass, 3 fail. All three also fail on `main`: `serve.test.ts` "root range port", `node-net.test.ts` "should not leak when connect({path}) fails synchronously on a reused handle" (times out under this environment), `worker_threads.test.ts` "process.exit() with a shell cp in flight" (a `ShellCpTask` leak). - 647 vendored `test-tls-*`, `test-https-*`, `test-net-*`, `test-http2-*`: the only two failures also fail on `main`. - The SNI matcher was diffed against both old matchers: 3 seeds × 1.23 M lookups × 3 registration flavours, every difference in one of the intended classes, TCP and HTTP/3 identical on every lookup. - The headline rows were also driven by hand with scripts against this build, `main` and Node v26.3.0: cleartext https, `crl`, `tls: []` / `{ secureContext }`, the `ca` / `requestCert` matrix, the client certificate on a wrong-name server, late `setSession()`, `destroy()` in `ALPNCallback`, `[rsa, ec]` identities with an intermediate from `ca`, `WebSocket` `checkServerIdentity`, a corrupted record, the Duplex read above, `tls.DEFAULT_CIPHERS`. - The `setSession()` guard was checked against the real `abort()` at 43 handshake states. - `bun run rust:check-all`: 12 of 12 targets. `tsc`, oxlint, source lints, prettier, rustfmt, mordant clean. - usockets' `_Nonnull` is compiled out of debug builds, so 105 of those files were also run on a local release ASAN build with the CI runner's environment (92 with leak checking): 4595 pass, 1 fail, `child_process.test.ts` "spawn reports EPERM after dropping privileges", which cannot pass as root and fails on `main` too. - The close of a TLS socket over another stream ("Worth a look" 5): eleven seeded differential fuzzers, 8,424 scenarios compared, each run on a release ASAN build of this branch, on `main`, on Node v26.3.0 and on the earlier state of the branch. Against `main`: - Data that `main` delivers in full is cut in 5 scenarios, and about 150 that `main` cuts arrive in full. Of the 5, in 2 `main` never notices the peer's close and keeps the socket for good, 2 call `end()` on the middle one of three sessions over an in-memory Duplex, and 1 does the same on Node. - No dead timeout, no silent reset and no uncaught error that `main` does not have (4 uncaught errors fewer). - A socket stays open where `main` closes it in 109, and closes where `main` keeps it in 295. 92 of the 109 do the same on Node or on the earlier state (a `destroy()` that an in-memory Duplex does not show its peer, half-open peers). 14 wait for a peer that paused reading and so does not read the FIN (#42332's backpressure, as in Node); the socket's own timeout fires there. 3 are left: one on a 5 ms timer, two with three sessions over an in-memory Duplex. - The earlier state of the branch cut data in 173 of the 400 scenarios of one of them, where `main` cuts none and this cuts none. - 23 new tests pin what they found. Each earlier attempt at this fix fails the ones that describe it, the earlier state of the branch fails 7, and all pass on Node. - After that change: 999 test files on the release ASAN build (20,246 pass; the 11 files that fail need a database, Docker, DNS or a non-root user, or share a temp directory with a parallel run and pass alone), 61 on the debug build. - TLS over a file descriptor (`openssl.c`, the path of `fetch`, `Bun.serve`, `tls.connect`, `Bun.connect`) got the same treatment after the rebase: seeded differential fuzzers on CI's release build of this branch, on `main` and, for `node:*`, on Node v26.3.0. Every runtime also against itself for the noise floor, injected faults and known bugs of `main` as positive controls, and a difference counts only if it shows in 5 of 5 fresh processes. - `node:tls` over TCP: 11,500 scenarios (one connection with Node as the oracle line by line; 2 to 60 connections beside stalled neighbours; raw peers that break the handshake). HTTPS: about 136,000 runs over `Bun.serve` + `fetch`, `node:https`, `node:http2` and `wss://`, also with the two ends in different runtimes. `Bun.connect` / `Bun.listen` / `upgradeTLS`: 11,500 scenarios and 720 slow connections, with writers driven by what `write()` returns, beside up to 6 stalled, dripping, closing or resetting neighbours, and plain TCP as a second oracle. No crash, hang, duplication, reordering or silent truncation, and no change in time or in connection reuse. - They found six things that `main` does better, none of which any test showed. All are fixed, each with a test that fails on the build before: what the peer sent lost behind a rejected `send()` (23 scenarios, and an early HTTPS response lost with only `EPIPE`), the same silently for a paused reader, `server.close()` never calling back on a half-open server after a ClientHello and a reset (17), `closeAllConnections()` taking 12 s with a stalled client, `end()` losing up to 1.3 of 4 MiB that `write()` had reported while the peer still uploads, and `end()` a little after a stall never closing beside other stalled TLS sockets. The last two fixes also deliver the 1 to 2 MiB that `main` loses there, and close the socket that `main` keeps for good without such neighbours. - All of them again after every fix, on CI's release build of it. That caught one regression of a fix itself (a reader stopped for backpressure lost 86,385 bytes, 1 of 6,000 scenarios), fixed too. On the last build: scenarios that lose data where `main` does not 23 → 2, and Node loses it in both, with the same `EPIPE`; `server.close()` that never calls back 17 → 0; connections held 4 → 0; requests that end in an error only where `main` has a response 6 → 0. With a Node server in another process, a request ends in an error only in 8 and 10 of 1,500 scenarios here, 5 and 3 on `main`, 10 with Node as the client. - `Bun.connect` / `Bun.listen` on the last build against `main`, in scenarios: hangs 0 against 1,031, sockets and fds never released 0 against 965, corrupted data 0 against 345, `abort()` 0 against 26 (`setSession()` after the handshake), writers that never close 0 against 101 of 720 connections. No kind of failure shows here and not on `main`. About a third of the slow connections close later than on `main`, in 1 to 16 s instead of at once, waiting for unsent ciphertext or for the peer's close_notify, and 79 more of them deliver all that `write()` reported. RSS and time with 16 to 256 stalled writers are the same. - What they found that `main` does worse: a `WebSocket` that calls `close()` with sends pending loses messages in 81 of 999 scenarios (0 here), 37 server sockets left open, 10 `server.close()` that never call back, 20 write callbacks that never run. - The kqueue fix cannot be run on Linux. The `connectionListener` count test now says what became of a missing connection, which is how the cause was found (`'tlsClientError'` "socket hang up", then `read ECONNRESET` at the client of the same port, after its `'secureConnect'`). On macOS x64 it failed every attempt of the three builds before the fix and passed at the first attempt of the build with it. - Windows and macOS were only run by CI. Four new tests asserted what only the Linux kernel does (a FIN read ahead of a reset, unread bytes surviving a reset, loopback buffer sizes, `fstat()` on a socket) and now say so per platform. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
robobun
changed the title
b5a402f to
1c49c40
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@Jarred-Sumner I reworked this PR after the reopen. The head is now What it is now
The question for you Can the deletion land before #43877? What I have for it is a run of 360 seeded close scenarios from the list in your comment on #43877. Node v26.3.0,
Not in this PR
I can open either of the first two as its own PR. I cannot run Windows. The named-pipe tests used the listener, so CI decides there. |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline nit, I also looked at the rewritten R-2 comments in src/runtime/socket/socket_body.rs (write_or_end_buffered and internal_flush): the re-entry they describe — a Duplex write() that calls flush()/terminate() on this handle while the buffered_data_for_node_net projection is live — predates this PR, and removing the native drain thunk closes the one re-entry path the runtime itself used to trigger, so this change narrows the hazard rather than adding to it. I also grepped src/ for leftover consumers of the old 4-element handler tuple (events[3], duplexOnWritable, on_writable_callback) and found none.
Extended reasoning...
The change removes the 'drain' listener thunk from UpgradedDuplex (Rust host fn, GC-visited slot, handler array shrunk to data/end/close) and updates both JS consumers (net.ts attachTLSEngine and _http2_upgrade.ts), adds a >2^31-1 byte write guard on stream-backed TLS handles in net.ts, and ships tests. It touches the TLS-over-stream path but no auth or credential handling. The only remaining native code touching the lent queue slice is unchanged; the comment edits there document a pre-existing re-entrancy hazard that the PR narrows. One inline nit was posted (a fixture that reports SKIP as a pass).
1c49c40 to
8ab1223
Compare
A TLS socket over a stream listened for 'drain' on that stream. The listener ran the socket's flush and its drain callback. Node's JSStreamSocket has no such listener. A stream that emits 'drain' inside its own write() ran that listener inside the send that called write(). NewSocket::internal_flush lends its queue to that send and settles it afterwards, so the nested flush sent the queue again: 524288 bytes for 262144, and a bad record MAC for HTTP/2 over such a stream. Bun 1.4.3 does this too. Since #44618 a TLS socket over a net.Socket that is still dialing is `connecting`, and node:net parks a write made then. The 'connect' of the socket and a drain of its handle each send that parked chunk. The listener delivered such a drain before the 'connect' when the dialing socket had a backlog of its own, so "hello" arrived as "hellohello". Without the listener no stock transport delivers a drain there. A net.Socket subclass that emits 'connect' after the handshake still does. The listener was also what sent the rest of one chunk above 2^31 - 1 bytes, because SSLWrapper::write_data takes at most that in one call. Socket.prototype._write now fails such a write with EPROTO on this engine, as Node does on every TLS socket. An HTTP/2 session writes through the handle: there the rest of such a batch now waits for the next write of the session. The two comments that said a send cannot reach the pending-write queue now state the condition: no JS that the send runs reaches the queue. The deletion is the one of d7b42e1, without the write-done callback that commit put in the freed slot. When write(cb) completes does not change. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
8ab1223 to
6a5875e
Compare
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline nit, I also checked what else relied on the removed 'drain' thunk: writes parked before the handshake or mid-renegotiation are retried by on_handshake -> handlers.on_writable (UpgradedDuplex.rs:225-228, reached again from handle_end_of_renegotiation), and SSLWrapper::write_data takes a whole chunk below c_int::MAX, so no sub-limit queue is left unflushed. UV_EPROTO is exported by the uv binding, and no reference to the removed slot or host fn remains under src/. One small thing: the header comment of node-tls-write-limit.fixture.ts names node-tls-connect.test.ts, but the test that spawns it lives in node-tls-upgrade.test.ts.
Extended reasoning...
The change removes the 'drain' listener thunk from the TLS-over-Duplex engine across net.ts, UpgradedDuplex.rs, sockets.classes.ts and socket_body.rs comments, and adds a marker-gated EPROTO guard in Socket.prototype._write for chunks above 2^31-1 bytes, with new tests and a 2 GiB fixture. It touches the TLS write path (security-adjacent native code) and leaves the pre-existing queue-lending across a JS-running send in place, which the updated comments now describe as conditionally valid. A nit on the explicit test timeout is posted inline; the ruled-out items above are what this run newly examined.
| expect(exitCode).toBe(0); | ||
| }, | ||
| // A build that does not refuse the chunk seals 2 GiB before the fixture can say so. | ||
| 30_000, |
There was a problem hiding this comment.
🟡 nit (optional): the new 2 GiB write-limit test passes an explicit 30_000 timeout, which test/CLAUDE.md forbids ("Do not set a timeout on tests"). The third argument at node-tls-upgrade.test.ts:742 raises this one test's budget sixfold instead of shrinking the work. Fix: drop the timeout argument and keep the test inside the default budget, for example by having the fixture report the first sealed chunk and exit at once, which it already does with process.exit(1) in its transport write().
Why this was flagged
test/js/node/tls/node-tls-upgrade.test.ts:722-743 registers the fixture-spawning test with a trailing 30_000 argument at line 742, with a comment that a build which does not refuse the chunk seals 2 GiB first. test/CLAUDE.md under "No timeouts" says never to set a per-test timeout, and the repository review guidance says not to raise a per-test timeout to make a slow test pass but to shrink the workload. The fixture at test/js/node/tls/node-tls-write-limit.fixture.ts:57-60 already exits with code 1 as soon as a transport write above 2 ** 26 bytes arrives, so a non-refusing build fails fast without the extra budget. On the base branch this test does not exist; after merging, this one test runs under a 30 s cap that masks a slow passing path on debug or ASAN lanes.
Verification: nit. Triggers whenever this test file runs: /home/claude/bun/test/js/node/tls/node-tls-upgrade.test.ts:742 passes the explicit per-test timeout 30_000. /home/claude/bun/test/CLAUDE.md:118-120 says "Do not set a timeout on tests. Bun already has timeouts." Nothing breaks on a correct build; this is a CLAUDE.md rule violation, severity nit.
Problem
'drain'insidewrite().524288 !== 262144bytes arrive, and HTTP/2 fails withDECRYPTION_FAILED_OR_BAD_RECORD_MAC. Node is correct.'drain'listener on the stream (src/js/node/net.ts:519) re-entersNewSocket::internal_flush(src/runtime/socket/socket_body.rs:3175) while its queue is lent to the send.hellohello). node:net also sends a write parked for'connect'at a drain, which the listener delivers first.Fix
EPROTO, as on Node.test/js/node/tls/node-tls-upgrade.test.tsandrenegotiation.test.ts. 9 fail on main. Alsotest/js/node/{tls,http2,net}and 660 Node tests.Background
stream.write()inside the send.internal_flushlends that send the writes queued before the handshake.'drain'raised inside the call into the stream (first version):hellohellostays.Downsides
'drain'.net.Socketsubclass that emits'connect'after the handshake still gets a parked write twice.Notes
What this PR is now. The first version kept the listener and ignored a
'drain'raised inside the engine's call into the stream. This version removes the listener. It has the deletion of d7b42e1 (#44618) and not the write-done callback that commit put in the freed slot.write(cb)andend(cb)complete as on main. The reasons given for the reverts (#43877 (comment)) are about write completion and close.Does the deletion hold without the write-done callback? I ran 360 seeded close scenarios built from the list in that comment: the peer closes first with more queued than the kernel takes, two-way traffic, paused readers,
destroySoon(), resets, half-open peers, timeouts on the wrapped socket, three sessions deep, and the window around the handshake. A scenario records the events of the closing socket, of its transport and of its peer, the byte counts and the end states. Node v26.3.0, main and this PR ran each scenario twice in each of three modes: the stream emits no'drain'of its own, it emits one on the next turn of the loop after each write, or it emits one insidewrite(). A scenario counts when every runtime gave the same record in both of its runs.'drain''drain''drain'write()'drain'insidewrite(), this PR and main differ in 2 of 703 records. Both are the number of bytes that reached a peer which then destroyed or reset the connection (16,384 or 49,152 of 262,144). Both values occur on both builds across runs.'drain'insidewrite(), main's record changes in 28 scenarios. In 13 of them the peer gets the payload twice. In others the early drain completes a write that fails at the close without it. This PR's record changes in 1, which is the same byte-count race.The two doubles.
'drain'insidewrite().internal_flushpasses a slice ofbuffered_data_for_node_netto the send and settles the queue afterwards. Over a stream the send callsstream.write(). A'drain'emitted there reached the listener, which raninternal_flushagain: the queue was still whole, so it went out a second time. Bun 1.4.3 does this too. The listener came with feat(tls) add duplex upgrade #13718.hellohello.raw = net.connect(port); raw.write(backlog); new tls.TLSSocket(raw, { isServer: true }).write("hello"). Since tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618 the TLS socket isconnectingwhilerawdials (linkUpgraded,net.ts:497), soSocket.prototype._writeparks the chunk (net.ts:3032). Two things send a parked chunk: theonce("connect")closure there, and the drain handlers (net.ts:836,:1694), which send_pendingData. At its'connect',rawfirst sends its own backlog and emits'drain'for it. The listener turned that into a drain of the TLS handle. Then the'connect'reached the TLS socket and the closure sent the chunk again. No lent queue is involved.bytesWrittenc6da4a4d3)hellohellohellohellohellonet.Socketwith at leastwritableHighWaterMarkbytes written to it, wrapped as a TLS server, and a TLS write before the TCP connect.'connect'. Anet.Socketsubclass that has no handle, carries the handshake, and emits'connect'after it still getshellohellowithERR_MULTIPLE_CALLBACK, on main and here. Node sends once. Bun 1.4.3 sends once.net.ts(not built). The other is in the server wrap (net.ts:2692): testconnection.connectingbeforehasUnflushedWrites(connection). A dialing socket then waits for its connect and gets the fd-backed engine when its backlog has left, as the client path atnet.ts:2416does. With only that change, main sendshelloonce. Neither is in this PR. I can open either as its own PR.The tail.
SSLWrapper::write_data(src/uws/lib.rs:908) takes at mostc_int::MAXbytes of one call. Over a stream, the listener was the only thing that sent the rest. Node fails such a write on every TLS socket: https://github.com/nodejs/node/blob/v26.3.0/src/crypto/crypto_tls.cc#L1105-L1126.Socket.prototype._writehas the check, after theconnectingpark and before the closed-handle check. It is not an own_writeon the socket: a subclass_writestill runs.attachTLSEnginemarks the handle it installs (kTLSOverStream), so the check applies to this engine only and ends with the handle. An fd-backed TLS socket still sends a long chunk in parts, as on main.chunk.length > 715827882. Below that length no string or buffer can reach the limit, so a shorter write skips the byte count.bytesWritten. Bun does not.Socket.prototype._write. Its writer joins the frames of one flow-control window into one batch. One write of about 2^31 - 1 bytes to a peer whose windows are at the maximum makes a batch above the limit. The engine takes 2^31 - 1 bytes of it, and the rest now waits for the next write or flush of the session. On main the listener flushed it when the stream emitted'drain'. I did not run this case: it needs about 12 GiB.What the sender still does. This PR came from a review of #44291, which asked that the queue not be lent across a send that runs JS. That is still open.
internal_flush(socket_body.rs:3187) and the slow path ofwrite_or_end_buffered(:2909) still pass a slice of the queue to a send that runs the stream'swrite().SSL_writecopies the slice into its memory BIO before the transport runs any JS, and the code after the send reads the queue again, not the slice. An ASAN build gives no report.write().socket._handle.flush()there, during the flush of a parked 64 KiB write: 131,072 bytes arrive for 65,536. After a secondsocket._write()called directly while the first write is parked, aflush(), aterminate()or aresetAndDestroy()there ends the process withpanic: range start index 12 out of range for slice of length 0. Both results are the same on main and with this PR.socket_body.rsit changes only those two comments and one more, so it meets socket: drain node:net's pending-write queue with a cursor #44219, socket: every edit of the pending-write queue goes through one method #44291 and socket: shutdown() sends the FIN after queued bytes; a close frees the queue and keeps bytesWritten #44300 in comment lines only.release()at the top ofNewSocket::on_close(socket: shutdown() sends the FIN after queued bytes; a close frees the queue and keeps bytesWritten #44300): over a stream it frees the queue during such a send, asclose_and_detachalready does on main forresetAndDestroy(). Nothing reads the slice afterwards.Related. #43877 (open) keeps the listener and adds a write-done thunk beside it. #43874 and #43877 are about when a write over a stream completes. An earlier version of this PR also edited
_http2_upgrade.ts. #46270 deleted that file:tls.Servernow wraps a socket that is injected into anHttp2SecureServer, so that path goes throughattachTLSEngineor the fd-backed engine.Measurements. Release builds of
91e7e3b140(an earlier main) and of this PR's source diff on it, linux-x64. They were taken before the rebase onto the current main. The diff then also had the edits of_http2_upgrade.ts, and the rest of it is unchanged.bun-profiletext / data / bssbunfile sizetls.connect({ socket }): Function cells / heap byteslistenerCount('drain')on the wrapped streamUpgradedDuplex/DuplexUpgradeContext/JSTLSSocketbyteswrite(chunk, cb), read fromperf_event_opencounters with the concurrent JIT off and the process on one core. A figure is the median of 5 runs (3 for fd-backed TLS and for the two "after" rows). A run is 200 batches of 10,000 writes, and its value is the median of its last third. The runs of one cell span at most 5 instructions in the first two rows and at most 17 in the others.Socket.prototype._writesee two classes once such a socket has written. Both orders of the check cost the same there.NewSocket<*>,*SocketPrototype__*,UpgradedDuplex, theSSLWrapperover it,us_socket_write*), 351 have the same size and the same instructions. The other two areUpgradedDuplex::teardown(716 to 657 bytes) and one abort handler (38 to 41). The listener's host function (287 bytes) is gone.perf,valgrind,strace,bloaty.Still wrong on main and with this PR.
write()override emits'drain': a 512 KiB POST arrives with all 524,288 bytes in the wrong order.http2.tshas its own'drain'listeners (:4566,:5816) that run the frame writer. Node is correct.stream.duplexPair(), where the server echoes and the client callsend()from its write callback: the client gets 65,536 of 262,144 bytes, and 65,536 of 1,048,576. Node gets all.TLSSocket.end(cb)/write(cb)fire before the wrapped stream's backlog is flushed #43874 and node:tls: complete write(cb) and end(cb) over a Duplex when the stream completes the ciphertext #43877 are about that.Tests.
node-tls-upgrade.test.ts: 9 new tests. With main'ssrc/on a debug build 8 fail. The ninth ("a subclass's _write() runs for each write") passes on main and pins where the length check lives.renegotiation.test.ts: the test of a write made in the middle of a renegotiation now also runs over a Duplex that emits'drain'in itswrite(). That case fails on main.node-tls-write-limit.fixture.tsmakes the writes of 2^31 bytes in a child process. The buffer is never written, so the child stays small. A build without the check seals 2 GiB before the fixture can report it, which is why the test has a 30 second timeout. The test is skipped where a probe process cannot reserve such a buffer.>=for>, the string arm, the encoding, the marker, and the prefilter.node-tls-upgradetests, checked with scripts that mirror them. The exceptions are the cases of the fixture that close the handle or use a string that Node cannot build.'drain'insidewrite()doubles too, and the dialing-socket case is correct. I checked both with scripts: the test file uses wraps that 1.4.3 does not have.test/js/node/tls,test/js/node/http2andtest/js/node/net, andtest/js/bun/net/socket-retention.test.ts. 45 files have no failure. The failures in the other 7 are 5 second timeouts of tests that spawn processes, four tests that connect to bun.sh and one that resolves www.example.com (this machine reaches neither),node-tls-root-certs-concurrent-init, and the RST_STREAM flood test ofh2-conformance. The timeouts of tests that run TLS over a stream pass when those tests run alone.test/js/node/test/parallel): 655 pass. The 5 that fail are the 5 that failed on both builds before the rebase.bun run rust:check-all: 12 of 12 targets, before the rebase. The Rust diff did not change in it.Left as is. Comments in
node-tls-connect.test.ts:2455,:2532andsocket-retention.test.ts:194,:251still count four listener functions. There are three now. This PR does not touch those files.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/tls/node-tls-upgrade.test.ts