From 573a3fd256b2e17aec2d413f28dd33c303619889 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 22 Mar 2026 21:58:11 +0000 Subject: [PATCH] Address AI code review feedback on PgSQL FFTO tests (#5517) Fixes from Copilot and CodeRabbit reviews on PR #5526: 1. Buffer overflow risk in mysql_real_escape_string: All 7 PgSQL test files used fixed 256-byte buffers for escaped credentials. Changed to VLAs sized from strlen() (2*len+1), matching the safe pattern used in the existing test_ffto_pgsql-t.cpp. 2. NULL dereference in polling loops: mysql_store_result() can return NULL, and calling mysql_fetch_row(NULL) is undefined behavior. Added NULL checks in all verify_pg_digest(), poll_pg_digest_count(), verify_digest(), and poll_digest_count() polling loops across both PgSQL and MySQL test files. In loop contexts, uses continue; in non-loop contexts, uses conditional fetch (res ? fetch(res) : NULL). Co-Authored-By: Claude Opus 4.6 (1M context) --- test/tap/tests/test_ffto_mysql_bypass_recovery-t.cpp | 3 ++- test/tap/tests/test_ffto_mysql_concurrent-t.cpp | 4 +++- test/tap/tests/test_ffto_mysql_large_queries-t.cpp | 4 +++- test/tap/tests/test_ffto_mysql_large_resultsets-t.cpp | 3 ++- test/tap/tests/test_ffto_mysql_mixed_protocol-t.cpp | 1 + test/tap/tests/test_ffto_mysql_transactions-t.cpp | 2 ++ test/tap/tests/test_ffto_pgsql_command_types-t.cpp | 4 +++- test/tap/tests/test_ffto_pgsql_concurrent-t.cpp | 2 +- test/tap/tests/test_ffto_pgsql_errors-t.cpp | 3 ++- test/tap/tests/test_ffto_pgsql_large_resultsets-t.cpp | 3 ++- test/tap/tests/test_ffto_pgsql_mixed_protocol-t.cpp | 3 ++- test/tap/tests/test_ffto_pgsql_pipeline-t.cpp | 3 ++- test/tap/tests/test_ffto_pgsql_stmt_portal-t.cpp | 3 ++- 13 files changed, 27 insertions(+), 11 deletions(-) diff --git a/test/tap/tests/test_ffto_mysql_bypass_recovery-t.cpp b/test/tap/tests/test_ffto_mysql_bypass_recovery-t.cpp index 64bfcdf16..5f1b47dff 100644 --- a/test/tap/tests/test_ffto_mysql_bypass_recovery-t.cpp +++ b/test/tap/tests/test_ffto_mysql_bypass_recovery-t.cpp @@ -81,6 +81,7 @@ static int poll_digest_count(MYSQL* admin, const char* template_text, int max_at for (int attempt = 0; attempt < max_attempts; attempt++) { run_q(admin, query); MYSQL_RES* res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } MYSQL_ROW row = mysql_fetch_row(res); count = row ? atoi(row[0]) : 0; if (res) mysql_free_result(res); @@ -237,7 +238,7 @@ int main(int argc, char** argv) { run_q(admin, "SELECT count(*) FROM stats_mysql_query_digest " "WHERE digest_text LIKE '%post_bypass_a%'"); MYSQL_RES* res = mysql_store_result(admin); - MYSQL_ROW row = mysql_fetch_row(res); + MYSQL_ROW row = res ? mysql_fetch_row(res) : NULL; int count = row ? atoi(row[0]) : -1; ok(count == 0, "Post-bypass query NOT recorded on conn A (bypass sticky, count: %d)", count); diff --git a/test/tap/tests/test_ffto_mysql_concurrent-t.cpp b/test/tap/tests/test_ffto_mysql_concurrent-t.cpp index d92f20e0d..07b8f78af 100644 --- a/test/tap/tests/test_ffto_mysql_concurrent-t.cpp +++ b/test/tap/tests/test_ffto_mysql_concurrent-t.cpp @@ -328,6 +328,7 @@ int main(int argc, char** argv) { "SELECT SUM(count_star) FROM stats_mysql_query_digest " "WHERE digest_text LIKE '%ffto_concurrent%'"); MYSQL_RES* res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } MYSQL_ROW row = mysql_fetch_row(res); total_count = row && row[0] ? atoi(row[0]) : 0; if (res) mysql_free_result(res); @@ -352,7 +353,8 @@ int main(int argc, char** argv) { "SELECT SUM(sum_rows_affected) FROM stats_mysql_query_digest " "WHERE digest_text LIKE '%INSERT INTO ffto_concurrent%'"); MYSQL_RES* res = mysql_store_result(admin); - MYSQL_ROW row = mysql_fetch_row(res); + + MYSQL_ROW row = res ? mysql_fetch_row(res) : NULL; int total_affected = row && row[0] ? atoi(row[0]) : 0; if (res) mysql_free_result(res); diff --git a/test/tap/tests/test_ffto_mysql_large_queries-t.cpp b/test/tap/tests/test_ffto_mysql_large_queries-t.cpp index 07c8e9de1..31fec97ef 100644 --- a/test/tap/tests/test_ffto_mysql_large_queries-t.cpp +++ b/test/tap/tests/test_ffto_mysql_large_queries-t.cpp @@ -97,6 +97,7 @@ void verify_digest(MYSQL* admin, const char* template_text, int expected_count, int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); @@ -150,7 +151,7 @@ void verify_no_digest(MYSQL* admin, const char* template_text) { template_text); run_q(admin, query); MYSQL_RES* res = mysql_store_result(admin); - MYSQL_ROW row = mysql_fetch_row(res); + MYSQL_ROW row = res ? mysql_fetch_row(res) : NULL; int count = row ? atoi(row[0]) : -1; ok(count == 0, "No digest recorded for '%s' (count: %d)", template_text, count); if (res) mysql_free_result(res); @@ -428,6 +429,7 @@ int main(int argc, char** argv) { for (int attempt = 0; attempt < 20; attempt++) { run_q(admin, check); MYSQL_RES* res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } MYSQL_ROW row = mysql_fetch_row(res); count = row ? atoi(row[0]) : 0; mysql_free_result(res); diff --git a/test/tap/tests/test_ffto_mysql_large_resultsets-t.cpp b/test/tap/tests/test_ffto_mysql_large_resultsets-t.cpp index 824dcb065..89b4ef360 100644 --- a/test/tap/tests/test_ffto_mysql_large_resultsets-t.cpp +++ b/test/tap/tests/test_ffto_mysql_large_resultsets-t.cpp @@ -119,6 +119,7 @@ void verify_digest(MYSQL* admin, const char* template_text, int expected_count, continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); @@ -173,7 +174,7 @@ void verify_no_digest(MYSQL* admin, const char* template_text) { template_text); run_q(admin, query); MYSQL_RES* res = mysql_store_result(admin); - MYSQL_ROW row = mysql_fetch_row(res); + MYSQL_ROW row = res ? mysql_fetch_row(res) : NULL; int count = row ? atoi(row[0]) : -1; ok(count == 0, "No digest recorded for '%s' (count: %d)", template_text, count); if (res) mysql_free_result(res); diff --git a/test/tap/tests/test_ffto_mysql_mixed_protocol-t.cpp b/test/tap/tests/test_ffto_mysql_mixed_protocol-t.cpp index 0825df292..7040e9e68 100644 --- a/test/tap/tests/test_ffto_mysql_mixed_protocol-t.cpp +++ b/test/tap/tests/test_ffto_mysql_mixed_protocol-t.cpp @@ -95,6 +95,7 @@ void verify_digest(MYSQL* admin, const char* template_text, int expected_count, int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); diff --git a/test/tap/tests/test_ffto_mysql_transactions-t.cpp b/test/tap/tests/test_ffto_mysql_transactions-t.cpp index 93ae669df..4d5a3cfd6 100644 --- a/test/tap/tests/test_ffto_mysql_transactions-t.cpp +++ b/test/tap/tests/test_ffto_mysql_transactions-t.cpp @@ -98,6 +98,7 @@ void verify_digest(MYSQL* admin, const char* template_text, int expected_count, int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); @@ -363,6 +364,7 @@ int main(int argc, char** argv) { int rc = run_q(admin, q); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); res = NULL; diff --git a/test/tap/tests/test_ffto_pgsql_command_types-t.cpp b/test/tap/tests/test_ffto_pgsql_command_types-t.cpp index d4c3d19f8..9cf4a8a10 100644 --- a/test/tap/tests/test_ffto_pgsql_command_types-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_command_types-t.cpp @@ -84,6 +84,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); @@ -135,6 +136,7 @@ static int poll_pg_digest_count(MYSQL* admin, const char* template_text) { for (int attempt = 0; attempt < 20; attempt++) { run_q(admin, query); MYSQL_RES* res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } MYSQL_ROW row = mysql_fetch_row(res); count = row ? atoi(row[0]) : 0; if (res) mysql_free_result(res); @@ -174,7 +176,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char escaped_user[256], escaped_pass[256]; + char escaped_user[2 * strlen(cl.pgsql_root_username) + 1]; char escaped_pass[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, escaped_user, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, escaped_pass, cl.pgsql_root_password, diff --git a/test/tap/tests/test_ffto_pgsql_concurrent-t.cpp b/test/tap/tests/test_ffto_pgsql_concurrent-t.cpp index 26d030402..83719aa54 100644 --- a/test/tap/tests/test_ffto_pgsql_concurrent-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_concurrent-t.cpp @@ -138,7 +138,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char eu[256], ep[256]; + char eu[2 * strlen(cl.pgsql_root_username) + 1]; char ep[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, eu, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, ep, cl.pgsql_root_password, diff --git a/test/tap/tests/test_ffto_pgsql_errors-t.cpp b/test/tap/tests/test_ffto_pgsql_errors-t.cpp index 853096b10..741e9704c 100644 --- a/test/tap/tests/test_ffto_pgsql_errors-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_errors-t.cpp @@ -81,6 +81,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); res = NULL; @@ -136,7 +137,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char eu[256], ep[256]; + char eu[2 * strlen(cl.pgsql_root_username) + 1]; char ep[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, eu, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, ep, cl.pgsql_root_password, strlen(cl.pgsql_root_password)); char uq[1024]; diff --git a/test/tap/tests/test_ffto_pgsql_large_resultsets-t.cpp b/test/tap/tests/test_ffto_pgsql_large_resultsets-t.cpp index 0e4917e54..925c01f00 100644 --- a/test/tap/tests/test_ffto_pgsql_large_resultsets-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_large_resultsets-t.cpp @@ -97,6 +97,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); @@ -172,7 +173,7 @@ int main(int argc, char** argv) { /* Configure PgSQL user with fast_forward=1 */ { - char escaped_user[256], escaped_pass[256]; + char escaped_user[2 * strlen(cl.pgsql_root_username) + 1]; char escaped_pass[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, escaped_user, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, escaped_pass, cl.pgsql_root_password, diff --git a/test/tap/tests/test_ffto_pgsql_mixed_protocol-t.cpp b/test/tap/tests/test_ffto_pgsql_mixed_protocol-t.cpp index 5d265dd51..e8ca48741 100644 --- a/test/tap/tests/test_ffto_pgsql_mixed_protocol-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_mixed_protocol-t.cpp @@ -81,6 +81,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); res = NULL; @@ -137,7 +138,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char eu[256], ep[256]; + char eu[2 * strlen(cl.pgsql_root_username) + 1]; char ep[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, eu, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, ep, cl.pgsql_root_password, strlen(cl.pgsql_root_password)); char uq[1024]; diff --git a/test/tap/tests/test_ffto_pgsql_pipeline-t.cpp b/test/tap/tests/test_ffto_pgsql_pipeline-t.cpp index dcd9babf8..844686597 100644 --- a/test/tap/tests/test_ffto_pgsql_pipeline-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_pipeline-t.cpp @@ -68,6 +68,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); res = NULL; @@ -124,7 +125,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char eu[256], ep[256]; + char eu[2 * strlen(cl.pgsql_root_username) + 1]; char ep[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, eu, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, ep, cl.pgsql_root_password, strlen(cl.pgsql_root_password)); char uq[1024]; diff --git a/test/tap/tests/test_ffto_pgsql_stmt_portal-t.cpp b/test/tap/tests/test_ffto_pgsql_stmt_portal-t.cpp index 31d30c735..3cbcbda38 100644 --- a/test/tap/tests/test_ffto_pgsql_stmt_portal-t.cpp +++ b/test/tap/tests/test_ffto_pgsql_stmt_portal-t.cpp @@ -68,6 +68,7 @@ void verify_pg_digest(MYSQL* admin, const char* template_text, int expected_coun int rc = run_q(admin, query); if (rc != 0) { usleep(100000); continue; } res = mysql_store_result(admin); + if (!res) { usleep(100000); continue; } row = mysql_fetch_row(res); if (row) break; mysql_free_result(res); res = NULL; @@ -123,7 +124,7 @@ int main(int argc, char** argv) { MYSQL_QUERY(admin, "LOAD PGSQL VARIABLES TO RUNTIME"); { - char eu[256], ep[256]; + char eu[2 * strlen(cl.pgsql_root_username) + 1]; char ep[2 * strlen(cl.pgsql_root_password) + 1]; mysql_real_escape_string(admin, eu, cl.pgsql_root_username, strlen(cl.pgsql_root_username)); mysql_real_escape_string(admin, ep, cl.pgsql_root_password, strlen(cl.pgsql_root_password)); char uq[1024];