From fc1435d0c2d40c4a8141d6974971b32957e93a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 4 Sep 2017 14:59:39 +0300 Subject: [PATCH] MXS-1346: Hide DbfwSession internals The DbfwSession now only exposes the necessary methods with the exception of the DOWNSTREAM and UPSTREAM structures. These will be handled when the session implements the filter template. --- .../modules/filter/dbfwfilter/dbfwfilter.cc | 44 +++++++++++-------- .../modules/filter/dbfwfilter/dbfwfilter.hh | 14 +++--- server/modules/filter/dbfwfilter/rules.cc | 9 +--- 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.cc b/server/modules/filter/dbfwfilter/dbfwfilter.cc index 528ae2399..20c44ae49 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -1315,15 +1315,13 @@ static std::string get_sql(GWBUF* buffer) } DbfwSession::DbfwSession(Dbfw* instance, MXS_SESSION* session): - query_speed(NULL), - instance(instance), - session(session) + m_instance(instance), + m_session(session) { } DbfwSession::~DbfwSession() { - delete query_speed; } void DbfwSession::set_error(std::string error) @@ -1343,19 +1341,29 @@ void DbfwSession::clear_error() std::string DbfwSession::user() const { - return session->client_dcb->user; + return m_session->client_dcb->user; } std::string DbfwSession::remote() const { - return session->client_dcb->remote; + return m_session->client_dcb->remote; +} + +QuerySpeed* DbfwSession::query_speed() +{ + return &m_qs; +} + +fw_actions DbfwSession::get_action() const +{ + return m_instance->get_action(); } int DbfwSession::send_error() { - ss_dassert(session && session->client_dcb); - DCB* dcb = session->client_dcb; - const char* db = mxs_mysql_get_current_db(session); + ss_dassert(m_session && m_session->client_dcb); + DCB* dcb = m_session->client_dcb; + const char* db = mxs_mysql_get_current_db(m_session); std::stringstream ss; ss << "Access denied for user '" << user() << "'@'" << remote() << "'"; @@ -1412,9 +1420,9 @@ int DbfwSession::routeQuery(GWBUF* buffer) if (suser) { char* rname = NULL; - bool match = suser->match(instance, this, analyzed_queue, &rname); + bool match = suser->match(m_instance, this, analyzed_queue, &rname); - switch (instance->get_action()) + switch (m_instance->get_action()) { case FW_ACTION_ALLOW: query_ok = match; @@ -1429,23 +1437,23 @@ int DbfwSession::routeQuery(GWBUF* buffer) break; default: - MXS_ERROR("Unknown dbfwfilter action: %d", instance->get_action()); + MXS_ERROR("Unknown dbfwfilter action: %d", m_instance->get_action()); ss_dassert(false); break; } - if (instance->get_log_bitmask() != FW_LOG_NONE) + if (m_instance->get_log_bitmask() != FW_LOG_NONE) { - if (match && instance->get_log_bitmask() & FW_LOG_MATCH) + if (match && m_instance->get_log_bitmask() & FW_LOG_MATCH) { MXS_NOTICE("[%s] Rule '%s' for '%s' matched by %s@%s: %s", - session->service->name, rname, suser->name(), + m_session->service->name, rname, suser->name(), user().c_str(), remote().c_str(), get_sql(buffer).c_str()); } - else if (!match && instance->get_log_bitmask() & FW_LOG_NO_MATCH) + else if (!match && m_instance->get_log_bitmask() & FW_LOG_NO_MATCH) { MXS_NOTICE("[%s] Query for '%s' by %s@%s was not matched: %s", - session->service->name, suser->name(), user().c_str(), + m_session->service->name, suser->name(), user().c_str(), remote().c_str(), get_sql(buffer).c_str()); } } @@ -1454,7 +1462,7 @@ int DbfwSession::routeQuery(GWBUF* buffer) } /** If the instance is in whitelist mode, only users that have a rule * defined for them are allowed */ - else if (instance->get_action() != FW_ACTION_ALLOW) + else if (m_instance->get_action() != FW_ACTION_ALLOW) { query_ok = true; } diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index 101d7fc87..6f2fb7f55 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -252,15 +252,17 @@ public: std::string remote() const; int routeQuery(GWBUF* query); + QuerySpeed* query_speed(); // TODO: Remove this, it exposes internals to a Rule + fw_actions get_action() const; - 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 */ - Dbfw *instance; /*< Router instance */ + MXS_DOWNSTREAM down; /*< Next object in the downstream chain */ + MXS_UPSTREAM up; /*< Next object in the upstream chain */ private: - MXS_SESSION *session; /*< Client session structure */ - std::string m_error; /*< Rule specific error message */ + Dbfw *m_instance; /*< Router instance */ + MXS_SESSION *m_session; /*< Client session structure */ + std::string m_error; /*< Rule specific error message */ + QuerySpeed m_qs; /*< How fast the user has executed queries */ }; /** Typedef for a list of strings */ diff --git a/server/modules/filter/dbfwfilter/rules.cc b/server/modules/filter/dbfwfilter/rules.cc index d30e250d1..09135120f 100644 --- a/server/modules/filter/dbfwfilter/rules.cc +++ b/server/modules/filter/dbfwfilter/rules.cc @@ -172,7 +172,7 @@ bool FunctionRule::matches_query(DbfwSession* session, GWBUF* buffer, char** msg size_t n_infos; qc_get_function_info(buffer, &infos, &n_infos); - if (n_infos == 0 && session->instance->get_action() == FW_ACTION_ALLOW) + if (n_infos == 0 && session->get_action() == FW_ACTION_ALLOW) { rval = true; } @@ -230,12 +230,7 @@ bool FunctionUsageRule::matches_query(DbfwSession* session, GWBUF* buffer, char* bool LimitQueriesRule::matches_query(DbfwSession* session, GWBUF* buffer, char** msg) const { - if (session->query_speed == NULL) - { - session->query_speed = new QuerySpeed(m_timeperiod, m_holdoff, m_max); - } - - QuerySpeed* queryspeed = session->query_speed; + QuerySpeed* queryspeed = session->query_speed(); time_t time_now = time(NULL); bool matches = false;