Cleanup: Refactor custom ACL-controlled actions configuration code.

* Reduced parsing code duplication by adding ParseAclWithAction().

* Used functors for generic action-to-string conversions. It is possible
  now to perform such conversions providing lambda expressions.

* Used vectors of strings instead of C-style arrays for storing
  conversion tables and check against bounds with vector::at().

diff --git a/src/acl/Tree.cc b/src/acl/Tree.cc
index fad53c62ba850c3a40cb924d82e51eceb8b95879..e30f945194731013fa07daf680107f31efa75d70 100644 (file)
--- a/src/acl/Tree.cc
+++ b/src/acl/Tree.cc
@@ -57,35 +57,6 @@ Acl::Tree::add(ACL *rule)
     InnerNode::add(rule);
 }
 
-SBufList
-Acl::Tree::treeDump(const char *prefix, const ActionToString &convert) const
-{
-    SBufList text;
-    Actions::const_iterator action = actions.begin();
-    typedef Nodes::const_iterator NCI;
-    for (NCI node = nodes.begin(); node != nodes.end(); ++node) {
-
-        text.push_back(SBuf(prefix));
-
-        if (action != actions.end()) {
-            const char *act = convert ? convert[action->kind] :
-                              (*action == ACCESS_ALLOWED ? "allow" : "deny");
-            text.push_back(act?SBuf(act):SBuf("???"));
-            ++action;
-        }
-
-#if __cplusplus >= 201103L
-        text.splice(text.end(), (*node)->dump());
-#else
-        // temp is needed until c++11 move constructor
-        SBufList temp = (*node)->dump();
-        text.splice(text.end(), temp);
-#endif
-        text.push_back(SBuf("\n"));
-    }
-    return text;
-}
-
 bool
 Acl::Tree::bannedAction(ACLChecklist *checklist, Nodes::const_iterator node) const
 {

diff --git a/src/acl/Tree.h b/src/acl/Tree.h
index d507e0e7beb21cd629fc5eb4f9ee67ce478ca9c8..8becc44b28654966784757b2721f100a252180c0 100644 (file)
--- a/src/acl/Tree.h
+++ b/src/acl/Tree.h
@@ -25,9 +25,9 @@ class Tree: public OrNode
 
 public:
     /// dumps <name, action, rule, new line> tuples
-    /// action.kind is mapped to a string using the supplied conversion table
-    typedef const char **ActionToString;
-    SBufList treeDump(const char *name, const ActionToString &convert) const;
+    /// the supplied converter maps action.kind to a string
+    template <class ActionToStringConverter>
+    SBufList treeDump(const char *name, ActionToStringConverter converter) const;
 
     /// Returns the corresponding action after a successful tree match.
     allow_t winningAction() const;
@@ -49,6 +49,42 @@ protected:
     Actions actions;
 };
 
+inline const char *
+AllowOrDeny(const allow_t &action)
+{
+    return action == ACCESS_ALLOWED ? "allow" : "deny";
+}
+
+template <class ActionToStringConverter>
+inline SBufList
+Tree::treeDump(const char *prefix, ActionToStringConverter converter) const
+{
+    SBufList text;
+    Actions::const_iterator action = actions.begin();
+    typedef Nodes::const_iterator NCI;
+    for (NCI node = nodes.begin(); node != nodes.end(); ++node) {
+
+        text.push_back(SBuf(prefix));
+
+        if (action != actions.end()) {
+            static const SBuf DefaultActString("???");
+            const char *act = converter(*action);
+            text.push_back(act ? SBuf(act) : DefaultActString);
+            ++action;
+        }
+
+#if __cplusplus >= 201103L
+        text.splice(text.end(), (*node)->dump());
+#else
+        // temp is needed until c++11 move constructor
+        SBufList temp = (*node)->dump();
+        text.splice(text.end(), temp);
+#endif
+        text.push_back(SBuf("\n"));
+    }
+    return text;
+}
+
 } // namespace Acl
 
 #endif /* SQUID_ACL_TREE_H */

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 2e277e5ac424812e8b21a1bd5e351b57c91c651c..95cb33e14ee5ecab2d3812d473cd9d3d742820d7 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -1340,7 +1340,7 @@ void
 dump_acl_access(StoreEntry * entry, const char *name, acl_access * head)
 {
     if (head)
-        dump_SBufList(entry, head->treeDump(name,NULL));
+        dump_SBufList(entry, head->treeDump(name, &Acl::AllowOrDeny));
 }
 
 static void
