From 65b4ac7c1bef4c536b216d4b99a3d3c890c73525 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Wed, 20 Mar 2019 15:56:16 +0200 Subject: [PATCH] MXS-2389 Handle MariaDB comment correctly A non version specific executable comment, such as "/*! SELECT 1; */" is during classification handled as if it would not be a comment. That is, the contained statement will *always* be parsed. A version specific executable comment, such as "/*!99999 CREATE PROCEDURE bypass BEGIN */ SELECT ... " is during classification handled as it would be a general comment. That is, the contained statement will *never* be parsed. In addition, in the latter case the parse result will never be better than QC_QUERY_PARTIALLY_PARSED. The rationale is that since the comment is version specific, we cannot know how the server will actually interpret the statement. This will have an impact on the masking filter and the database firewall that now will reject statements containing _version specific_ executable comments. --- query_classifier/qc_sqlite/qc_sqlite.cc | 26 +++++++++++++++++++ .../sqlite-src-3110100/src/tokenize.c | 16 +++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 4193e824f..1a1c94a3e 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -2654,6 +2654,12 @@ public: return rv; } + void maxscaleSetStatusCap(int cap) + { + mxb_assert(cap >= QC_QUERY_TOKENIZED && cap <= QC_QUERY_PARSED); + m_status_cap = static_cast(cap); + } + void maxscaleRenameTable(Parse* pParse, SrcList* pTables) { mxb_assert(this_thread.initialized); @@ -3066,6 +3072,7 @@ private: QcSqliteInfo(uint32_t cllct) : m_refs(1) , m_status(QC_QUERY_INVALID) + , m_status_cap(QC_QUERY_PARSED) , m_collect(cllct) , m_collected(0) , m_pQuery(NULL) @@ -3271,6 +3278,7 @@ public: // TODO: Make these private once everything's been updated. int32_t m_refs; // The reference count. qc_parse_result_t m_status; // The validity of the information in this structure. + qc_parse_result_t m_status_cap; // The cap on 'm_status', it won't be set to higher than this. uint32_t m_collect; // What information should be collected. uint32_t m_collected; // What information has been collected. const char* m_pQuery; // The query passed to sqlite. @@ -3372,6 +3380,7 @@ extern "C" extern int maxscaleComment(); extern int maxscaleKeyword(int token); + extern void maxscaleSetStatusCap(int cap); extern int maxscaleTranslateKeyword(int token); } @@ -3423,6 +3432,11 @@ static void parse_query_string(const char* query, int len, bool suppress_logging const char* suffix = (len > max_len ? "..." : ""); const char* format; + if (this_thread.pInfo->m_status > this_thread.pInfo->m_status_cap) + { + this_thread.pInfo->m_status = this_thread.pInfo->m_status_cap; + } + if (this_thread.pInfo->m_operation == QUERY_OP_EXPLAIN) { this_thread.pInfo->m_status = QC_QUERY_PARSED; @@ -4322,6 +4336,18 @@ void maxscaleLock(Parse* pParse, mxs_lock_t type, SrcList* pTables) QC_EXCEPTION_GUARD(pInfo->maxscaleLock(pParse, type, pTables)); } +void maxscaleSetStatusCap(int cap) +{ + QC_TRACE(); + + mxb_assert((cap >= QC_QUERY_INVALID) && (cap <= QC_QUERY_PARSED)); + + QcSqliteInfo* pInfo = this_thread.pInfo; + mxb_assert(pInfo); + + QC_EXCEPTION_GUARD(pInfo->maxscaleSetStatusCap(cap)); +} + int maxscaleTranslateKeyword(int token) { QC_TRACE(); diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c b/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c index 3284a2e2e..11360dc56 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/tokenize.c @@ -259,10 +259,18 @@ int sqlite3GetToken(const unsigned char *z, int *tokenType){ // MySQL-specific code for (i=3, c=z[2]; (c!='*' || z[i]!='/') && (c=z[i])!=0; i++){} if (c=='*' && z[i]=='/'){ - char* znc = (char*) z; - znc[0]=znc[1]=znc[2]=znc[i-1]=znc[i]=' '; // Remove comment chars, i.e. "/*!" and "*/". - for (i=3; sqlite3Isdigit(z[i]); ++i){} // Jump over the MySQL version number. - for (; sqlite3Isspace(z[i]); ++i){} // Jump over any space. + if (sqlite3Isdigit(z[3])) { + // A version specific executable comment, e.g. "/*!99999 ..." => never parsed. + extern void maxscaleSetStatusCap(int); + maxscaleSetStatusCap(2); // QC_QUERY_PARTIALLY_PARSED, see query_classifier.h:qc_parse_result + ++i; // Next after the trailing '/' + } + else { + // A non-version specific executable comment, e.g. "/*! select 1 */ => always parsed. + char* znc = (char*) z; + znc[0]=znc[1]=znc[2]=znc[i-1]=znc[i]=' '; // Remove comment chars, i.e. "/*!" and "*/". + for (i=3; sqlite3Isspace(z[i]); ++i){} // Jump over any space. + } } } else { for(i=3, c=z[2]; (c!='*' || z[i]!='/') && (c=z[i])!=0; i++){}