MXS-1346: Refactor column, function and function usage rules

The rule matching implementations are now done in the ColumnsRule,
FunctionRule and FunctionUsageRule classes. The query_matches function now
also takes the session as its first parameter to relay session related
information to the rule. This will be needed by the LimitQueriesRule
class.

server/modules/filter/dbfwfilter/dbfwfilter.cc

@@ -1024,11 +1024,7 @@ void define_columns_rule(void* scanner)
 {
     struct parser_stack* rstack = (struct parser_stack*)dbfw_yyget_extra((yyscan_t) scanner);
     ss_dassert(rstack);
-    Rule* rule = create_rule(rstack->name);
+    rstack->rule.push_front(SRule(new ColumnsRule(rstack->name, rstack->values)));
-
-    rule->type = RT_COLUMN;
-    rule->data = valuelist_to_strlink(&rstack->values);
-    rstack->rule.push_front(SRule(rule));
 }
 
 /**
@@ -1040,11 +1036,7 @@ void define_function_rule(void* scanner)
 {
     struct parser_stack* rstack = (struct parser_stack*)dbfw_yyget_extra((yyscan_t) scanner);
     ss_dassert(rstack);
-    Rule* rule = create_rule(rstack->name);
+    rstack->rule.push_front(SRule(new FunctionRule(rstack->name, rstack->values)));
-
-    rule->type = RT_FUNCTION;
-    rule->data = valuelist_to_strlink(&rstack->values);
-    rstack->rule.push_front(SRule(rule));
 }
 
 /**
@@ -1059,11 +1051,7 @@ void define_function_usage_rule(void* scanner)
 {
     struct parser_stack* rstack = (struct parser_stack*)dbfw_yyget_extra((yyscan_t) scanner);
     ss_dassert(rstack);
-    Rule* rule = create_rule(rstack->name);
+    rstack->rule.push_front(SRule(new FunctionUsageRule(rstack->name, rstack->values)));
-
-    rule->type = RT_USES_FUNCTION;
-    rule->data = valuelist_to_strlink(&rstack->values);
-    rstack->rule.push_front(SRule(rule));
 }
 
 /**
@@ -1341,16 +1329,17 @@ createInstance(const char *name, char **options, MXS_CONFIG_PARAMETER *params)
  * @param session   The session itself
  * @return Session specific data for this session
  */
-static MXS_FILTER_SESSION *
+static MXS_FILTER_SESSION* newSession(MXS_FILTER *instance, MXS_SESSION *session)
-newSession(MXS_FILTER *instance, MXS_SESSION *session)
 {
-    FW_SESSION *my_session;
+    FW_INSTANCE *my_instance = (FW_INSTANCE*)instance;
+    FW_SESSION *my_session = (FW_SESSION*)MXS_CALLOC(1, sizeof(FW_SESSION));
 
-    if ((my_session = (FW_SESSION*)MXS_CALLOC(1, sizeof(FW_SESSION))) == NULL)
+    if (my_session)
     {
-        return NULL;
+        my_session->session = session;
+        my_session->instance = my_instance;
     }
-    my_session->session = session;
+
     return (MXS_FILTER_SESSION*)my_session;
 }
 
@@ -1648,92 +1637,6 @@ bool match_throttle(FW_SESSION* my_session, SRule rule, char **msg)
     return matches;
 }
 
