From: Eduard Bagdasaryan Date: Thu, 8 Dec 2016 20:38:12 +0000 (+1300) Subject: Cleanup: Refactor custom ACL-controlled actions configuration code. X-Git-Tag: M-staged-PR71~345 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ba6fffba7bbad6c0efcb9613b16aa97573ee7ef0;p=thirdparty%2Fsquid.git 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 fad53c62ba..e30f945194 100644 --- 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 d507e0e7be..8becc44b28 100644 --- a/src/acl/Tree.h +++ b/src/acl/Tree.h @@ -25,9 +25,9 @@ class Tree: public OrNode public: /// dumps 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 + 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 +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 2e277e5ac4..95cb33e14e 100644 --- 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 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 e39fa25d38..b0d17f88c5 100644 --- 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 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 163d239581..b2e267ee57 100644 --- 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::vectorBumpModeStr; /** \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 15c410243a..60adc6d906 100644 --- 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 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)