From 7bd587ef0cd574cd6e6e44888217b8d817d0d912 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Nov 2019 20:07:00 +0100 Subject: [PATCH] src: use BaseObjectPtr to store SNI context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than relying on a link to the JS object, store a pointer to the C++ object directly. PR-URL: https://github.com/nodejs/node/pull/30548 Reviewed-By: Ben Noordhuis Reviewed-By: David Carlier Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- src/node_crypto.cc | 12 +++++++++--- src/node_crypto.h | 2 +- src/tls_wrap.cc | 3 +-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 8f80db52297e4a..91704732d18992 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2991,9 +2991,15 @@ void SSLWrap::CertCbDone(const FunctionCallbackInfo& args) { goto fire_cb; if (cons->HasInstance(ctx)) { - SecureContext* sc; - ASSIGN_OR_RETURN_UNWRAP(&sc, ctx.As()); - w->sni_context_.Reset(env->isolate(), ctx); + SecureContext* sc = Unwrap(ctx.As()); + CHECK_NOT_NULL(sc); + // XXX: There is a method w->SetSNIContext(sc), and you might think that + // it makes sense to call that here and make setting w->sni_context_ part + // of it. In fact, that passes the test suite, although SetSNIContext() + // performs a lot more operations. + // If anybody is familiar enough with the TLS code to know whether it makes + // sense, please do so or document why it doesn't. + w->sni_context_ = BaseObjectPtr(sc); int rv; diff --git a/src/node_crypto.h b/src/node_crypto.h index d2bdd40ed283ed..96292b42780f19 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -310,7 +310,7 @@ class SSLWrap { ClientHelloParser hello_parser_; v8::Global ocsp_response_; - v8::Global sni_context_; + BaseObjectPtr sni_context_; friend class SecureContext; }; diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 626662c9a5ef8e..bacb1a0f27ee79 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -1065,10 +1065,9 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) { return SSL_TLSEXT_ERR_NOACK; } - p->sni_context_.Reset(env->isolate(), ctx); - SecureContext* sc = Unwrap(ctx.As()); CHECK_NOT_NULL(sc); + p->sni_context_ = BaseObjectPtr(sc); p->SetSNIContext(sc); return SSL_TLSEXT_ERR_OK; }