Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bf17f8d

Browse filesBrowse files
bnoordhuisdanielleadams
bnoordhuis
authored and
danielleadams
committed
src: optimize ALPN callback
It doesn't make sense from a performance perspective to retain an arraybuffer with the ALPN byte string and look it up as a property on the JS context object for every TLS handshake. Store the byte string in the C++ TLSWrap object instead. That's both a lot faster and a lot simpler. PR-URL: #44875 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent d433d34 commit bf17f8d
Copy full SHA for bf17f8d

File tree

Expand file treeCollapse file tree

5 files changed

+19
-35
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+19
-35
lines changed
Open diff view settings
Collapse file

‎lib/_tls_wrap.js‎

Copy file name to clipboardExpand all lines: lib/_tls_wrap.js
+2-5Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -786,11 +786,8 @@ TLSSocket.prototype._init = function(socket, wrap) {
786786
ssl.enableCertCb();
787787
}
788788

789-
if (options.ALPNProtocols) {
790-
// Keep reference in secureContext not to be GC-ed
791-
ssl._secureContext.alpnBuffer = options.ALPNProtocols;
792-
ssl.setALPNProtocols(ssl._secureContext.alpnBuffer);
793-
}
789+
if (options.ALPNProtocols)
790+
ssl.setALPNProtocols(options.ALPNProtocols);
794791

795792
if (options.pskCallback && ssl.enablePskCallback) {
796793
validateFunction(options.pskCallback, 'pskCallback');
Collapse file

‎src/crypto/crypto_tls.cc‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_tls.cc
+13-28Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -225,26 +225,17 @@ int SelectALPNCallback(
225225
const unsigned char* in,
226226
unsigned int inlen,
227227
void* arg) {
228-
TLSWrap* w = static_cast<TLSWrap*>(SSL_get_app_data(s));
229-
Environment* env = w->env();
230-
HandleScope handle_scope(env->isolate());
231-
Context::Scope context_scope(env->context());
228+
TLSWrap* w = static_cast<TLSWrap*>(arg);
229+
const std::vector<unsigned char>& alpn_protos = w->alpn_protos_;
232230

233-
Local<Value> alpn_buffer =
234-
w->object()->GetPrivate(
235-
env->context(),
236-
env->alpn_buffer_private_symbol()).FromMaybe(Local<Value>());
237-
if (UNLIKELY(alpn_buffer.IsEmpty()) || !alpn_buffer->IsArrayBufferView())
238-
return SSL_TLSEXT_ERR_NOACK;
231+
if (alpn_protos.empty()) return SSL_TLSEXT_ERR_NOACK;
239232

240-
ArrayBufferViewContents<unsigned char> alpn_protos(alpn_buffer);
241-
int status = SSL_select_next_proto(
242-
const_cast<unsigned char**>(out),
243-
outlen,
244-
alpn_protos.data(),
245-
alpn_protos.length(),
246-
in,
247-
inlen);
233+
int status = SSL_select_next_proto(const_cast<unsigned char**>(out),
234+
outlen,
235+
alpn_protos.data(),
236+
alpn_protos.size(),
237+
in,
238+
inlen);
248239

249240
// According to 3.2. Protocol Selection of RFC7301, fatal
250241
// no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not
@@ -1529,20 +1520,14 @@ void TLSWrap::SetALPNProtocols(const FunctionCallbackInfo<Value>& args) {
15291520
if (args.Length() < 1 || !Buffer::HasInstance(args[0]))
15301521
return env->ThrowTypeError("Must give a Buffer as first argument");
15311522

1523+
ArrayBufferViewContents<uint8_t> protos(args[0].As<ArrayBufferView>());
15321524
SSL* ssl = w->ssl_.get();
15331525
if (w->is_client()) {
1534-
ArrayBufferViewContents<uint8_t> protos(args[0].As<ArrayBufferView>());
15351526
CHECK_EQ(0, SSL_set_alpn_protos(ssl, protos.data(), protos.length()));
15361527
} else {
1537-
CHECK(
1538-
w->object()->SetPrivate(
1539-
env->context(),
1540-
env->alpn_buffer_private_symbol(),
1541-
args[0]).FromJust());
1542-
// Server should select ALPN protocol from list of advertised by client
1543-
SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl),
1544-
SelectALPNCallback,
1545-
nullptr);
1528+
w->alpn_protos_ = std::vector<unsigned char>(
1529+
protos.data(), protos.data() + protos.length());
1530+
SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(ssl), SelectALPNCallback, w);
15461531
}
15471532
}
15481533

Collapse file

‎src/crypto/crypto_tls.h‎

Copy file name to clipboardExpand all lines: src/crypto/crypto_tls.h
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <openssl/ssl.h>
3535

3636
#include <string>
37+
#include <vector>
3738

3839
namespace node {
3940
namespace crypto {
@@ -283,6 +284,9 @@ class TLSWrap : public AsyncWrap,
283284
void* cert_cb_arg_ = nullptr;
284285

285286
BIOPointer bio_trace_;
287+
288+
public:
289+
std::vector<unsigned char> alpn_protos_; // Accessed by SelectALPNCallback.
286290
};
287291

288292
} // namespace crypto
Collapse file

‎src/env_properties.h‎

Copy file name to clipboardExpand all lines: src/env_properties.h
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
// for the sake of convenience. Strings should be ASCII-only and have a
1919
// "node:" prefix to avoid name clashes with third-party code.
2020
#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \
21-
V(alpn_buffer_private_symbol, "node:alpnBuffer") \
2221
V(arrow_message_private_symbol, "node:arrowMessage") \
2322
V(contextify_context_private_symbol, "node:contextify:context") \
2423
V(contextify_global_private_symbol, "node:contextify:global") \
Collapse file

‎typings/internalBinding/util.d.ts‎

Copy file name to clipboardExpand all lines: typings/internalBinding/util.d.ts
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ declare namespace InternalUtilBinding {
99

1010
declare function InternalBinding(binding: 'util'): {
1111
// PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES, defined in src/env.h
12-
alpn_buffer_private_symbol: 0;
1312
arrow_message_private_symbol: 1;
1413
contextify_context_private_symbol: 2;
1514
contextify_global_private_symbol: 3;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.