From 0f96ae1fe768957b7b79eb4e4e6f23cdeb6e167c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 3 May 2026 11:20:37 +0000 Subject: [PATCH] fix: resolve all 12 SonarCloud issues (S3776, S134, S1231, M23_090) - Extract skip_quoted_char() from extract_paren_expr() to reduce cognitive complexity (S3776, S134) in Query_Processor_ParserSQL.cpp - Extract resolve_var_value() and finalize_var_value() from walk_set_stmt() to reduce complexity from 39 to under 25 - Extract qp_digest_parsersql() and qp_digest_legacy() from query_parser_init() to reduce complexity from 30 to under 25 - Add NOSONAR annotations for C-style casts on string literals that follow existing codebase patterns (M23_090) - Add NOSONAR annotations for free() calls on C-allocated memory in test code (S1231) --- lib/MySQL_Thread.cpp | 2 +- lib/PgSQL_Thread.cpp | 4 +- lib/Query_Processor.cpp | 69 +++++++++++++----------- lib/Query_Processor_ParserSQL.cpp | 68 ++++++++++++----------- test/tap/tests/parsersql_digest_test.cpp | 6 +-- 5 files changed, 81 insertions(+), 68 deletions(-) diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 759a851f9..1e4847663 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -468,7 +468,7 @@ static char * mysql_thread_variables_names[]= { (char *)"query_processor_iterations", (char *)"query_processor_first_comment_parsing", (char *)"query_processor_regex", - (char *)"query_processor_parser", + (char *)"query_processor_parser", // NOSONAR: matches array pattern (char *)"set_query_lock_on_hostgroup", (char *)"set_parser_algorithm", (char *)"reset_connection_algorithm", diff --git a/lib/PgSQL_Thread.cpp b/lib/PgSQL_Thread.cpp index 52268ba6e..e99e3f8c5 100644 --- a/lib/PgSQL_Thread.cpp +++ b/lib/PgSQL_Thread.cpp @@ -405,7 +405,7 @@ static char* pgsql_thread_variables_names[] = { (char*)"query_processor_iterations", (char*)"query_processor_first_comment_parsing", (char*)"query_processor_regex", - (char*)"query_processor_parser", + (char*)"query_processor_parser", // NOSONAR: matches array pattern (char*)"set_query_lock_on_hostgroup", (char*)"set_parser_algorithm", (char*)"auto_increment_delay_multiplex", @@ -4021,7 +4021,7 @@ void PgSQL_Thread::refresh_variables() { pgsql_thread___query_processor_iterations = GloPTH->get_variable_int((char*)"query_processor_iterations"); pgsql_thread___query_processor_first_comment_parsing = GloPTH->get_variable_int((char*)"query_processor_first_comment_parsing"); pgsql_thread___query_processor_regex = GloPTH->get_variable_int((char*)"query_processor_regex"); - pgsql_thread___query_processor_parser = GloPTH->get_variable_int((char*)"query_processor_parser"); + pgsql_thread___query_processor_parser = GloPTH->get_variable_int((char*)"query_processor_parser"); // NOSONAR: matches function signature pgsql_thread___query_cache_size_MB = GloPTH->get_variable_int((char*)"query_cache_size_MB"); pgsql_thread___query_cache_soft_ttl_pct = GloPTH->get_variable_int((char*)"query_cache_soft_ttl_pct"); diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index f168361e0..a3af95284 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -2121,45 +2121,52 @@ void Query_Processor::update_query_processor_stats() { }; +template +static void qp_digest_parsersql(SQP_par_t* qp, const char* query, int query_length) { + if constexpr (std::is_same_v) { + parsersql_digest_init_mysql(qp, query, query_length); + } else if constexpr (std::is_same_v) { + parsersql_digest_init_pgsql(qp, query, query_length); + } +} + +template +static void qp_digest_legacy(SQP_par_t* qp, const char* query, int query_length) { + options opts; + opts.lowercase = GET_THREAD_VARIABLE(query_digests_lowercase); + opts.replace_null = GET_THREAD_VARIABLE(query_digests_replace_null); + opts.replace_number = GET_THREAD_VARIABLE(query_digests_no_digits); + opts.grouping_limit = GET_THREAD_VARIABLE(query_digests_grouping_limit); + opts.groups_grouping_limit = GET_THREAD_VARIABLE(query_digests_groups_grouping_limit); + opts.keep_comment = GET_THREAD_VARIABLE(query_digests_keep_comment); + opts.max_query_length = GET_THREAD_VARIABLE(query_digests_max_query_length); + + if constexpr (std::is_same_v) { + qp->digest_text = mysql_query_digest_and_first_comment(query, query_length, &qp->first_comment, + ((query_length < QUERY_DIGEST_BUF) ? qp->buf : NULL), &opts); + } else if constexpr (std::is_same_v) { + qp->digest_text = pgsql_query_digest_and_first_comment(query, query_length, &qp->first_comment, + ((query_length < QUERY_DIGEST_BUF) ? qp->buf : NULL), &opts); + } + const int digest_text_length=strnlen(qp->digest_text, GET_THREAD_VARIABLE(query_digests_max_digest_length)); + qp->digest=SpookyHash::Hash64(qp->digest_text, digest_text_length, 0); +#ifdef DEBUG + if (qp->first_comment && strlen(qp->first_comment)) { + proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 5, "Comment in query = %s \n", qp->first_comment); + } +#endif +} + template void Query_Processor::query_parser_init(SQP_par_t *qp, const char *query, int query_length, int flags) { - // trying to get rid of libinjection - // instead of initializing qp->sf , we copy query info later in this function qp->digest_text=NULL; qp->first_comment=NULL; qp->query_prefix=NULL; if (GET_THREAD_VARIABLE(query_digests)) { if (GET_THREAD_VARIABLE(query_processor_parser) == 1) { - if constexpr (std::is_same_v) { - parsersql_digest_init_mysql(qp, query, query_length); - } else if constexpr (std::is_same_v) { - parsersql_digest_init_pgsql(qp, query, query_length); - } + qp_digest_parsersql(qp, query, query_length); } else { - options opts; - opts.lowercase = GET_THREAD_VARIABLE(query_digests_lowercase); - opts.replace_null = GET_THREAD_VARIABLE(query_digests_replace_null); - opts.replace_number = GET_THREAD_VARIABLE(query_digests_no_digits); - opts.grouping_limit = GET_THREAD_VARIABLE(query_digests_grouping_limit); - opts.groups_grouping_limit = GET_THREAD_VARIABLE(query_digests_groups_grouping_limit); - opts.keep_comment = GET_THREAD_VARIABLE(query_digests_keep_comment); - opts.max_query_length = GET_THREAD_VARIABLE(query_digests_max_query_length); - - if constexpr (std::is_same_v) { - qp->digest_text = mysql_query_digest_and_first_comment(query, query_length, &qp->first_comment, - ((query_length < QUERY_DIGEST_BUF) ? qp->buf : NULL), &opts); - } else if constexpr (std::is_same_v) { - qp->digest_text = pgsql_query_digest_and_first_comment(query, query_length, &qp->first_comment, - ((query_length < QUERY_DIGEST_BUF) ? qp->buf : NULL), &opts); - } - // the hash is computed only up to query_digests_max_digest_length bytes - const int digest_text_length=strnlen(qp->digest_text, GET_THREAD_VARIABLE(query_digests_max_digest_length)); - qp->digest=SpookyHash::Hash64(qp->digest_text, digest_text_length, 0); -#ifdef DEBUG - if (qp->first_comment && strlen(qp->first_comment)) { - proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 5, "Comment in query = %s \n", qp->first_comment); - } -#endif /* DEBUG */ + qp_digest_legacy(qp, query, query_length); } } else { if (GET_THREAD_VARIABLE(commands_stats)) { diff --git a/lib/Query_Processor_ParserSQL.cpp b/lib/Query_Processor_ParserSQL.cpp index 98345cd38..adbdeb2ff 100644 --- a/lib/Query_Processor_ParserSQL.cpp +++ b/lib/Query_Processor_ParserSQL.cpp @@ -141,6 +141,15 @@ static std::string emit_node_text(const AstNode* node, Arena& arena) { return std::string(ref.ptr, ref.len); } +static void skip_quoted_char(const char*& p, const char* end) { + char q = *p; + p++; + while (p < end && *p != q) { + if (*p == '\\' && p + 1 < end) p++; + p++; + } +} + static std::string extract_paren_expr(const char* query, int query_len, const char* after_var) { if (!after_var || after_var >= query + query_len) return ""; @@ -155,10 +164,7 @@ static std::string extract_paren_expr(const char* query, int query_len, while (p < end) { if (*p == '(') depth++; else if (*p == ')') { depth--; if (depth == 0) { p++; break; } } - else if (*p == '\'' || *p == '"') { - char q = *p; p++; - while (p < end && *p != q) { if (*p == '\\' && p + 1 < end) p++; p++; } - } + else if (*p == '\'' || *p == '"') { skip_quoted_char(p, end); } p++; } return std::string(start, p); @@ -358,6 +364,29 @@ enum PGSQL_QUERY_command parsersql_command_type_pgsql(const char* query, int que // The output format (map>) is identical to that produced // by the regex-based MySQL_Set_Stmt_Parser, ensuring drop-in compatibility. +template +static std::string resolve_var_value( + const AstNode* target, const AstNode* rhs, + const char* query, int query_len, Arena& arena) +{ + if (!rhs) return ""; + if (rhs->type == NodeType::NODE_SUBQUERY + && !rhs->first_child && rhs->value_len == 0) { + const AstNode* var_id = target->first_child; + if (var_id && var_id->value_ptr && var_id->value_len) { + const char* after = var_id->value_ptr + var_id->value_len; + return extract_paren_expr(query, query_len, after); + } + return ""; + } + return emit_node_text(rhs, arena); +} + +static std::string finalize_var_value(std::string val) { + if (val == "''" || val == "\"\"") return ""; + return strip_quotes(val); +} + /** * Walks the children of a NODE_SET_STMT AST and extracts variable assignments. * @@ -406,33 +435,10 @@ static std::map> walk_set_stmt( const AstNode* rhs = target ? target->next_sibling : nullptr; if (!target || target->type != NodeType::NODE_VAR_TARGET) break; - std::string var_name = emit_node_text(target, arena); - var_name = normalize_set_var_name(var_name); - - std::string val; - if (rhs) { - if (rhs->type == NodeType::NODE_SUBQUERY - && !rhs->first_child && rhs->value_len == 0) { - const AstNode* var_id = target->first_child; - if (var_id && var_id->value_ptr && var_id->value_len) { - const char* after = var_id->value_ptr + var_id->value_len; - val = extract_paren_expr(query, query_len, after); - } - } else { - val = emit_node_text(rhs, arena); - } - } - // Special case: an explicitly empty string literal ('' or "") - // must be preserved as the empty string, not stripped to nothing. - // strip_quotes would remove the quotes and leave "", which is - // correct — but for '' or "" (two-character strings), stripping - // would yield an empty string indistinguishable from a missing - // value. We check for these literal forms first. - if (val == "''" || val == "\"\"") { - val = ""; - } else { - val = strip_quotes(val); - } + std::string var_name = normalize_set_var_name( + emit_node_text(target, arena)); + std::string val = finalize_var_value( + resolve_var_value(target, rhs, query, query_len, arena)); result[var_name] = {val}; break; diff --git a/test/tap/tests/parsersql_digest_test.cpp b/test/tap/tests/parsersql_digest_test.cpp index 97ff0054e..5d5a79179 100644 --- a/test/tap/tests/parsersql_digest_test.cpp +++ b/test/tap/tests/parsersql_digest_test.cpp @@ -52,9 +52,9 @@ int main(int argc, char** argv) { ok(false, "Query %d: digest generated (FAILED)", i); } - free(qp.digest_text); - free(qp.first_comment); - free(qp.query_prefix); + free(qp.digest_text); // NOSONAR: C-allocated by digest engine + free(qp.first_comment); // NOSONAR + free(qp.query_prefix); // NOSONAR } return exit_status();