@@ -1826,6 +1826,23 @@ dump_authparam(StoreEntry * entry, const char *name, Auth::ConfigVector cfg)
 }
 #endif /* USE_AUTH */
 
+static void
+ParseAclWithAction(acl_access **access, const allow_t &action, const char *desc, ACL *acl = nullptr)
+{
+    assert(access);
+    SBuf name;
+    if (!*access) {
+        *access = new Acl::Tree;
+        name.Printf("(%s rules)", desc);
+        (*access)->context(name.c_str(), config_input_line);
+    }
+    Acl::AndNode *rule = new Acl::AndNode;
+    name.Printf("(%s rule)", desc);
+    rule->context(name.c_str(), config_input_line);
+    acl ? rule->add(acl) : rule->lineParse();
+    (*access)->add(rule, action);
+}
+
 /* TODO: just return the object, the # is irrelevant */
 static int
 find_fstype(char *type)
@@ -4626,24 +4643,16 @@ static void parse_sslproxy_ssl_bump(acl_access **ssl_bump)
 
     bumpCfgStyleLast = bumpCfgStyleNow;
 
-    Acl::AndNode *rule = new Acl::AndNode;
-    rule->context("(ssl_bump rule)", config_input_line);
-    rule->lineParse();
     // empty rule OK
-
-    assert(ssl_bump);
-    if (!*ssl_bump) {
-        *ssl_bump = new Acl::Tree;
-        (*ssl_bump)->context("(ssl_bump rules)", config_input_line);
-    }
-
-    (*ssl_bump)->add(rule, action);
+    ParseAclWithAction(ssl_bump, action, "ssl_bump");
 }
 
 static void dump_sslproxy_ssl_bump(StoreEntry *entry, const char *name, acl_access *ssl_bump)
 {
     if (ssl_bump)
-        dump_SBufList(entry, ssl_bump->treeDump(name, Ssl::BumpModeStr));
+        dump_SBufList(entry, ssl_bump->treeDump(name, [](const allow_t &action) {
+                    return Ssl::BumpModeStr.at(action.kind);
+        }));
 }
 
 static void free_sslproxy_ssl_bump(acl_access **ssl_bump)
@@ -4770,21 +4779,16 @@ static void parse_ftp_epsv(acl_access **ftp_epsv)
     if (ftpEpsvIsDeprecatedRule) {
         // overwrite previous ftp_epsv lines
         delete *ftp_epsv;
+        *ftp_epsv = nullptr;
+
         if (ftpEpsvDeprecatedAction == allow_t(ACCESS_DENIED)) {
-            Acl::AndNode *ftpEpsvRule = new Acl::AndNode;
-            ftpEpsvRule->context("(ftp_epsv rule)", config_input_line);
-            ACL *a = ACL::FindByName("all");
-            if (!a) {
-                delete ftpEpsvRule;
+            if (ACL *a = ACL::FindByName("all"))
+                ParseAclWithAction(ftp_epsv, ftpEpsvDeprecatedAction, "ftp_epsv", a);
+            else {
                 self_destruct();
                 return;
             }
-            ftpEpsvRule->add(a);
-            *ftp_epsv = new Acl::Tree;
-            (*ftp_epsv)->context("(ftp_epsv rules)", config_input_line);
-            (*ftp_epsv)->add(ftpEpsvRule, ftpEpsvDeprecatedAction);
-        } else
-            *ftp_epsv = NULL;
+        }
         FtpEspvDeprecated = true;
     } else {
         aclParseAccessLine(cfg_directive, LegacyParser, ftp_epsv);
@@ -4794,7 +4798,7 @@ static void parse_ftp_epsv(acl_access **ftp_epsv)
 static void dump_ftp_epsv(StoreEntry *entry, const char *name, acl_access *ftp_epsv)
 {
     if (ftp_epsv)
-        dump_SBufList(entry, ftp_epsv->treeDump(name, NULL));
+        dump_SBufList(entry, ftp_epsv->treeDump(name, Acl::AllowOrDeny));
 }
 
 static void free_ftp_epsv(acl_access **ftp_epsv)
@@ -4919,31 +4923,22 @@ parse_on_unsupported_protocol(acl_access **access)
         return;
     }
 
