node:tls: test bytesRead on the sockets that a TLS upgrade wraps

[robobun]

Follow-up to #42304 (closed in favor of #42313). Stacked on #42313. Tests only.

Problem

• node:net: count bytesRead on the native socket handle #42313 moves bytesRead to a counter on the native socket. Its TLS upgrade path has no test: the raw half of the [raw, tls] pair inherits the count of the handle it replaces.
• On main one case of that path is wrong. A net.Socket with an onread buffer that tls.connect({ socket }) wraps reports bytesRead === 0. Node v26.3.0 reports 3055, the TLS records.

Fix

• 4 tests in test/js/node/tls/node-tls-upgrade.test.ts. Three rows (no reader, readable: false, an onread buffer) check that the wrapped socket counts the TLS records: tlsSocket.bytesRead is 6, raw.bytesRead is greater.
• One STARTTLS test checks both wrapped sockets. The count right after the upgrade equals the count before it (2 on the client, 8 on the server). Then it grows.
• Verified on a debug build, expected values from node v26.3.0. With node:net: count bytesRead on the native socket handle #42313's src/ all 9 tests of the file pass. With main's src/ (4b5862f) the an onread buffer row fails with Expected: > 6, Received: 0.

Background

• tls.connect({ socket }) and new tls.TLSSocket(socket, { isServer: true }) run TLS over an existing net.Socket. Bun's native upgradeTLS turns the TCP handle into a [raw, tls] pair over one fd. The wrapped socket keeps raw, which sees the ciphertext.
• In node the upgrade keeps the handle, and the handle counts each read before TLSWrap consumes it. So socket.bytesRead goes on from its value before the upgrade.
• onread: { buffer, callback } makes a socket read into buffer and call callback(nread, buffer).

Notes

Why a separate PR. #42313 had a CI run in progress and no human review yet. A push there restarts the run. If the tests are wanted inside #42313, cherry-pick a6cbad9.

After #42313 merges I rebase this branch on main, so that it holds the one test commit only.

Why the onread row is 0 on main. The data handler that the onread option installs never added to bytesRead (#42304 was the JS fix for that). #42265 then added an early return for the wrapped socket to that handler, before the deliver loop. #42313 counts on the handle before any JS handler runs, so the row passes there.

Probe numbers (tls.connect({ socket: raw }), the server sends "banner"):

	node v26.3.0	main (4b5862f)	#42313 (d2dd882)
raw.bytesRead, no onread	3055	2813	2813
raw.bytesRead, raw has onread	3055	0	2813
calls of the onread callback	0	0	0

STARTTLS probe (plaintext greeting\n, go, ok\n, then TLS in both directions). Client raw.bytesRead: 12 before, 12 right after the upgrade, at the end 3027 in node and 2821 in bun. Server: 2, 2, then 1720 in node and 1588 in bun. Bun 1.4.3, main and #42313 print the same numbers for this probe. Node and bun differ in the size of the handshake, so the tests compare against the plaintext count and not against a fixed number.

[robobun]

^{Updated 1:25 PM PT - Sep 11th, 2026}

@robobun, your commit a6cbad9 is building: #114427

[robobun]

Status: merged into the branch of #42313 as b293a54. The 4 tests now land with #42313. Requested in #42304 (comment).

How I reproduced the case that is wrong on main:

import net from "node:net";
import tls from "node:tls";
import { once } from "node:events";
// key and cert: any self-signed pair for localhost
const server = tls.createServer({ key, cert }, s => s.end("banner"));
await once(server.listen(0, "127.0.0.1"), "listening");
const raw = net.connect({
  port: server.address().port,
  host: "127.0.0.1",
  onread: { buffer: Buffer.alloc(64), callback() {} },
});
await once(raw, "connect");
const secure = tls.connect({ socket: raw, ca: cert, servername: "localhost" });
secure.resume();
await once(secure, "close");
console.log(raw.bytesRead, secure.bytesRead);
server.close();

• node v26.3.0: 3055 6
• main (4b5862f): 0 6
• node:net: count bytesRead on the native socket handle #42313 (d2dd882): 2813 6

test/js/node/tls/node-tls-upgrade.test.ts: with #42313's src/ all 9 tests pass. With main's src/ the an onread buffer row of the new tests fails with Expected: > 6, Received: 0.

[claude]

LGTM — test-only addition that mirrors the file's existing patterns.

What was reviewed:

• Both new tests use port: 0, once() (auto-rejects on 'error'), and try/finally cleanup registered before assertions; no sleeps or per-test timeouts.
• The test.each rows cover the same three raw-socket variants as the existing test above them; the STARTTLS test wires server-side error/close to the awaited resolver so it can't hang.
• Exact plaintext byte counts are asserted with .toBe/.toEqual; only handshake-size-dependent values use .toBeGreaterThan, which the PR justifies against Node v26.3.0 numbers and the cited nodejs/node permalink.

Extended reasoning...

Overview

This PR appends two test cases (72 lines) to test/js/node/tls/node-tls-upgrade.test.ts, exercising bytesRead accounting on both the raw and TLS sockets when tls.connect({ socket }) / new tls.TLSSocket(socket, { isServer: true }) wrap an existing net.Socket. The first is a test.each over three raw-socket configurations (no reader, readable: false, onread buffer); the second models a full STARTTLS exchange and checks that pre-upgrade byte counts are preserved and continue to grow on both peers. No production code is touched.

Security risks

None. The change adds tests only, uses in-process servers on 127.0.0.1 with port: 0, and reuses the harness-provided self-signed cert already imported by the file. Nothing touches auth, crypto primitives, or trust decisions.

Level of scrutiny

Low-to-moderate. Test-only changes to an existing Node-compat test file, following the exact structure of the two tests already in the file (same imports, same once/try-finally/Promise.withResolvers shape, same server setup). I checked against CLAUDE.md, test/CLAUDE.md, and REVIEW.md testing rules: port: 0 is used, no time-based waits, cleanup is in finally before any assertion can throw, once() from events rejects on 'error' so failure paths surface, related assertions use .toEqual on a combined object, and the variant matrix uses test.each. The nodejs/node source permalink and observed-in-Node comments satisfy the Node-compat citation convention.

Other factors

The toBeGreaterThan assertions on raw.bytesRead are the loosest checks here, but they're the strongest invariant available since TLS handshake record sizes differ between Node and Bun (documented in the PR notes with concrete numbers), while the plaintext counts (6, 2, 7, 8) are asserted exactly. No CODEOWNERS entry covers this path. The bug-hunting run exited on dry_streak with no findings, and there are no outstanding third-party reviews on the timeline.