From ce8d8c06accac210e4c3bf31d8adb600eae2307d Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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 <info@bnoordhuis.nl>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 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 3e956098e48666..61be6afb1ce068 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2423,9 +2423,15 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
     goto fire_cb;
 
   if (cons->HasInstance(ctx)) {
-    SecureContext* sc;
-    ASSIGN_OR_RETURN_UNWRAP(&sc, ctx.As<Object>());
-    w->sni_context_.Reset(env->isolate(), ctx);
+    SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
+    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<SecureContext>(sc);
 
     if (UseSNIContext(w->ssl_, sc) && !w->SetCACerts(sc)) {
       // Not clear why sometimes we throw error, and sometimes we call
diff --git a/src/node_crypto.h b/src/node_crypto.h
index dd043813fa22cd..1c479353ad4c28 100644
--- a/src/node_crypto.h
+++ b/src/node_crypto.h
@@ -303,7 +303,7 @@ class SSLWrap {
   ClientHelloParser hello_parser_;
 
   v8::Global<v8::ArrayBufferView> ocsp_response_;
-  v8::Global<v8::Value> sni_context_;
+  BaseObjectPtr<SecureContext> sni_context_;
 
   friend class SecureContext;
 };
diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 47563366dbae51..46aee6f16614d5 100644
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -1091,10 +1091,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<SecureContext>(ctx.As<Object>());
   CHECK_NOT_NULL(sc);
+  p->sni_context_ = BaseObjectPtr<SecureContext>(sc);
   p->SetSNIContext(sc);
   return SSL_TLSEXT_ERR_OK;
 }