-void match_column(SRule rule, GWBUF *queue, bool *matches, char **msg)
-{
-    const QC_FIELD_INFO* infos;
-    size_t n_infos;
-    qc_get_field_info(queue, &infos, &n_infos);
-
-    for (size_t i = 0; i < n_infos; ++i)
-    {
-        const char* tok = infos[i].column;
-
-        STRLINK* strln = (STRLINK*)rule->data;
-        while (strln)
-        {
-            if (strcasecmp(tok, strln->value) == 0)
-            {
-                MXS_NOTICE("rule '%s': query targets forbidden column: %s",
-                           rule->name.c_str(), strln->value);
-                *msg = create_error("Permission denied to column '%s'.", strln->value);
-                *matches = true;
-                break;
-            }
-            strln = strln->next;
-        }
-    }
-}
-
-void match_function(SRule rule, GWBUF *queue, enum fw_actions mode,
-                    bool *matches, char **msg)
-{
-    const QC_FUNCTION_INFO* infos;
-    size_t n_infos;
-    qc_get_function_info(queue, &infos, &n_infos);
-
-    if (n_infos == 0 && mode == FW_ACTION_ALLOW)
-    {
-        *matches = true;
-    }
-
-    for (size_t i = 0; i < n_infos; ++i)
-    {
-        const char* tok = infos[i].name;
-
-        STRLINK* strln = (STRLINK*)rule->data;
-        while (strln)
-        {
-            if (strcasecmp(tok, strln->value) == 0)
-            {
-                MXS_NOTICE("rule '%s': query uses forbidden function: %s",
-                           rule->name.c_str(), strln->value);
-                *msg = create_error("Permission denied to function '%s'.", strln->value);
-                *matches = true;
-                break;
-            }
-            strln = strln->next;
-        }
-    }
-}
-
-void match_function_usage(SRule rule, GWBUF *queue, enum fw_actions mode,
-                          bool *matches, char **msg)
-{
-    const QC_FUNCTION_INFO* infos;
-    size_t n_infos;
-    qc_get_function_info(queue, &infos, &n_infos);
-
-    for (size_t i = 0; i < n_infos; ++i)
-    {
-        for (size_t j = 0; j < infos[i].n_fields; j++)
-        {
-            const char* tok = infos[i].fields[j].column;
-
-            for (STRLINK* s = (STRLINK*)rule->data; s; s = s->next)
-            {
-                if (strcasecmp(tok, s->value) == 0)
-                {
-                    MXS_NOTICE("rule '%s': query uses a function with forbidden column: %s",
-                               rule->name.c_str(), s->value);
-                    *msg = create_error("Permission denied to column '%s' with function.", s->value);
-                    *matches = true;
-                    return;
-                }
-            }
-        }
-    }
-}
-
 /**
  * Check if a query matches a single rule
  * @param my_instance Fwfilter instance
@@ -1772,7 +1675,7 @@ bool rule_matches(FW_INSTANCE* my_instance,
 
     if (rule->matches_query_type(queue))
     {
-        if (rule->matches_query(queue, &msg))
+        if (rule->matches_query(my_session, queue, &msg))
         {
             /** New style rule matched */
             matches = true;
@@ -1796,24 +1699,15 @@ bool rule_matches(FW_INSTANCE* my_instance,
             break;
 
         case RT_COLUMN:
-            if (is_sql)
+            /** Handled in ColumnsRule::matches_query */
-            {
-                match_column(rule, queue, &matches, &msg);
-            }
             break;
 
         case RT_FUNCTION:
-            if (is_sql)
+            /** Handled in FunctionRule::matches_query */
-            {
-                match_function(rule, queue, my_instance->action, &matches, &msg);
-            }
             break;
 
         case RT_USES_FUNCTION:
-            if (is_sql)
+            /** Handled in FunctionUsageRule::matches_query */
-            {
-                match_function_usage(rule, queue, my_instance->action, &matches, &msg);
-            }
             break;
 
         case RT_WILDCARD:

server/modules/filter/dbfwfilter/dbfwfilter.hh

@@ -194,6 +194,7 @@ typedef struct
     QUERYSPEED    *query_speed;  /*< How fast the user has executed queries */
     MXS_DOWNSTREAM down;         /*< Next object in the downstream chain */
     MXS_UPSTREAM   up;           /*< Next object in the upstream chain */
+    FW_INSTANCE   *instance;     /*< Router instance */
 } FW_SESSION;
 
 /** Typedef for a list of strings */

server/modules/filter/dbfwfilter/rules.cc

@@ -13,6 +13,8 @@
 
 #include "rules.hh"
 
