[squid-dev] [PATCH] ServerBump class cleanup

[Amos Jeffries]

This patch is a general cleanup of coding styles and current code
requirements for the ServerBump class.

Is there any other general cleanup that should be done to this class
while I'm at it ?

Amos
-------------- next part --------------
=== modified file 'src/ssl/PeekingPeerConnector.cc'
--- src/ssl/PeekingPeerConnector.cc	2016-11-19 13:25:15 +0000
+++ src/ssl/PeekingPeerConnector.cc	2016-11-30 13:29:17 +0000
@@ -184,41 +184,41 @@
                 }
             }
         } else {
             // Set client SSL options
             SSL_set_options(serverSession.get(), ::Security::ProxyOutgoingConfig.parsedOptions);
 
             // Use SNI TLS extension only when we connect directly
             // to the origin server and we know the server host name.
             const char *sniServer = NULL;
             const bool redirected = request->flags.redirected && ::Config.onoff.redir_rewrites_host;
             if (!hostName || redirected)
                 sniServer = !request->url.hostIsNumeric() ? request->url.host() : NULL;
             else
                 sniServer = hostName->c_str();
 
             if (sniServer)
                 Ssl::setClientSNI(serverSession.get(), sniServer);
         }
 
         if (Ssl::ServerBump *serverBump = csd->serverBump()) {
-            serverBump->attachServerSSL(serverSession.get());
+            serverBump->attachServerSession(serverSession);
             // store peeked cert to check SQUID_X509_V_ERR_CERT_CHANGE
             if (X509 *peeked_cert = serverBump->serverCert.get()) {
                 X509_up_ref(peeked_cert);
                 SSL_set_ex_data(serverSession.get(), ssl_ex_index_ssl_peeked_cert, peeked_cert);
             }
         }
     }
 
     return true;
 }
 
 void
 Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error)
 {
     // Check the list error with
     if (!request->clientConnectionManager.valid() || !fd_table[serverConnection()->fd].ssl)
         return;
 
     // remember the server certificate from the ErrorDetail object
     if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {

=== modified file 'src/ssl/ServerBump.cc'
--- src/ssl/ServerBump.cc	2016-10-06 22:05:50 +0000
+++ src/ssl/ServerBump.cc	2016-11-30 13:29:17 +0000
@@ -27,47 +27,47 @@
     debugs(33, 4, "will peek at " << request->url.authority(true));
     act.step1 = md;
     act.step2 = act.step3 = Ssl::bumpNone;
 
     if (e) {
         entry = e;
         entry->lock("Ssl::ServerBump");
     } else {
         // XXX: Performance regression. c_str() reallocates
         SBuf uriBuf(request->effectiveRequestUri());
         const char *uri = uriBuf.c_str();
         entry = storeCreateEntry(uri, uri, request->flags, request->method);
     }
     // We do not need to be a client because the error contents will be used
     // later, but an entry without any client will trim all its contents away.
     sc = storeClientListAdd(entry, this);
 }
 
 Ssl::ServerBump::~ServerBump()
 {
-    debugs(33, 4, HERE << "destroying");
+    debugs(33, 4, "destroying");
     if (entry) {
-        debugs(33, 4, HERE << *entry);
+        debugs(33, 4, *entry);
         storeUnregister(sc, entry, this);
         entry->unlock("Ssl::ServerBump");
     }
 }
 
 void
-Ssl::ServerBump::attachServerSSL(SSL *ssl)
+Ssl::ServerBump::attachServerSession(const Security::SessionPointer &s)
 {
-    if (serverSSL.get())
+    if (serverSession)
         return;
 
-    serverSSL.resetAndLock(ssl);
+    serverSession = s;
 }
 
 const Security::CertErrors *
 Ssl::ServerBump::sslErrors() const
 {
-    if (!serverSSL.get())
-        return NULL;
+    if (!serverSession)
+        return nullptr;
 
-    const Security::CertErrors *errs = static_cast<const Security::CertErrors*>(SSL_get_ex_data(serverSSL.get(), ssl_ex_index_ssl_errors));
+    const Security::CertErrors *errs = static_cast<const Security::CertErrors*>(SSL_get_ex_data(serverSession.get(), ssl_ex_index_ssl_errors));
     return errs;
 }
 

=== modified file 'src/ssl/ServerBump.h'
--- src/ssl/ServerBump.h	2016-09-13 11:38:07 +0000
+++ src/ssl/ServerBump.h	2016-11-30 13:31:28 +0000
@@ -1,60 +1,60 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
-#ifndef _SQUID_SSL_PEEKER_H
-#define _SQUID_SSL_PEEKER_H
+#ifndef _SQUID_SRC_SSL_SERVERBUMP_H
+#define _SQUID_SRC_SSL_SERVERBUMP_H
 
 #include "base/AsyncJob.h"
 #include "base/CbcPointer.h"
 #include "comm/forward.h"
 #include "HttpRequest.h"
 #include "ip/Address.h"
 #include "security/forward.h"
 
 class ConnStateData;
 class store_client;
 
 namespace Ssl
 {
 
 /**
  * Maintains bump-server-first related information.
  */
 class ServerBump
 {
     CBDATA_CLASS(ServerBump);
 
 public:
-    explicit ServerBump(HttpRequest *fakeRequest, StoreEntry *e = NULL, Ssl::BumpMode mode = Ssl::bumpServerFirst);
+    explicit ServerBump(HttpRequest *fakeRequest, StoreEntry *e = nullptr, Ssl::BumpMode mode = Ssl::bumpServerFirst);
     ~ServerBump();
-    void attachServerSSL(SSL *); ///< Sets the server SSL object
+    void attachServerSession(const Security::SessionPointer &); ///< Sets the server SSL object
     const Security::CertErrors *sslErrors() const; ///< SSL [certificate validation] errors
 
     /// faked, minimal request; required by Client API
     HttpRequest::Pointer request;
     StoreEntry *entry; ///< for receiving Squid-generated error messages
     /// HTTPS server certificate. Maybe it is different than the one
-    /// it is stored in serverSSL object (error SQUID_X509_V_ERR_CERT_CHANGE)
+    /// stored in serverSession object (error SQUID_X509_V_ERR_CERT_CHANGE)
     Security::CertPointer serverCert;
     struct {
         Ssl::BumpMode step1; ///< The SSL bump mode at step1
         Ssl::BumpMode step2; ///< The SSL bump mode at step2
         Ssl::BumpMode step3; ///< The SSL bump mode at step3
     } act; ///< bumping actions at various bumping steps
     Ssl::BumpStep step; ///< The SSL bumping step
-    SBuf clientSni; ///< the SSL client SNI name
-    Security::SessionPointer serverSSL; ///< The SSL object on server side.
+    SBuf clientSni; ///< the client provided SNI name
 
 private:
+    Security::SessionPointer serverSession; ///< The session object on server side.
     store_client *sc; ///< dummy client to prevent entry trimming
 };
 
 } // namespace Ssl
 
 #endif