MXS-1346: Clean up unused code

Removed the rule type enum and replaced it with a string description of
the type. Moved the rule type and name strings as private to the Rule
class. Replaced the need_full_parsing of the base class with a simple
constant.

Removed the unused array of rule names as well and the STRLINK structure
and the functions that use it.

server/modules/filter/dbfwfilter/dbfwfilter.cc

@@ -102,20 +102,6 @@ static void diagnostic(MXS_FILTER *instance, MXS_FILTER_SESSION *fsession, DCB *
 static json_t* diagnostic_json(const MXS_FILTER *instance, const MXS_FILTER_SESSION *fsession);
 static uint64_t getCapabilities(MXS_FILTER* instance);
 
-static const char* rule_names[] =
-{
-    "UNDEFINED",
-    "COLUMN",
-    "FUNCTION",
-    "THROTTLE",
-    "PERMISSION",
-    "WILDCARD",
-    "REGEX",
-    "CLAUSE"
-};
-
-const int rule_names_len = sizeof(rule_names) / sizeof(char**);
-
 /** The rules and users for each thread */
 thread_local struct
 {
@@ -132,32 +118,16 @@ bool replace_rules(FW_INSTANCE* instance);
 
 static void print_rule(Rule *rules, char *dest)
 {
-    int type = 0;
+    sprintf(dest, "%s, %s, %d", rules->name().c_str(),
-
+            rules->type().c_str(), rules->times_matched);
-    if ((int)rules->type > 0 && (int)rules->type < rule_names_len)
-    {
-        type = (int)rules->type;
-    }
-
-    sprintf(dest, "%s, %s, %d",
-            rules->name.c_str(),
-            rule_names[type],
-            rules->times_matched);
 }
 
 static json_t* rule_to_json(const SRule& rule)
 {
-    int type = 0;
-
-    if ((int)rule->type > 0 && (int)rule->type < rule_names_len)
-    {
-        type = (int)rule->type;
-    }
-
     json_t* rval = json_object();
 
-    json_object_set_new(rval, "name", json_string(rule->name.c_str()));
+    json_object_set_new(rval, "name", json_string(rule->name().c_str()));
-    json_object_set_new(rval, "type", json_string(rule_names[type]));
+    json_object_set_new(rval, "type", json_string(rule->type().c_str()));
     json_object_set_new(rval, "times_matched", json_integer(rule->times_matched));
 
     return rval;
@@ -176,86 +146,6 @@ static json_t* rules_to_json(const RuleList& rules)
     return rval;
 }
 
-/**
- * Push a string onto a string stack
- * @param head Head of the stack
- * @param value value to add
- * @return New top of the stack or NULL if memory allocation fails
- */
-static STRLINK* strlink_push(STRLINK* head, const char* value)
-{
-    STRLINK* link = (STRLINK*)MXS_MALLOC(sizeof(STRLINK));
-
-    if (link && (link->value = MXS_STRDUP(value)))
-    {
-        link->next = head;
-    }
-    else
-    {
-        MXS_FREE(link);
-        link = NULL;
-    }
-    return link;
-}
-
-/**
- * Pop a string off of a string stack
- * @param head Head of the stack
- * @return New head of the stack or NULL if stack is empty
- */
-static STRLINK* strlink_pop(STRLINK* head)
-{
-    if (head)
-    {
-        STRLINK* next = head->next;
-        MXS_FREE(head->value);
-        MXS_FREE(head);
-        return next;
-    }
-    return NULL;
-}
-
-/**
- * Free a string stack
- * @param head Head of the stack
- */
-static void strlink_free(STRLINK* head)
-{
-    while (head)
-    {
-        STRLINK* tmp = head;
-        head = head->next;
-        MXS_FREE(tmp->value);
-        MXS_FREE(tmp);
-    }
-}
-
-/**
- * Clone a string stack. This function reverses the order of the stack.
- * @param head Head of the stack to be cloned
- * @return Clone of the head or NULL if memory allocation failed
- */
-static STRLINK* strlink_reverse_clone(STRLINK* head)
-{
-    STRLINK* clone = NULL;
-    while (head)
-    {
-        STRLINK *tmp = strlink_push(clone, head->value);
-        if (tmp)
-        {
-            clone = tmp;
-        }
-        else
-        {
-            strlink_free(clone);
-            clone = NULL;
-            break;
-        }
-        head = head->next;
-    }
-    return clone;
-}
-
 /**
  * Parses a string that contains an IP address and converts the last octet to '%'.
  * This modifies the string passed as the parameter.
@@ -574,7 +464,7 @@ bool dbfw_show_rules(const MODULECMD_ARG *argv, json_t** output)
     for (RuleList::const_iterator it = this_thread.rules.begin(); it != this_thread.rules.end(); it++)
     {
         const SRule& rule = *it;
-        char buf[rule->name.length() + 200]; // Some extra space
+        char buf[rule->name().length() + 200]; // Some extra space
         print_rule(rule.get(), buf);
         dcb_printf(dcb, "%s\n", buf);
     }
@@ -835,7 +725,7 @@ static SRule find_rule_by_name(const RuleList& rules, std::string name)
     {
         const SRule& rule = *it;
 
-        if (rule->name == name)
+        if (rule->name() == name)
         {
             return rule;
         }
@@ -989,21 +879,6 @@ void set_matching_mode(void* scanner, enum match_type mode)
     rstack->active_mode = mode;
 }
 
-STRLINK* valuelist_to_strlink(ValueList* arr)
-{
-
-    STRLINK* list = NULL;
-
-    for (ValueList::const_iterator it = arr->begin(); it != arr->end(); it++)
-    {
-        list = strlink_push(list, it->c_str());
-    }
-
-    arr->clear();
-
-    return list;
-}
-
 /**
  * Define the topmost rule as a wildcard rule
  * @param scanner Current scanner
@@ -1851,7 +1726,7 @@ diagnostic(MXS_FILTER *instance, MXS_FILTER_SESSION *fsession, DCB *dcb)
     for (RuleList::const_iterator it = this_thread.rules.begin(); it != this_thread.rules.end(); it++)
     {
         const SRule& rule = *it;
-        char buf[rule->name.length() + 200];
+        char buf[rule->name().length() + 200];
         print_rule(rule.get(), buf);
         dcb_printf(dcb, "%s\n", buf);
     }

server/modules/filter/dbfwfilter/dbfwfilter.hh

@@ -27,22 +27,6 @@
 
 #include "dbfwfilter.h"
 
-/**
- * Rule types
- */
-typedef enum
-{
-    RT_UNDEFINED = 0x00, /*< Undefined rule */
-    RT_COLUMN, /*<  Column name rule*/
-    RT_FUNCTION, /*<  Function name rule*/
-    RT_USES_FUNCTION, /*<  Function usage rule*/
-    RT_THROTTLE, /*< Query speed rule */
-    RT_PERMISSION, /*< Simple denying rule */
-    RT_WILDCARD, /*< Wildcard denial rule */
-    RT_REGEX, /*< Regex matching rule */
-    RT_CLAUSE /*< WHERE-clause requirement rule */
-} ruletype_t;
-
 /**
  * What operator a rule should apply to.
  *
@@ -137,15 +121,6 @@ enum fw_actions
 /** Maximum length of the match/nomatch messages */
 #define FW_MAX_SQL_LEN      400
 
-/**
- * Linked list of strings.
- */
-typedef struct strlink_t
-{
-    struct strlink_t *next;     /*< Next node in the list */
-    char*             value;    /*< Value of the current node */
-} STRLINK;
-
 /**
  * A structure defining a range of time
  */

server/modules/filter/dbfwfilter/rules.cc

@@ -26,13 +26,12 @@ static inline bool query_is_sql(GWBUF* query)
     return modutil_is_SQL(query) || modutil_is_SQL_prepare(query);
 }
 
-Rule::Rule(std::string name):
+Rule::Rule(std::string name, std::string type):
-    data(NULL),
-    name(name),
-    type(RT_PERMISSION),
     on_queries(FW_OP_UNDEFINED),
     times_matched(0),
-    active(NULL)
+    active(NULL),
+    m_name(name),
+    m_type(type)
 {
 }
 
@@ -43,37 +42,10 @@ Rule::~Rule()
 bool Rule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
 {
     *msg = create_error("Permission denied at this time.");
-    MXS_NOTICE("rule '%s': query denied at this time.", name.c_str());
+    MXS_NOTICE("rule '%s': query denied at this time.", name().c_str());
     return true;
 }
 
-bool Rule::need_full_parsing(GWBUF* buffer) const
-{
-    bool rval = false;
-
-    if (type == RT_COLUMN ||
-        type == RT_FUNCTION ||
-        type == RT_USES_FUNCTION ||
-        type == RT_WILDCARD ||
-        type == RT_CLAUSE)
-    {
-        switch (qc_get_operation(buffer))
-        {
-        case QUERY_OP_SELECT:
-        case QUERY_OP_UPDATE:
-        case QUERY_OP_INSERT:
-        case QUERY_OP_DELETE:
-            rval = true;
-            break;
-
-        default:
-            break;
-        }
-    }
-
-    return rval;
-}
-
 bool Rule::matches_query_type(GWBUF* buffer)
 {
     qc_query_op_t optype = qc_get_operation(buffer);
@@ -84,6 +56,16 @@ bool Rule::matches_query_type(GWBUF* buffer)
             (on_queries & FW_OP_CHANGE_DB));
 }
 
