src: deduplicate SetALPN implementations

tniessen · danielleadams · commit 17cb27237dd2 · 2022-07-25T21:43:34.000-04:00
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: #43756 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
diff --git 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
@@ -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
@@ -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(
