Repository: denoland/deno
Pull request: fix(ext/node): Correctly send ALPN on node TLS connections

Landing work from #21903, plus fixing a node compat bug.

We were always sending the HTTP/2 ALPN on TLS connections which might confuse upstream servers.

Changes:
 - Configure HTTP/2 ALPN when making the TLS connection from the HTTP/2 code
 - Read the `ALPNProtocols` property from the TLS connection options rather than the deno `alpnProtocols` field
 - Add tests

Prereq for landing Deno.serveHttp on Deno.serve: removing older HTTP servers from the codebase.


Changed files:
filename: ext/node/polyfills/_tls_wrap.ts
patch: @@ -96,7 +96,7 @@ export class TLSSocket extends net.Socket {
       caCerts = [new TextDecoder().decode(caCerts)];
     }
     tlsOptions.caCerts = caCerts;
-    tlsOptions.alpnProtocols = ["h2", "http/1.1"];
+    tlsOptions.alpnProtocols = opts.ALPNProtocols;
 
     super({
       handle: _wrapHandle(tlsOptions, socket),
@@ -114,7 +114,7 @@ export class TLSSocket extends net.Socket {
     this.secureConnecting = true;
     this._SNICallback = null;
     this.servername = null;
-    this.alpnProtocols = tlsOptions.alpnProtocols;
+    this.alpnProtocols = tlsOptions.ALPNProtocols;
     this.authorized = false;
     this.authorizationError = null;
     this[kRes] = null;

filename: ext/node/polyfills/http2.ts
patch: @@ -1677,7 +1677,10 @@ export function connect(
       case "https:":
         // TODO(bartlomieju): handle `initializeTLSOptions` here
         url = `https://${host}${port == 443 ? "" : (":" + port)}`;
-        socket = tlsConnect(port, host, { manualStart: true });
+        socket = tlsConnect(port, host, {
+          manualStart: true,
+          ALPNProtocols: ["h2", "http/1.1"],
+        });
         break;
       default:
         throw new ERR_HTTP2_UNSUPPORTED_PROTOCOL(protocol);

filename: tests/unit_node/http_test.ts
patch: @@ -9,7 +9,6 @@ import { assertSpyCalls, spy } from "@std/testing/mock.ts";
 
 import { gzip } from "node:zlib";
 import { Buffer } from "node:buffer";
-import { serve } from "@std/http/server.ts";
 import { execCode } from "../unit/test_util.ts";
 
 Deno.test("[node/http listen]", async () => {
@@ -338,20 +337,18 @@ Deno.test("[node/http] send request with non-chunked body", async () => {
   const hostname = "localhost";
   const port = 4505;
 
-  // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used.
-  // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634
   const handler = async (req: Request) => {
     requestHeaders = req.headers;
     requestBody = await req.text();
     return new Response("ok");
   };
   const abortController = new AbortController();
-  const servePromise = serve(handler, {
+  const servePromise = Deno.serve({
     hostname,
     port,
     signal: abortController.signal,
     onListen: undefined,
-  });
+  }, handler).finished;
 
   const opts: RequestOptions = {
     host: hostname,
@@ -393,20 +390,18 @@ Deno.test("[node/http] send request with chunked body", async () => {
   const hostname = "localhost";
   const port = 4505;
 
-  // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used.
-  // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634
   const handler = async (req: Request) => {
     requestHeaders = req.headers;
     requestBody = await req.text();
     return new Response("ok");
   };
   const abortController = new AbortController();
-  const servePromise = serve(handler, {
+  const servePromise = Deno.serve({
     hostname,
     port,
     signal: abortController.signal,
     onListen: undefined,
-  });
+  }, handler).finished;
 
   const opts: RequestOptions = {
     host: hostname,
@@ -442,20 +437,18 @@ Deno.test("[node/http] send request with chunked body as default", async () => {
   const hostname = "localhost";
   const port = 4505;
 
-  // NOTE: Instead of node/http.createServer(), serve() in std/http/server.ts is used.
-  // https://github.com/denoland/deno_std/pull/2755#discussion_r1005592634
   const handler = async (req: Request) => {
     requestHeaders = req.headers;
     requestBody = await req.text();
     return new Response("ok");
   };
   const abortController = new AbortController();
-  const servePromise = serve(handler, {
+  const servePromise = Deno.serve({
     hostname,
     port,
     signal: abortController.signal,
     onListen: undefined,
-  });
+  }, handler).finished;
 
   const opts: RequestOptions = {
     host: hostname,

filename: tests/unit_node/net_test.ts
patch: @@ -55,6 +55,7 @@ Deno.test("[node/net] net.connect().unref() works", async () => {
   const ctl = new AbortController();
   const server = Deno.serve({
     signal: ctl.signal,
+    port: 0, // any available port will do
     handler: () => new Response("hello"),
     onListen: async ({ port, hostname }) => {
       const { stdout, stderr } = await new Deno.Command(Deno.execPath(), {

filename: tests/unit_node/tls_test.ts
patch: @@ -3,7 +3,6 @@
 import { assertEquals, assertInstanceOf } from "@std/assert/mod.ts";
 import { delay } from "@std/async/delay.ts";
 import { fromFileUrl, join } from "@std/path/mod.ts";
-import { serveTls } from "@std/http/server.ts";
 import * as tls from "node:tls";
 import * as net from "node:net";
 import * as stream from "node:stream";
@@ -13,24 +12,61 @@ const tlsTestdataDir = fromFileUrl(
 );
 const keyFile = join(tlsTestdataDir, "localhost.key");
 const certFile = join(tlsTestdataDir, "localhost.crt");
-const key = await Deno.readTextFile(keyFile);
-const cert = await Deno.readTextFile(certFile);
-const rootCaCert = await Deno.readTextFile(join(tlsTestdataDir, "RootCA.pem"));
+const key = Deno.readTextFileSync(keyFile);
+const cert = Deno.readTextFileSync(certFile);
+const rootCaCert = Deno.readTextFileSync(join(tlsTestdataDir, "RootCA.pem"));
+
+for (
+  const [alpnServer, alpnClient, expected] of [
+    [["a", "b"], ["a"], ["a"]],
+    [["a", "b"], ["b"], ["b"]],
+    [["a", "b"], ["a", "b"], ["a"]],
+    [["a", "b"], [], []],
+    [[], ["a", "b"], []],
+  ]
+) {
+  Deno.test(`tls.connect sends correct ALPN: '${alpnServer}' + '${alpnClient}' = '${expected}'`, async () => {
+    const listener = Deno.listenTls({
+      port: 0,
+      key,
+      cert,
+      alpnProtocols: alpnServer,
+    });
+    const outgoing = tls.connect({
+      host: "localhost",
+      port: listener.addr.port,
+      ALPNProtocols: alpnClient,
+      secureContext: {
+        ca: rootCaCert,
+        // deno-lint-ignore no-explicit-any
+      } as any,
+    });
+
+    const conn = await listener.accept();
+    const handshake = await conn.handshake();
+    assertEquals(handshake.alpnProtocol, expected[0] || null);
+    conn.close();
+    outgoing.destroy();
+    listener.close();
+  });
+}
 
 Deno.test("tls.connect makes tls connection", async () => {
   const ctl = new AbortController();
-  const serve = serveTls(() => new Response("hello"), {
-    port: 8443,
+  let port;
+  const serve = Deno.serve({
+    port: 0,
     key,
     cert,
     signal: ctl.signal,
-  });
+    onListen: (listen) => port = listen.port,
+  }, () => new Response("hello"));
 
   await delay(200);
 
   const conn = tls.connect({
     host: "localhost",
-    port: 8443,
+    port,
     secureContext: {
       ca: rootCaCert,
       // deno-lint-ignore no-explicit-any
@@ -41,26 +77,29 @@ Host: localhost
 Connection: close
 
 `);
-  conn.on("data", (chunk) => {
-    const text = new TextDecoder().decode(chunk);
-    const bodyText = text.split("\r\n\r\n").at(-1)?.trim();
-    assertEquals(bodyText, "hello");
+  const chunk = Promise.withResolvers<Uint8Array>();
+  conn.on("data", (received) => {
     conn.destroy();
     ctl.abort();
+    chunk.resolve(received);
   });
 
-  await serve;
+  await serve.finished;
+
+  const text = new TextDecoder().decode(await chunk.promise);
+  const bodyText = text.split("\r\n\r\n").at(-1)?.trim();
+  assertEquals(bodyText, "hello");
 });
 
 // https://github.com/denoland/deno/pull/20120
 Deno.test("tls.connect mid-read tcp->tls upgrade", async () => {
   const ctl = new AbortController();
-  const serve = serveTls(() => new Response("hello"), {
+  const serve = Deno.serve({
     port: 8443,
     key,
     cert,
     signal: ctl.signal,
-  });
+  }, () => new Response("hello"));
 
   await delay(200);
 
@@ -81,7 +120,7 @@ Deno.test("tls.connect mid-read tcp->tls upgrade", async () => {
     ctl.abort();
   });
 
-  await serve;
+  await serve.finished;
 });
 
 Deno.test("tls.createServer creates a TLS server", async () => {