+const std::string& Rule::name() const
+{
+    return m_name;
+}
+
+const std::string& Rule::type() const
+{
+    return m_type;
+}
+
 bool WildCardRule::matches_query(FW_SESSION* session, GWBUF *queue, char **msg)
 {
     bool rval = false;
@@ -98,7 +80,7 @@ bool WildCardRule::matches_query(FW_SESSION* session, GWBUF *queue, char **msg)
         {
             if (strcmp(infos[i].column, "*") == 0)
             {
-                MXS_NOTICE("rule '%s': query contains a wildcard.", name.c_str());
+                MXS_NOTICE("rule '%s': query contains a wildcard.", name().c_str());
                 rval = true;
                 *msg = create_error("Usage of wildcard denied.");
             }
@@ -117,7 +99,7 @@ bool NoWhereClauseRule::matches_query(FW_SESSION* session, GWBUF* buffer, char**
         rval = true;
         *msg = create_error("Required WHERE/HAVING clause is missing.");
         MXS_NOTICE("rule '%s': query has no where/having "
-                   "clause, query is denied.", name.c_str());
+                   "clause, query is denied.", name().c_str());
     }
 
     return rval;
@@ -139,7 +121,7 @@ bool RegexRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
 
         if (pcre2_match(re, (PCRE2_SPTR)sql, (size_t)len, 0, 0, mdata, NULL) > 0)
         {
-            MXS_NOTICE("rule '%s': regex matched on query", name.c_str());
+            MXS_NOTICE("rule '%s': regex matched on query", name().c_str());
             rval = true;
             *msg = create_error("Permission denied, query matched regular expression.");
         }
@@ -168,7 +150,7 @@ bool ColumnsRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
             if (it != m_values.end())
             {
                 MXS_NOTICE("rule '%s': query targets forbidden column: %s",
-                           name.c_str(), tok.c_str());
+                           name().c_str(), tok.c_str());
                 *msg = create_error("Permission denied to column '%s'.", tok.c_str());
                 rval = true;
                 break;
@@ -204,7 +186,7 @@ bool FunctionRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
                 if (it != m_values.end())
                 {
                     MXS_NOTICE("rule '%s': query uses forbidden function: %s",
-                               name.c_str(), tok.c_str());
+                               name().c_str(), tok.c_str());
                     *msg = create_error("Permission denied to function '%s'.", tok.c_str());
                     rval = true;
                     break;
@@ -235,7 +217,7 @@ bool FunctionUsageRule::matches_query(FW_SESSION* session, GWBUF* buffer, char**
                 if (it != m_values.end())
                 {
                     MXS_NOTICE("rule '%s': query uses a function with forbidden column: %s",
-                               name.c_str(), tok.c_str());
+                               name().c_str(), tok.c_str());
                     *msg = create_error("Permission denied to column '%s' with function.", tok.c_str());
                     return true;
                 }
@@ -266,7 +248,7 @@ bool LimitQueriesRule::matches_query(FW_SESSION* session, GWBUF* buffer, char**
             matches = true;
 
             MXS_INFO("rule '%s': user denied for %f seconds",
-                     name.c_str(), blocked_for);
+                     name().c_str(), blocked_for);
         }
         else
         {
@@ -279,7 +261,7 @@ bool LimitQueriesRule::matches_query(FW_SESSION* session, GWBUF* buffer, char**
         if (queryspeed->count >= queryspeed->limit)
         {
             MXS_INFO("rule '%s': query limit triggered (%d queries in %d seconds), "
-                     "denying queries from user for %d seconds.", name.c_str(),
+                     "denying queries from user for %d seconds.", name().c_str(),
                      queryspeed->limit, queryspeed->period, queryspeed->cooldown);
 
             queryspeed->triggered = time_now;

server/modules/filter/dbfwfilter/rules.hh

@@ -28,18 +28,26 @@ class Rule
     Rule& operator=(const Rule&);
 
 public:
-    Rule(std::string name);
+    Rule(std::string name, std::string type = "PERMISSION");
     virtual ~Rule();
     virtual bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
-    virtual bool need_full_parsing(GWBUF* buffer) const;
-    bool matches_query_type(GWBUF* buffer);
 
-    void*          data;          /*< Actual implementation of the rule */
+    virtual bool need_full_parsing(GWBUF* buffer) const
-    std::string    name;          /*< Name of the rule */
+    {
-    ruletype_t     type;          /*< Type of the rule */
+        return false;
+    }
+
+    bool matches_query_type(GWBUF* buffer);
+    const std::string& name() const;
+    const std::string& type() const;
+
     uint32_t       on_queries;    /*< Types of queries to inspect */
     int            times_matched; /*< Number of times this rule has been matched */
     TIMERANGE*     active;        /*< List of times when this rule is active */
+
+private:
+    std::string    m_name;          /*< Name of the rule */
+    std::string    m_type;          /*< Name of the rule */
 };
 
 /**
@@ -52,7 +60,7 @@ class WildCardRule: public Rule
 
 public:
     WildCardRule(std::string name):
-        Rule(name)
+        Rule(name, "WILDCARD")
     {
     }
 
@@ -78,7 +86,7 @@ class NoWhereClauseRule: public Rule
 
 public:
     NoWhereClauseRule(std::string name):
-        Rule(name)
+        Rule(name, "CLAUSE")
     {
     }
 
@@ -107,8 +115,8 @@ public:
     }
 
 protected:
-    ValueListRule(std::string name, const ValueList& values):
+    ValueListRule(std::string name, std::string type, const ValueList& values):
-        Rule(name),
+        Rule(name, type),
         m_values(values)
     {
     }
@@ -126,7 +134,7 @@ class ColumnsRule: public ValueListRule
 
 public:
     ColumnsRule(std::string name, const ValueList& values):
-        ValueListRule(name, values)
+        ValueListRule(name, "COLUMN", values)
     {
     }
 
@@ -143,7 +151,7 @@ class FunctionRule: public ValueListRule
 
 public:
     FunctionRule(std::string name, const ValueList& values):
-        ValueListRule(name, values)
+        ValueListRule(name, "FUNCTION", values)
     {
     }
 
@@ -161,7 +169,7 @@ class FunctionUsageRule: public ValueListRule
 
 public:
     FunctionUsageRule(std::string name, const ValueList& values):
-        ValueListRule(name, values)
+        ValueListRule(name, "FUNCTION_USAGE", values)
     {
     }
 
@@ -179,7 +187,7 @@ class LimitQueriesRule: public Rule
 
 public:
     LimitQueriesRule(std::string name, int max, int timeperiod, int holdoff):
-        Rule(name),
+        Rule(name, "THROTTLE"),
         m_max(max),
         m_timeperiod(timeperiod),
         m_holdoff(holdoff)
@@ -213,7 +221,7 @@ class RegexRule: public Rule
 
 public:
     RegexRule(std::string name, pcre2_code* re):
-        Rule(name),
+        Rule(name, "REGEX"),
         m_re(re)
     {
     }

server/modules/filter/dbfwfilter/user.cc

@@ -85,7 +85,7 @@ bool User::match_any(FW_INSTANCE* my_instance, FW_SESSION* my_session,
                 {
                     if (rule_matches(my_instance, my_session, queue, *it, fullquery))
                     {
-                        *rulename = MXS_STRDUP_A((*it)->name.c_str());
+                        *rulename = MXS_STRDUP_A((*it)->name().c_str());
                         rval = true;
                         break;
                     }
@@ -133,7 +133,7 @@ bool User::do_match(FW_INSTANCE* my_instance, FW_SESSION* my_session,
 
                     if (rule_matches(my_instance, my_session, queue, *it, fullquery))
                     {
-                        matching_rules += (*it)->name;
+                        matching_rules += (*it)->name();
                         matching_rules += " ";
                     }
                     else