Skip to content

Commit

Permalink
src: deduplicate SetALPN implementations
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
  • Loading branch information
tniessen authored and danielleadams committed Jul 26, 2022
1 parent b4c75a9 commit 17cb272
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 17 deletions.
17 changes: 4 additions & 13 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ namespace node {

using v8::Array;
using v8::ArrayBuffer;
using v8::ArrayBufferView;
using v8::BackingStore;
using v8::Context;
using v8::EscapableHandleScope;
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 2 additions & 3 deletions src/crypto/crypto_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/crypto_tls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 17cb272

Please sign in to comment.