diff --git a/ext/node/polyfills/_tls_wrap.ts b/ext/node/polyfills/_tls_wrap.ts index dc7dac77ac..6697bc97ac 100644 --- a/ext/node/polyfills/_tls_wrap.ts +++ b/ext/node/polyfills/_tls_wrap.ts @@ -79,7 +79,7 @@ export class TLSSocket extends net.Socket { ssl: any; _start() { - this[kHandle].afterConnect(); + this[kHandle].afterConnectTls(); } constructor(socket: any, opts: any = kEmptyObject) { @@ -150,16 +150,14 @@ export class TLSSocket extends net.Socket { const { promise, resolve } = Promise.withResolvers(); - // Patches `afterConnect` hook to replace TCP conn with TLS conn - const afterConnect = handle.afterConnect; - handle.afterConnect = async (req: any, status: number) => { + // Set `afterConnectTls` hook. This is called in the `afterConnect` method of net.Socket + handle.afterConnectTls = async () => { options.hostname ??= undefined; // coerce to undefined if null, startTls expects hostname to be undefined if (tlssock._needsSockInitWorkaround) { // skips the TLS handshake for @npmcli/agent as it's handled by // onSocket handler of ClientRequest object. tlssock.emit("secure"); tlssock.removeListener("end", onConnectEnd); - return afterConnect.call(handle, req, status); } try { @@ -190,7 +188,6 @@ export class TLSSocket extends net.Socket { } catch { // TODO(kt3k): Handle this } - return afterConnect.call(handle, req, status); }; handle.upgrading = promise; diff --git a/ext/node/polyfills/net.ts b/ext/node/polyfills/net.ts index 2d57507c1b..8fde8eac1e 100644 --- a/ext/node/polyfills/net.ts +++ b/ext/node/polyfills/net.ts @@ -378,6 +378,12 @@ function _afterConnect( socket.emit("connect"); socket.emit("ready"); + // Deno specific: run tls handshake if it's from a tls socket + // This swaps the handle[kStreamBaseField] from TcpConn to TlsConn + if (typeof handle.afterConnectTls === "function") { + handle.afterConnectTls(); + } + // Start the first read, or get an immediate EOF. // this doesn't actually consume any bytes, because len=0. if (readable && !socket.isPaused()) { diff --git a/tests/unit_node/tls_test.ts b/tests/unit_node/tls_test.ts index f34d9efb5b..97d753e4f8 100644 --- a/tests/unit_node/tls_test.ts +++ b/tests/unit_node/tls_test.ts @@ -12,6 +12,7 @@ import { fromFileUrl, join } from "@std/path"; import * as tls from "node:tls"; import * as net from "node:net"; import * as stream from "node:stream"; +import { text } from "node:stream/consumers"; import { execCode } from "../unit/test_util.ts"; const tlsTestdataDir = fromFileUrl( @@ -97,6 +98,18 @@ Connection: close assertEquals(bodyText, "hello"); }); +// Regression at https://github.com/denoland/deno/issues/27652 +Deno.test("tls.connect makes tls connection to example.com", async () => { + const socket = tls.connect(443, "example.com"); + await new Promise((resolve) => { + socket.on("secureConnect", resolve); + }); + socket.write( + "GET / HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n", + ); + assertStringIncludes(await text(socket), "Example Domain"); +}); + // https://github.com/denoland/deno/pull/20120 Deno.test("tls.connect mid-read tcp->tls upgrade", async () => { const { promise, resolve } = Promise.withResolvers();