-    Acl::AndNode *rule = new Acl::AndNode;
-    rule->context("(on_unsupported_protocol rule)", config_input_line);
-    rule->lineParse();
     // empty rule OK
-
-    assert(access);
-    if (!*access) {
-        *access = new Acl::Tree;
-        (*access)->context("(on_unsupported_protocol rules)", config_input_line);
-    }
-
-    (*access)->add(rule, action);
+    ParseAclWithAction(access, action, "on_unsupported_protocol");
 }
 
 static void
 dump_on_unsupported_protocol(StoreEntry *entry, const char *name, acl_access *access)
 {
-    const char *on_error_tunnel_mode_str[] = {
+    static const std::vector<const char *> onErrorTunnelMode = {
         "none",
         "tunnel",
-        "respond",
-        NULL
+        "respond"
     };
     if (access) {
-        SBufList lines = access->treeDump(name, on_error_tunnel_mode_str);
+        SBufList lines = access->treeDump(name, [](const allow_t &action) {
+                return onErrorTunnelMode.at(action.kind);
+        });
         dump_SBufList(entry, lines);
     }
 }

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index e39fa25d3850abbfae90794f2ccbd26ec6fd2d3c..b0d17f88c58bc114a33ed57da844cb24c7bd091e 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -44,7 +44,7 @@ static Ssl::CertsIndexedList SquidUntrustedCerts;
 
 const EVP_MD *Ssl::DefaultSignHash = NULL;
 
-const char *Ssl::BumpModeStr[] = {
+std::vector<const char *> Ssl::BumpModeStr = {
     "none",
     "client-first",
     "server-first",
@@ -52,9 +52,8 @@ const char *Ssl::BumpModeStr[] = {
     "stare",
     "bump",
     "splice",
-    "terminate",
-    /*"err",*/
-    NULL
+    "terminate"
+    /*,"err"*/
 };
 
 /**

diff --git a/src/ssl/support.h b/src/ssl/support.h
index 163d23958117d42fd5accee348a11f5da43ea253..b2e267ee575bb0da1f6cd42bb195377f928c9888 100644 (file)
--- a/src/ssl/support.h
+++ b/src/ssl/support.h
@@ -151,7 +151,7 @@ enum BumpStep {bumpStep1, bumpStep2, bumpStep3};
  \ingroup  ServerProtocolSSLAPI
  * Short names for ssl-bump modes
  */
-extern const char *BumpModeStr[];
+extern std::vector<const char *>BumpModeStr;
 
 /**
  \ingroup ServerProtocolSSLAPI
@@ -159,7 +159,7 @@ extern const char *BumpModeStr[];
  */
 inline const char *bumpMode(int bm)
 {
-    return (0 <= bm && bm < Ssl::bumpEnd) ? Ssl::BumpModeStr[bm] : NULL;
+    return (0 <= bm && bm < Ssl::bumpEnd) ? Ssl::BumpModeStr.at(bm) : NULL;
 }
 
 /// certificates indexed by issuer name

diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc
index 15c410243a50114ce928bcecf1575e7487a86049..60adc6d906336e9eebdf300157ae414ac5c711ea 100644 (file)
--- a/src/tests/stub_libsslsquid.cc
+++ b/src/tests/stub_libsslsquid.cc
@@ -66,7 +66,7 @@ namespace Ssl
 //GETX509ATTRIBUTE GetX509UserAttribute;
 //GETX509ATTRIBUTE GetX509CAAttribute;
 //GETX509ATTRIBUTE GetX509Fingerprint;
-const char *BumpModeStr[] = {""};
+std::vector<const char *> BumpModeStr = {""};
 bool generateUntrustedCert(Security::CertPointer & untrustedCert, EVP_PKEY_Pointer & untrustedPkey, Security::CertPointer const & cert, EVP_PKEY_Pointer const & pkey) STUB_RETVAL(false)
 Security::ContextPointer generateSslContext(CertificateProperties const &, AnyP::PortCfg &) STUB_RETVAL(Security::ContextPointer())
 bool verifySslCertificate(Security::ContextPointer &, CertificateProperties const &) STUB_RETVAL(false)