From 1e1b9a15218adf578c20a61b1b92d884914eee78 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 23 Nov 2012 19:12:26 -0700 Subject: [PATCH] libacl: polish and code fixes * removed several blocks of dead code and useless conditional checks * Improved handling of garbage config definitions for ACL random, max_conn * Initialized several uninitialized class members in ACL base class. Detected by Coverity Scan. Issues 740528, 740343, 740529, 740362, 434117, 740593, 740407. Possibly also resolves issue 740486. --- src/acl/Acl.cc | 7 ++++++- src/acl/Asn.cc | 2 -- src/acl/Checklist.cc | 3 ++- src/acl/DestinationIp.cc | 3 +-- src/acl/Ip.cc | 9 --------- src/acl/MaxConnection.cc | 9 ++++++++- src/acl/Random.cc | 21 +++++++++++++++------ src/acl/Random.h | 1 + 8 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/acl/Acl.cc b/src/acl/Acl.cc index 11245fc0ec..5043159ef8 100644 --- a/src/acl/Acl.cc +++ b/src/acl/Acl.cc @@ -80,7 +80,12 @@ ACL::Factory (char const *type) return result; } -ACL::ACL () :cfgline(NULL) {} +ACL::ACL() : + cfgline(NULL), + next(NULL) +{ + *name = 0; +} bool ACL::valid () const { diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc index f53420b440..daea68e5cf 100644 --- a/src/acl/Asn.cc +++ b/src/acl/Asn.cc @@ -509,8 +509,6 @@ destroyRadixNodeInfo(as_info * e_info) data = data->next; delete prev; } - - delete data; } static int diff --git a/src/acl/Checklist.cc b/src/acl/Checklist.cc index ff413eb082..9b98028c16 100644 --- a/src/acl/Checklist.cc +++ b/src/acl/Checklist.cc @@ -320,7 +320,8 @@ ACLChecklist::ACLChecklist() : async_(false), finished_(false), allow_(ACCESS_DENIED), - state_(NullState::Instance()) + state_(NullState::Instance()), + checking_(false) { } diff --git a/src/acl/DestinationIp.cc b/src/acl/DestinationIp.cc index ab2967b112..721fc0dd32 100644 --- a/src/acl/DestinationIp.cc +++ b/src/acl/DestinationIp.cc @@ -54,8 +54,7 @@ ACLDestinationIP::match(ACLChecklist *cl) // Bypass of browser same-origin access control in intercepted communication // To resolve this we will force DIRECT and only to the original client destination. // In which case, we also need this ACL to accurately match the destination - if (Config.onoff.client_dst_passthru && checklist->request && - (checklist->request->flags.intercepted || checklist->request->flags.spoofClientIp)) { + if (Config.onoff.client_dst_passthru && (checklist->request->flags.intercepted || checklist->request->flags.spoofClientIp)) { assert(checklist->conn() && checklist->conn()->clientConnection != NULL); return ACLIP::match(checklist->conn()->clientConnection->local); } diff --git a/src/acl/Ip.cc b/src/acl/Ip.cc index ad3f8a2622..578fca9628 100644 --- a/src/acl/Ip.cc +++ b/src/acl/Ip.cc @@ -409,15 +409,6 @@ acl_ip_data::FactoryParse(const char *t) memset(&hints, 0, sizeof(struct addrinfo)); - if ( iptype != AF_UNSPEC ) { - hints.ai_flags |= AI_NUMERICHOST; - } - -#if 0 - if (Ip::EnableIpv6&IPV6_SPECIAL_V4MAPPING) - hints.ai_flags |= AI_V4MAPPED | AI_ALL; -#endif - int errcode = getaddrinfo(addr1,NULL,&hints,&hp); if (hp == NULL) { debugs(28, DBG_CRITICAL, "aclIpParseIpData: Bad host/IP: '" << addr1 << diff --git a/src/acl/MaxConnection.cc b/src/acl/MaxConnection.cc index 49d97187ac..30c4eb5049 100644 --- a/src/acl/MaxConnection.cc +++ b/src/acl/MaxConnection.cc @@ -85,8 +85,15 @@ ACLMaxConnection::parse() limit = (atoi (t)); /* suck out file contents */ - + // ignore comments + bool ignore = false; while ((t = strtokFile())) { + ignore |= (*t != '#'); + + if (ignore) + continue; + + debugs(89, DBG_CRITICAL, "WARNING: max_conn only accepts a single limit value."); limit = 0; } } diff --git a/src/acl/Random.cc b/src/acl/Random.cc index ca2c25ec99..35353811d7 100644 --- a/src/acl/Random.cc +++ b/src/acl/Random.cc @@ -72,6 +72,12 @@ ACLRandom::empty () const return data == 0.0; } +bool +ACLRandom::valid() const +{ + return !empty(); +} + /*******************/ /* aclParseRandomList */ /*******************/ @@ -82,6 +88,9 @@ ACLRandom::parse() char bufa[256], bufb[256]; t = strtokFile(); + if (!t) + return; + debugs(28, 5, "aclParseRandomData: " << t); // seed random generator ... @@ -91,23 +100,23 @@ ACLRandom::parse() int a = xatoi(bufa); int b = xatoi(bufb); if (a == 0 || b == 0) { - debugs(28, DBG_CRITICAL, "aclParseRandomData: Bad Pattern: '" << t << "'"); - self_destruct(); + debugs(28, DBG_CRITICAL, "ERROR: ACL random with bad pattern: '" << t << "'"); + return; } else data = a / (double)(a+b); } else if (sscanf(t, "%[0-9]/%[0-9]", bufa, bufb) == 2) { int a = xatoi(bufa); int b = xatoi(bufb); if (a == 0 || b == 0) { - debugs(28, DBG_CRITICAL, "aclParseRandomData: Bad Pattern: '" << t << "'"); - self_destruct(); + debugs(28, DBG_CRITICAL, "ERROR: ACL random with bad pattern: '" << t << "'"); + return; } else data = (double) a / (double) b; } else if (sscanf(t, "0.%[0-9]", bufa) == 1) { data = atof(t); } else { - debugs(28, DBG_CRITICAL, "aclParseRandomData: Bad Pattern: '" << t << "'"); - self_destruct(); + debugs(28, DBG_CRITICAL, "ERROR: ACL random with bad pattern: '" << t << "'"); + return; } // save the exact input pattern. so we can display it later. diff --git a/src/acl/Random.h b/src/acl/Random.h index 7ebf3e9d4b..55d5fe10d1 100644 --- a/src/acl/Random.h +++ b/src/acl/Random.h @@ -53,6 +53,7 @@ public: virtual int match(ACLChecklist *checklist); virtual wordlist *dump() const; virtual bool empty () const; + virtual bool valid() const; protected: static Prototype RegistryProtoype; -- 2.47.2