+#include <algorithm>
+
 #include <maxscale/alloc.h>
 #include <maxscale/buffer.h>
 #include <maxscale/log_manager.h>
@@ -38,7 +40,7 @@ Rule::~Rule()
 {
 }
 
-bool Rule::matches_query(GWBUF* buffer, char** msg)
+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());
@@ -82,7 +84,7 @@ bool Rule::matches_query_type(GWBUF* buffer)
             (on_queries & FW_OP_CHANGE_DB));
 }
 
-bool WildCardRule::matches_query(GWBUF *queue, char **msg)
+bool WildCardRule::matches_query(FW_SESSION* session, GWBUF *queue, char **msg)
 {
     bool rval = false;
 
@@ -106,7 +108,7 @@ bool WildCardRule::matches_query(GWBUF *queue, char **msg)
     return rval;
 }
 
-bool NoWhereClauseRule::matches_query(GWBUF* buffer, char** msg)
+bool NoWhereClauseRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
 {
     bool rval = false;
 
@@ -121,7 +123,7 @@ bool NoWhereClauseRule::matches_query(GWBUF* buffer, char** msg)
     return rval;
 }
 
-bool RegexRule::matches_query(GWBUF* buffer, char** msg)
+bool RegexRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
 {
     bool rval = false;
 
@@ -147,3 +149,99 @@ bool RegexRule::matches_query(GWBUF* buffer, char** msg)
 
     return rval;
 }
