[dylan-conway]

What

On a plain (non-TLS) connection, node:http and node:net sockets had property names that only tls.TLSSocket has in Node: encrypted, _secureEstablished, authorized, authorizationError, servername, _requestCert, _rejectUnauthorized.

Why it matters

"encrypted" in socket is a common way to tell a TLSSocket from a net.Socket. With an own encrypted: false, that test is true for every plain HTTP request. @adonisjs/http-server does exactly this, so request.protocol() returned "https" and request.secure() returned true on plain HTTP.

Cause and fix

• src/js/node/_http_server.ts, one class for both. One socket class served plain and TLS connections. Its constructor always assigned encrypted and _secureEstablished, and the three TLS getters lived on the shared prototype. The TLS-only members are now in a subclass that only a TLS connection gets.
• src/js/node/_http_server.ts, one source for "is this TLS". The three places that make a socket each decided it differently: from the local tls of listen(), from the constructor's options, and from a native flag. Only the first knows about listen({ tls }). The TLS options of a listen() call are now on that call's listenerGeneration, which all three already pass. The class, the 'secureConnection' event, requestCert and resetAndDestroy() read them there. The encrypted constructor argument and the ssl argument of the native clientError callback are gone.
• src/js/node/net.ts. Socket.prototype.connect assigned this.servername on every socket, and the accept path assigned _requestCert and _rejectUnauthorized on every socket. Both now do so only for a TLS socket.

Every reader of these names in src/js that a plain socket can reach tests truthiness, so a missing property behaves like the old false / undefined.

Behaviour

"bug" means different from Node 26.7. listen({ tls }) does not exist in Node. What is right for it is what https.createServer(options) gives.

	cases
bug -> bug	1
bug -> non-bug	7
non-bug -> bug	0
non-bug -> non-bug	0

bug -> bug

	main	now	Node 26.7
index of _secureEstablished and encrypted in Object.keys(req.socket) on an https.Server socket	25 and 27, of 28	27 and 26, of 28	1 and 11, of 39

bug -> non-bug

	main	now	Node 26.7
names on a plain http.Server socket ('connection', req.socket)	encrypted, _secureEstablished, authorized, authorizationError, servername	none	none
names on the res.socket of http.get()	servername	none	none
req.socket.authorized = true on a plain socket, strict mode	throws TypeError	assigns	assigns
names on a net.connect() socket	servername	none	none
names on a socket a net.Server accepted	_requestCert, _rejectUnauthorized	none	none
socket.servername = "mine" before connect() on a plain socket	undefined	"mine"	"mine"
http.createServer().listen({ tls }) (Bun only): req.socket	encrypted=false _secureEstablished=true authorized=undefined authorizationError=undefined servername=undefined, 'secureConnection' not emitted	encrypted=true _secureEstablished=true authorized=false authorizationError=null servername="localhost", 'secureConnection' emitted	no such option

With listen({ tls: { requestCert: true, ... } }) and a client certificate that verifies, authorized is true (main: undefined).

non-bug -> bug

None.

non-bug -> non-bug

None.

Tests

• node-http.test.ts "is not marked encrypted (Express believes protocol=http when HTTPS request #5867)" asserted encrypted === false, which is the behaviour that differs from Node. It now asserts that none of five names is in the 'connection' socket, req.socket, or the client's res.socket. Fails on 1.4.3.
• node-http.test.ts "the socket of ... is marked encrypted (Express believes protocol=http when HTTPS request #5867)" runs for https.createServer(options) and for http.createServer().listen({ tls }), and checks all five values. On 1.4.3 the first passes (it guards the TLS side) and the second fails.
• node-net.test.ts "a plain socket has none of TLSSocket's property names" checks the connecting and the accepted socket. Fails on 1.4.3.
• node-http.test.ts and node-net.test.ts pass in full on a debug build.

[robobun]

^{Updated 4:03 AM PT - Oct 17th, 2026}

❌ @dylan-conway, your commit 95fbba0 has 2 failures in Build #127269 (All Failures):

• test/js/node/http/node-http-uaf.test.ts - code 1 on 🍎 any x64
• step failed outside runner - exit 1 on 🍎 any aarch64 - test-bun

---

🧪   To try this PR locally:

bunx bun-pr 45659

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

bun-45659 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 5ef13d03-7fc5-446a-8ebb-2a4f78ba9827

📥 Commits

Reviewing files that changed from the base of the PR and between 18d1ad5 and 95fbba0.

📒 Files selected for processing: 5

• src/js/node/_http_server.ts
• src/js/node/net.ts
• src/runtime/server/server_body.rs
• test/js/node/http/node-http.test.ts
• test/js/node/net/node-net.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

HTTP server socket creation now uses separate plain and TLS socket classes. TLS-only properties are limited to TLS sockets. Net socket setup and the client-error callback argument list also changed.

Changes

HTTP and net socket TLS properties

Layer / File(s)	Summary
HTTP server socket types and construction src/js/node/_http_server.ts	The HTTP server uses a shared factory to create plain or TLS sockets. TLS-only properties reside on the TLS subclass. resetAndDestroy() checks the listener generation’s TLS configuration or Unix-domain status.
HTTP socket dispatch and callback handling src/js/node/_http_server.ts, src/runtime/server/server_body.rs, test/js/node/http/node-http.test.ts	HTTP dispatch and connection handling use listener-generation TLS configuration. The on_clientError callback no longer receives an SSL boolean. HTTP tests check plain-socket properties and TLS socket properties for both server setup paths.
Net socket TLS field assignment src/js/node/net.ts, test/js/node/net/node-net.test.ts	Accepted sockets receive certificate-request settings only when they have a TLS implementation. Connect setup assigns the initial servername only to TLS sockets. Tests check that plain sockets lack TLS-specific properties.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 95fbb

This change makes plain HTTP and net sockets omit TLS-only property names, and the supplied context shows no concrete merge-blocking risk.

Pre-merge checks: 4

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.
Title check		The title clearly and concisely describes the main change: plain node:http and node:net sockets no longer expose TLSSocket property names.
Description check		The description explains the change, motivation, implementation, behavior, and verification results. It uses different headings from the template, but it provides the required information and is compl…

Full details: Description check

Explanation

The description explains the change, motivation, implementation, behavior, and verification results. It uses different headings from the template, but it provides the required information and is complete.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked that every in-tree reader of encrypted / _secureEstablished / authorized on an HTTP server socket (src/js/thirdparty/ws.ts:3302, the net.ts readStop/half-open guards) tests truthiness, so a missing property behaves like the old false; that the base NodeHTTPServerSocket constructor never reads those names before the TLS subclass's field initialisers run; and that the net.ts guard does not drop servername for TLS clients, since Socket.prototype.connect still assigns it from the TLS options at src/js/node/net.ts:2365.

Extended reasoning...

The change moves TLS-only property names off the shared HTTP server socket into a subclass constructed only for TLS connections, and stops net.Socket.connect from assigning servername on plain sockets; it touches node:http/node:net compat surface but no auth, crypto or input-parsing logic. Two confirmed findings are posted inline, so a human look is already signalled; this note records the adjacent concerns that were examined and ruled out.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.