From 17cb27237dd2af4a74b1dc90dac34bdb9efb17b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Fri, 15 Jul 2022 01:01:17 +0200
Subject: [PATCH] src: deduplicate `SetALPN` implementations

Instead of accepting either a std::string or a mysterious Local<Value>,
accept any std::string_view, which can trivially be constructed from
both strings and ArrayBufferViews.

This also removes the need to check IsArrayBufferView() inside of
SetALPN, which was dead code anyway.

PR-URL: https://github.com/nodejs/node/pull/43756
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
---
 src/crypto/crypto_common.cc | 17 ++++-------------
 src/crypto/crypto_common.h  |  5 ++---
 src/crypto/crypto_tls.cc    |  3 ++-
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc
index 3341859e04c890..eab144cfdf663b 100644
--- a/src/crypto/crypto_common.cc
+++ b/src/crypto/crypto_common.cc
@@ -26,7 +26,6 @@ namespace node {
 
 using v8::Array;
 using v8::ArrayBuffer;
-using v8::ArrayBufferView;
 using v8::BackingStore;
 using v8::Context;
 using v8::EscapableHandleScope;
@@ -87,18 +86,10 @@ void LogSecret(
   keylog_cb(ssl.get(), line.c_str());
 }
 
-bool SetALPN(const SSLPointer& ssl, const std::string& alpn) {
-  return SSL_set_alpn_protos(
-      ssl.get(),
-      reinterpret_cast<const uint8_t*>(alpn.c_str()),
-      alpn.length()) == 0;
-}
-
-bool SetALPN(const SSLPointer& ssl, Local<Value> alpn) {
-  if (!alpn->IsArrayBufferView())
-    return false;
-  ArrayBufferViewContents<unsigned char> protos(alpn.As<ArrayBufferView>());
-  return SSL_set_alpn_protos(ssl.get(), protos.data(), protos.length()) == 0;
+bool SetALPN(const SSLPointer& ssl, std::string_view alpn) {
+  return SSL_set_alpn_protos(ssl.get(),
+                             reinterpret_cast<const uint8_t*>(alpn.data()),
+                             alpn.length()) == 0;
 }
 
 MaybeLocal<Value> GetSSLOCSPResponse(
diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h
index 792760e9707328..55401252cffc1f 100644
--- a/src/crypto/crypto_common.h
+++ b/src/crypto/crypto_common.h
@@ -33,9 +33,8 @@ void LogSecret(
     const unsigned char* secret,
     size_t secretlen);
 
-bool SetALPN(const SSLPointer& ssl, const std::string& alpn);
-
-bool SetALPN(const SSLPointer& ssl, v8::Local<v8::Value> alpn);
+// TODO(tniessen): use std::u8string_view when we switch to C++20.
+bool SetALPN(const SSLPointer& ssl, std::string_view alpn);
 
 v8::MaybeLocal<v8::Value> GetSSLOCSPResponse(
     Environment* env,
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc
index a192956f0f7cfe..6e6ef5251fca0e 100644
--- a/src/crypto/crypto_tls.cc
+++ b/src/crypto/crypto_tls.cc
@@ -1530,7 +1530,8 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowTypeError("Must give a Buffer as first argument");
 
   if (w->is_client()) {
-    CHECK(SetALPN(w->ssl_, args[0]));
+    ArrayBufferViewContents<char> protos(args[0].As<ArrayBufferView>());
+    CHECK(SetALPN(w->ssl_, {protos.data(), protos.length()}));
   } else {
     CHECK(
         w->object()->SetPrivate(