+
+bool ColumnsRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
+{
+    bool rval = false;
+
+    if (query_is_sql(buffer))
+    {
+        const QC_FIELD_INFO* infos;
+        size_t n_infos;
+        qc_get_field_info(buffer, &infos, &n_infos);
+
+        for (size_t i = 0; !rval && i < n_infos; ++i)
+        {
+            std::string tok = infos[i].column;
+            ValueList::const_iterator it = std::find(m_values.begin(), m_values.end(), tok);
+
+            if (it != m_values.end())
+            {
+                MXS_NOTICE("rule '%s': query targets forbidden column: %s",
+                           name.c_str(), tok.c_str());
+                *msg = create_error("Permission denied to column '%s'.", tok.c_str());
+                rval = true;
+                break;
+            }
+        }
+    }
+
+    return rval;
+}
+
+
+bool FunctionRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
+{
+    bool rval = false;
+
+    if (query_is_sql(buffer))
+    {
+        const QC_FUNCTION_INFO* infos;
+        size_t n_infos;
+        qc_get_function_info(buffer, &infos, &n_infos);
+
+        if (n_infos == 0 && session->instance->action == FW_ACTION_ALLOW)
+        {
+            rval = true;
+        }
+        else
+        {
+            for (size_t i = 0; i < n_infos; ++i)
+            {
+                std::string tok = infos[i].name;
+                ValueList::const_iterator it = std::find(m_values.begin(), m_values.end(), tok);
+
+                if (it != m_values.end())
+                {
+                    MXS_NOTICE("rule '%s': query uses forbidden function: %s",
+                               name.c_str(), tok.c_str());
+                    *msg = create_error("Permission denied to function '%s'.", tok.c_str());
+                    rval = true;
+                    break;
+                }
+
+            }
+        }
+    }
+
+    return rval;
+}
+
+bool FunctionUsageRule::matches_query(FW_SESSION* session, GWBUF* buffer, char** msg)
+{
+    if (query_is_sql(buffer))
+    {
+        const QC_FUNCTION_INFO* infos;
+        size_t n_infos;
+        qc_get_function_info(buffer, &infos, &n_infos);
+
+        for (size_t i = 0; i < n_infos; ++i)
+        {
+            for (size_t j = 0; j < infos[i].n_fields; j++)
+            {
+                std::string tok = infos[i].fields[j].column;
+                ValueList::const_iterator it = std::find(m_values.begin(), m_values.end(), tok);
+
+                if (it != m_values.end())
+                {
+                    MXS_NOTICE("rule '%s': query uses a function with forbidden column: %s",
+                               name.c_str(), tok.c_str());
+                    *msg = create_error("Permission denied to column '%s' with function.", tok.c_str());
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}

server/modules/filter/dbfwfilter/rules.hh

@@ -30,7 +30,7 @@ class Rule
 public:
     Rule(std::string name);
     virtual ~Rule();
-    virtual bool matches_query(GWBUF* buffer, char** msg);
+    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);
 
@@ -65,7 +65,7 @@ public:
         return true;
     }
 
-    bool matches_query(GWBUF* buffer, char** msg);
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
 };
 
 /**
@@ -91,100 +91,84 @@ public:
         return true;
     }
 
-    bool matches_query(GWBUF* buffer, char** msg);
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
 
 };
 
+class ValueListRule: public Rule
+{
+    ValueListRule(const ValueListRule&);
+    ValueListRule& operator=(const ValueListRule&);
+
+public:
+    bool need_full_parsing(GWBUF* buffer) const
+    {
+        return true;
+    }
+
+protected:
+    ValueListRule(std::string name, const ValueList& values):
+        Rule(name),
+        m_values(values)
+    {
+    }
+
+    ValueList m_values;
+};
+
 /**
  * Matches if a query uses one of the columns
  */
-class ColumnsRule: public Rule
+class ColumnsRule: public ValueListRule
 {
     ColumnsRule(const ColumnsRule&);
     ColumnsRule& operator=(const ColumnsRule&);
 
 public:
     ColumnsRule(std::string name, const ValueList& values):
-        Rule(name),
+        ValueListRule(name, values)
-        m_values(values)
     {
     }
 
-    ~ColumnsRule()
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
-    {
-    }
-
-    bool need_full_parsing(GWBUF* buffer) const
-    {
-        return true;
-    }
-
-    bool matches_query(GWBUF* buffer, char** msg);
-
-private:
-    ValueList m_values;
 };
 
 /**
  * Matches if a query uses one of the functions
  */
-class FunctionRule: public Rule
+class FunctionRule: public ValueListRule
 {
     FunctionRule(const FunctionRule&);
     FunctionRule& operator=(const FunctionRule&);
 
 public:
     FunctionRule(std::string name, const ValueList& values):
-        Rule(name),
+        ValueListRule(name, values)
-        m_values(values)
     {
     }
 
-    ~FunctionRule()
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
-    {
-    }
-
-    bool need_full_parsing(GWBUF* buffer) const
-    {
-        return true;
-    }
-
-    bool matches_query(GWBUF* buffer, char** msg);
-
-private:
-    ValueList m_values;
 };
 
+
 /**
  * Matches if a query uses any functions
  */
-class FunctionUsageRule: public Rule
+class FunctionUsageRule: public ValueListRule
 {
     FunctionUsageRule(const FunctionUsageRule&);
     FunctionUsageRule& operator=(const FunctionUsageRule&);
 
 public:
     FunctionUsageRule(std::string name, const ValueList& values):
-        Rule(name),
+        ValueListRule(name, values)
-        m_values(values)
     {
     }
 
-    ~FunctionUsageRule()
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
-    {
-    }
-
-    bool need_full_parsing(GWBUF* buffer) const
-    {
-        return true;
-    }
-
-    bool matches_query(GWBUF* buffer, char** msg);
-
-private:
-    ValueList m_values;
 };
 
+
 /**
  * Matches if a queries are executed too quickly
  */
@@ -211,7 +195,7 @@ public:
         return true;
     }
 
-    bool matches_query(GWBUF* buffer, char** msg);
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
 
 private:
     int m_max;
@@ -243,7 +227,7 @@ public:
         return false;
     }
 
-    bool matches_query(GWBUF* buffer, char** msg);
+    bool matches_query(FW_SESSION* session, GWBUF* buffer, char** msg);
 
 private:
     mxs::Closer<pcre2_code*> m_re;