src: inline SetSNICallback

Refs: nodejs#30548 (comment) PR-URL: nodejs#30548 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
targos · Apr 25, 2020 · ef3654a · ef3654a
1 parent ba789d1
commit ef3654a
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 18 deletions.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -119,7 +119,6 @@ static bool extra_root_certs_loaded = false;
 template void SSLWrap<TLSWrap>::AddMethods(Environment* env,
                                            Local<FunctionTemplate> t);
 template void SSLWrap<TLSWrap>::ConfigureSecureContext(SecureContext* sc);
-template void SSLWrap<TLSWrap>::SetSNIContext(SecureContext* sc);
 template int SSLWrap<TLSWrap>::SetCACerts(SecureContext* sc);
 template void SSLWrap<TLSWrap>::MemoryInfo(MemoryTracker* tracker) const;
 template SSL_SESSION* SSLWrap<TLSWrap>::GetSessionCallback(
@@ -2425,12 +2424,7 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
   if (cons->HasInstance(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.
+    // Store the SNI context for later use.
     w->sni_context_ = BaseObjectPtr<SecureContext>(sc);
 
     if (UseSNIContext(w->ssl_, sc) && !w->SetCACerts(sc)) {
@@ -2471,15 +2465,6 @@ void SSLWrap<Base>::DestroySSL() {
 }
 
 
-template <class Base>
-void SSLWrap<Base>::SetSNIContext(SecureContext* sc) {
-  ConfigureSecureContext(sc);
-  CHECK_EQ(SSL_set_SSL_CTX(ssl_.get(), sc->ctx_.get()), sc->ctx_.get());
-
-  SetCACerts(sc);
-}
-
-
 template <class Base>
 int SSLWrap<Base>::SetCACerts(SecureContext* sc) {
   int err = SSL_set1_verify_cert_store(ssl_.get(),

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -281,7 +281,6 @@ class SSLWrap {
 
   void DestroySSL();
   void WaitForCertCb(CertCb cb, void* arg);
-  void SetSNIContext(SecureContext* sc);
   int SetCACerts(SecureContext* sc);
 
   inline Environment* ssl_env() const {

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
@@ -1094,7 +1094,11 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
   SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
   CHECK_NOT_NULL(sc);
   p->sni_context_ = BaseObjectPtr<SecureContext>(sc);
-  p->SetSNIContext(sc);
+
+  p->ConfigureSecureContext(sc);
+  CHECK_EQ(SSL_set_SSL_CTX(p->ssl_.get(), sc->ctx_.get()), sc->ctx_.get());
+  p->SetCACerts(sc);
+
   return SSL_TLSEXT_ERR_OK;
 }