From 0d3a2f7d1117dfad7dda1bd36aa1422baa855233 Mon Sep 17 00:00:00 2001
From: Tim Perry <pimterry@gmail.com>
Date: Fri, 5 May 2023 12:57:53 +0200
Subject: [PATCH] Update ALPNCallback param names & other tweaks after review

---
 doc/api/tls.md                               |  8 ++++----
 lib/_tls_wrap.js                             | 16 ++++++++++------
 test/parallel/test-tls-alpn-server-client.js |  8 ++++----
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/doc/api/tls.md b/doc/api/tls.md
index 326443a3660f1b..69029b36d2f7e7 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -2045,13 +2045,13 @@ changes:
     e.g. `0x05hello0x05world`, where the first byte is the length of the next
     protocol name. Passing an array is usually much simpler, e.g.
     `['hello', 'world']`. (Protocols should be ordered by their priority.)
-  * `ALPNCallback(params)`: {Function} If set, this will be called when a
+  * `ALPNCallback`: {Function} If set, this will be called when a
     client opens a connection using the ALPN extension. One argument will
-    be passed to the callback: an object containing `serverName` and
-    `clientALPNProtocols` fields, respectively containing the server name from
+    be passed to the callback: an object containing `servername` and
+    `protocols` fields, respectively containing the server name from
     the SNI extension (if any) and an array of ALPN protocol name strings. The
     callback must return either one of the strings listed in
-    `clientALPNProtocols`, which will be returned to the client as the selected
+    `protocols`, which will be returned to the client as the selected
     ALPN protocol, or `undefined`, to reject the connection with a fatal alert.
     If a string is returned that does not match one of the client's ALPN
     protocols, an error will be thrown. This option cannot be used with the
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 49b2e6ff6e65c9..36a1f14486fe34 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -109,6 +109,7 @@ const kErrorEmitted = Symbol('error-emitted');
 const kHandshakeTimeout = Symbol('handshake-timeout');
 const kRes = Symbol('res');
 const kSNICallback = Symbol('snicallback');
+const kALPNCallback = Symbol('alpncallback');
 const kEnableTrace = Symbol('enableTrace');
 const kPskCallback = Symbol('pskcallback');
 const kPskIdentityHint = Symbol('pskidentityhint');
@@ -239,7 +240,7 @@ function callALPNCallback(protocolsBuffer) {
   const handle = this;
   const socket = handle[owner_symbol];
 
-  const serverName = handle.getServername();
+  const servername = handle.getServername();
 
   // Collect all the protocols from the given buffer:
   const protocols = [];
@@ -254,9 +255,9 @@ function callALPNCallback(protocolsBuffer) {
     protocols.push(protocol.toString('ascii'));
   }
 
-  const selectedProtocol = socket._ALPNCallback({
-    serverName,
-    clientALPNProtocols: protocols
+  const selectedProtocol = socket[kALPNCallback]({
+    servername,
+    protocols,
   });
 
   // Undefined -> all proposed protocols rejected
@@ -534,7 +535,7 @@ function TLSSocket(socket, opts) {
   this._controlReleased = false;
   this.secureConnecting = true;
   this._SNICallback = null;
-  this._ALPNCallback = null;
+  this[kALPNCallback] = null;
   this.servername = null;
   this.alpnProtocol = null;
   this.authorized = false;
@@ -761,8 +762,11 @@ TLSSocket.prototype._init = function(socket, wrap) {
     ssl.handshakes = 0;
 
     if (options.ALPNCallback) {
+      if (typeof options.ALPNCallback !== 'function') {
+        throw new ERR_INVALID_ARG_TYPE('options.ALPNCallback', 'Function', options.ALPNCallback);
+      }
       assert(typeof options.ALPNCallback === 'function');
-      this._ALPNCallback = options.ALPNCallback;
+      this[kALPNCallback] = options.ALPNCallback;
       ssl.ALPNCallback = callALPNCallback;
       ssl.enableALPNCb();
     }
diff --git a/test/parallel/test-tls-alpn-server-client.js b/test/parallel/test-tls-alpn-server-client.js
index 1e1c0aa4768144..a77c15b77ab254 100644
--- a/test/parallel/test-tls-alpn-server-client.js
+++ b/test/parallel/test-tls-alpn-server-client.js
@@ -221,9 +221,9 @@ function TestFatalAlert() {
 function TestALPNCallback() {
   // Server always selects the client's 2nd preference:
   const serverOptions = {
-    ALPNCallback: ({ clientALPNProtocols }) => {
-      return clientALPNProtocols[1];
-    }
+    ALPNCallback: common.mustCall(({ protocols }) => {
+      return protocols[1];
+    }, 2)
   };
 
   const clientsOptions = [{
@@ -249,7 +249,7 @@ function TestALPNCallback() {
 function TestBadALPNCallback() {
   // Server always returns a fixed invalid value:
   const serverOptions = {
-    ALPNCallback: () => 'http/5'
+    ALPNCallback: common.mustCall(() => 'http/5')
   };
 
   const clientsOptions = [{