From 98297b92f566d2bf970e987c5efaf9a7707a6302 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 19 Nov 2019 22:15:17 +0100
Subject: [PATCH] src: inline SetSNICallback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Refs: https://github.com/nodejs/node/pull/30548#discussion_r348168855

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 | 17 +----------------
 src/node_crypto.h  |  1 -
 src/tls_wrap.cc    |  6 +++++-
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 61be6afb1ce068..921b5a25a1024b 100644
--- 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
index 1c479353ad4c28..bc91198b015264 100644
--- 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
index 46aee6f16614d5..2f8da61f647f44 100644
--- 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;
 }