fix: address PR review feedback from coding agents

Actionable fixes from CodeRabbit / agent reviews on PR #5549:

1. include/Stats_Tool_Handler.h: add #include <cstdint> — uint64_t
   used in parse_digest_filter() without direct include

2. lib/MCP_Endpoint.cpp: use strncasecmp() for Bearer token prefix
   check — RFC 7235 Section 2.1 says auth-scheme is case-insensitive.
   This is a real bug fix in production code.

3. test/tap/tests/unit/sqlite3db_unit-t.cpp: guard result->rows_count
   dereference with null check to avoid segfault if execute_statement
   returns null

4. test/tap/tests/unit/genai_mcp_endpoint_unit-t.cpp: remove memset()
   calls on MCP_Threads_Handler that leaked constructor's strdup("")
   allocations for auth fields

5. test/tap/groups/check_groups.py: use os.walk() instead of hardcoded
   directory list so new test subdirectories are discovered

6. test/tap/tests/unit/admin_disk_upgrade_unit-t.cpp: add comment
   documenting intentional calloc pattern (ProxySQL_Admin constructor
   has too many daemon deps for unit tests)
v3.0-CodeCov0325
Rene Cannao 3 weeks ago
parent 4fb7aac73f
commit acaa100dc8

@ -3,6 +3,7 @@
#ifdef PROXYSQLGENAI
#include <cstdint>
#include <map>
#include <string>
#include <vector>

@ -31,10 +31,10 @@ bool MCP_JSONRPC_Resource::validate_bearer_token(const std::string& auth_header,
return false;
}
// Check if it's a Bearer token
// Check if it's a Bearer token (RFC 7235: auth-scheme is case-insensitive)
const std::string bearer_prefix = "Bearer ";
if (auth_header.length() <= bearer_prefix.length() ||
auth_header.compare(0, bearer_prefix.length(), bearer_prefix) != 0) {
strncasecmp(auth_header.c_str(), bearer_prefix.c_str(), bearer_prefix.length()) != 0) {
return false;
}

@ -25,27 +25,27 @@ import sys
def find_compiled_tests(tap_root):
"""Find all compiled test binaries (*-t) under the TAP test directories."""
test_dirs = [
scan_roots = [
os.path.join(tap_root, "tests"),
os.path.join(tap_root, "tests", "unit"),
os.path.join(tap_root, "tests_with_deps", "deprecate_eof_support"),
os.path.join(tap_root, "tests_with_deps"),
]
compiled = set()
for d in test_dirs:
if not os.path.isdir(d):
for scan_root in scan_roots:
if not os.path.isdir(scan_root):
continue
for entry in os.listdir(d):
path = os.path.join(d, entry)
# Must be a file ending in -t, executable, and an ELF binary (not .cpp, .py, etc.)
if (
entry.endswith("-t")
and os.path.isfile(path)
and not entry.endswith(".cpp")
and os.access(path, os.X_OK)
and _is_elf(path)
):
compiled.add(entry)
for dirpath, _, filenames in os.walk(scan_root):
for entry in filenames:
path = os.path.join(dirpath, entry)
# Must be a file ending in -t, executable, and an ELF binary (not .cpp, .py, etc.)
if (
entry.endswith("-t")
and os.path.isfile(path)
and not entry.endswith(".cpp")
and os.access(path, os.X_OK)
and _is_elf(path)
):
compiled.add(entry)
return compiled

@ -51,6 +51,12 @@ public:
ProxySQL_Admin *admin;
TestDiskUpgrade() {
// NOTE: We intentionally skip the ProxySQL_Admin constructor because it
// requires GloProxyStats, GloAdmin, and other daemon globals that are
// unavailable in unit tests. The disk_upgrade_*() methods only access
// this->configdb (a SQLite3DB pointer), so zeroed memory + setting
// configdb is sufficient for these tests. This is technically UB in
// C++ but is a pragmatic choice for test-only code.
void *mem = calloc(1, sizeof(ProxySQL_Admin));
admin = reinterpret_cast<ProxySQL_Admin*>(mem);
SQLite3DB *db_ = new SQLite3DB();

@ -181,8 +181,6 @@ static void test_long_token() {
static void test_jsonrpc_response_with_string_id() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_response(
@ -197,8 +195,6 @@ static void test_jsonrpc_response_with_string_id() {
static void test_jsonrpc_response_with_numeric_id() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_response(
@ -213,8 +209,6 @@ static void test_jsonrpc_response_with_numeric_id() {
static void test_jsonrpc_response_with_null_id() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_response(
@ -233,8 +227,6 @@ static void test_jsonrpc_response_with_null_id() {
static void test_jsonrpc_error_parse_error() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_error(
@ -250,8 +242,6 @@ static void test_jsonrpc_error_parse_error() {
static void test_jsonrpc_error_method_not_found() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_error(
@ -266,8 +256,6 @@ static void test_jsonrpc_error_method_not_found() {
static void test_jsonrpc_error_null_id() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_error(
@ -282,8 +270,6 @@ static void test_jsonrpc_error_null_id() {
static void test_jsonrpc_error_unauthorized() {
MCP_Threads_Handler handler;
memset(&handler.variables, 0, sizeof(handler.variables));
memset(&handler.status_variables, 0, sizeof(handler.status_variables));
auto resource = MCP_Endpoint_Test::make_resource(&handler, "config");
std::string result = MCP_Endpoint_Test::call_create_jsonrpc_error(

@ -46,7 +46,7 @@ static void test_execute_insert_select() {
char *error = nullptr;
SQLite3_result *result = db->execute_statement("SELECT COUNT(*) FROM test_t", &error);
ok(result != nullptr, "execute_statement: returns result for SELECT");
ok(result->rows_count == 1, "execute_statement: COUNT(*) returns 1 row");
ok(result != nullptr && result->rows_count == 1, "execute_statement: COUNT(*) returns 1 row");
if (result && result->rows_count > 0) {
ok(strcmp(result->rows[0]->fields[0], "2") == 0,
"execute_statement: COUNT(*) = 2");

Loading…
Cancel
Save