From 6cf61aa5edb4f773834a5843ec13ba0496180647 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 15 Feb 2026 12:42:10 +0000 Subject: [PATCH 1/8] Log bound parameter values for prepared statements Hook mysqlnd stmt execute() to capture bound param values at execution time. Previously only the query template with ? placeholders was logged at prepare() time. C extension (PHP 7.0+): - Hook execute(): read stmt->data->param_bind zvals, build JSON array - Modify prepare(): store query template in HashTable, defer logging - Hook dtor(): clean up stored template when stmt is destroyed - Add profiler_log_query_with_params() for params-aware logging - JSONL output: new "params" field e.g. ["123","active",null] - Raw log output: "params: [...]" line after query PHP 5.x retains existing behavior (template-only at prepare time). JetBrains plugin: - QueryEntry: add params field, boundQuery computed property - QueryDetailPanel: show "Bound Parameters" section with ?N = value - SQL area shows interpolated query when params are available https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- ext/mariadb_profiler/mariadb_profiler.c | 13 ++ ext/mariadb_profiler/php_mariadb_profiler.h | 9 +- .../php_mariadb_profiler_compat.h | 12 ++ ext/mariadb_profiler/profiler_log.c | 50 ++++- .../profiler_mysqlnd_plugin.c | 195 +++++++++++++++++- .../plugin/model/QueryEntry.kt | 16 ++ .../plugin/ui/panel/QueryDetailPanel.kt | 44 +++- 7 files changed, 320 insertions(+), 19 deletions(-) diff --git a/ext/mariadb_profiler/mariadb_profiler.c b/ext/mariadb_profiler/mariadb_profiler.c index 95ac35b..476f561 100644 --- a/ext/mariadb_profiler/mariadb_profiler.c +++ b/ext/mariadb_profiler/mariadb_profiler.c @@ -175,6 +175,11 @@ PHP_RINIT_FUNCTION(mariadb_profiler) profiler_ensure_log_dir(TSRMLS_C); /* Load active jobs at request start */ profiler_job_refresh_active_jobs(); +#if PHP_VERSION_ID >= 70000 + /* Initialize prepared statement query template storage */ + ALLOC_HASHTABLE(PROFILER_G(stmt_queries)); + zend_hash_init(PROFILER_G(stmt_queries), 16, NULL, ZVAL_PTR_DTOR, 0); +#endif } return SUCCESS; @@ -187,6 +192,14 @@ PHP_RSHUTDOWN_FUNCTION(mariadb_profiler) if (PROFILER_G(enabled)) { profiler_tag_clear_all(); profiler_job_free_active_jobs(); +#if PHP_VERSION_ID >= 70000 + /* Free prepared statement query template storage */ + if (PROFILER_G(stmt_queries)) { + zend_hash_destroy(PROFILER_G(stmt_queries)); + FREE_HASHTABLE(PROFILER_G(stmt_queries)); + PROFILER_G(stmt_queries) = NULL; + } +#endif } return SUCCESS; } diff --git a/ext/mariadb_profiler/php_mariadb_profiler.h b/ext/mariadb_profiler/php_mariadb_profiler.h index caaeaec..a74bcc9 100644 --- a/ext/mariadb_profiler/php_mariadb_profiler.h +++ b/ext/mariadb_profiler/php_mariadb_profiler.h @@ -62,6 +62,10 @@ ZEND_BEGIN_MODULE_GLOBALS(mariadb_profiler) int tag_depth; /* Trace settings */ zend_long trace_depth; /* 0=disabled, N=capture N frames */ +#if PHP_VERSION_ID >= 70000 + /* Prepared statement query template storage (PHP 7.0+) */ + HashTable *stmt_queries; /* stmt ptr -> query template string */ +#endif ZEND_END_MODULE_GLOBALS(mariadb_profiler) /* Globals accessor: extern declaration for use across compilation units */ @@ -97,8 +101,11 @@ char **profiler_job_get_active_list(int *count); /* Logging */ void profiler_log_query(const char *query, size_t query_len); +void profiler_log_query_with_params(const char *query, size_t query_len, + const char *params_json); void profiler_log_raw(const char *job_key, const char *query, size_t query_len, - const char *tag, const char *trace_json); + const char *tag, const char *trace_json, + const char *params_json); void profiler_log_init(void); void profiler_log_shutdown(void); diff --git a/ext/mariadb_profiler/php_mariadb_profiler_compat.h b/ext/mariadb_profiler/php_mariadb_profiler_compat.h index 05fcfc5..cbcbab8 100644 --- a/ext/mariadb_profiler/php_mariadb_profiler_compat.h +++ b/ext/mariadb_profiler/php_mariadb_profiler_compat.h @@ -179,4 +179,16 @@ typedef long zend_long; zend_hash_str_find((ht), (key), (key_len)) #endif +/* + * ---- bool type compatibility for mysqlnd stmt dtor ---- + * + * PHP 8.0+: uses C99 bool + * PHP 7.x: uses zend_bool (unsigned char) + */ +#if PHP_VERSION_ID >= 80000 +# define PROFILER_BOOL_T bool +#else +# define PROFILER_BOOL_T zend_bool +#endif + #endif /* PHP_MARIADB_PROFILER_COMPAT_H */ diff --git a/ext/mariadb_profiler/profiler_log.c b/ext/mariadb_profiler/profiler_log.c index f9502f7..67b59e7 100644 --- a/ext/mariadb_profiler/profiler_log.c +++ b/ext/mariadb_profiler/profiler_log.c @@ -108,9 +108,10 @@ static double profiler_log_get_microtime(void) /* {{{ profiler_log_raw * Write raw query to job's raw log file. - * tag and trace_json may be NULL. */ + * tag, trace_json, and params_json may be NULL. */ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, - const char *tag, const char *trace_json) + const char *tag, const char *trace_json, + const char *params_json) { char *filepath; FILE *fp; @@ -137,6 +138,11 @@ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, fprintf(fp, "[%s] %.*s\n", timestamp, (int)query_len, query); } + /* Append bound parameter values if present */ + if (params_json && params_json[0] == '[' && params_json[1] != ']') { + fprintf(fp, " params: %s\n", params_json); + } + /* Append trace lines (indented with arrow prefix) */ if (trace_json && trace_json[0] == '[' && trace_json[1] != ']') { /* @@ -196,10 +202,11 @@ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, /* {{{ profiler_log_jsonl * Write JSON line to job's parsed log file. - * tag and trace_json may be NULL. + * tag, trace_json, and params_json may be NULL. * SQL parsing (table/column extraction) is done by the CLI tool. */ static void profiler_log_jsonl(const char *job_key, const char *query, size_t query_len, - const char *tag, const char *trace_json) + const char *tag, const char *trace_json, + const char *params_json) { char *filepath; FILE *fp; @@ -227,13 +234,18 @@ static void profiler_log_jsonl(const char *job_key, const char *query, size_t qu } ts = profiler_log_get_microtime(); - /* Build JSON line with optional tag and trace fields */ + /* Build JSON line with optional tag, params, and trace fields */ fprintf(fp, "{\"k\":\"%s\",\"q\":\"%s\"", escaped_key, escaped_query); if (escaped_tag) { fprintf(fp, ",\"tag\":\"%s\"", escaped_tag); } + /* params_json is already a valid JSON array string e.g. ["123","active",null] */ + if (params_json) { + fprintf(fp, ",\"params\":%s", params_json); + } + if (trace_json) { /* trace_json is already a valid JSON array string */ fprintf(fp, ",\"trace\":%s", trace_json); @@ -252,10 +264,11 @@ static void profiler_log_jsonl(const char *job_key, const char *query, size_t qu } /* }}} */ -/* {{{ profiler_log_query - * Main entry point: log a query to all active jobs. +/* {{{ profiler_log_query_internal + * Internal: log a query to all active jobs with optional params. * Captures the current context tag and PHP trace once, shared across all jobs. */ -void profiler_log_query(const char *query, size_t query_len) +static void profiler_log_query_internal(const char *query, size_t query_len, + const char *params_json) { char **jobs; int job_count; @@ -276,11 +289,11 @@ void profiler_log_query(const char *query, size_t query_len) for (i = 0; i < job_count; i++) { /* Write JSONL entry */ - profiler_log_jsonl(jobs[i], query, query_len, tag, trace_json); + profiler_log_jsonl(jobs[i], query, query_len, tag, trace_json, params_json); /* Write raw log if enabled */ if (PROFILER_G(raw_log)) { - profiler_log_raw(jobs[i], query, query_len, tag, trace_json); + profiler_log_raw(jobs[i], query, query_len, tag, trace_json, params_json); } } @@ -290,6 +303,23 @@ void profiler_log_query(const char *query, size_t query_len) } /* }}} */ +/* {{{ profiler_log_query + * Main entry point: log a query (without params) to all active jobs. */ +void profiler_log_query(const char *query, size_t query_len) +{ + profiler_log_query_internal(query, query_len, NULL); +} +/* }}} */ + +/* {{{ profiler_log_query_with_params + * Log a prepared statement query with bound parameter values to all active jobs. */ +void profiler_log_query_with_params(const char *query, size_t query_len, + const char *params_json) +{ + profiler_log_query_internal(query, query_len, params_json); +} +/* }}} */ + /* {{{ profiler_log_init */ void profiler_log_init(void) { diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index 93f85ec..7f51ed5 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -13,6 +13,7 @@ #include "php.h" #include "php_mariadb_profiler.h" +#include "profiler_log.h" /* * mysqlnd internal API changed across PHP versions: @@ -124,21 +125,202 @@ MYSQLND_METHOD(profiler_conn, send_query)( /* }}} */ /* {{{ profiler_stmt_prepare_hook - * Intercepts prepared statements at prepare time to log the query template */ + * Intercepts prepared statements at prepare time. + * PHP 7.0+: stores query template for later use at execute() time. + * PHP 5.x: logs template immediately (no param capture support). */ static enum_func_status MYSQLND_METHOD(profiler_stmt, prepare)( MYSQLND_STMT * const stmt, const char *query, PROFILER_QUERY_LEN_T query_len TSRMLS_DC) { - /* Log prepared statement query template */ - if (PROFILER_G(enabled) && profiler_job_is_any_active()) { + enum_func_status result; + + result = orig_stmt_methods->prepare(stmt, query, query_len TSRMLS_CC); + +#if PHP_VERSION_ID >= 70000 + /* Store query template on success - will be logged at execute() with bound params */ + if (result == PASS && PROFILER_G(enabled) && PROFILER_G(stmt_queries)) { + zval zv; + ZVAL_STRINGL(&zv, query, query_len); + zend_hash_index_update( + PROFILER_G(stmt_queries), + (zend_ulong)(uintptr_t)stmt, + &zv + ); + } +#else + /* PHP 5.x: log template at prepare time (no param support) */ + if (result == PASS && PROFILER_G(enabled) && profiler_job_is_any_active()) { profiler_log_query(query, query_len); } - return orig_stmt_methods->prepare(stmt, query, query_len TSRMLS_CC); +#endif + + return result; } /* }}} */ +#if PHP_VERSION_ID >= 70000 + +/* {{{ profiler_build_params_json + * Build JSON array string from stmt's bound parameter values. + * Returns emalloc'd string or NULL if no params. Caller must efree. */ +static char *profiler_build_params_json(MYSQLND_STMT * const stmt) +{ + MYSQLND_STMT_DATA *data = stmt->data; + unsigned int i; + size_t buf_size, pos; + char *buf; + + if (!data || !data->param_bind || data->param_count == 0) { + return NULL; + } + + buf_size = 256; + buf = (char *)emalloc(buf_size); + pos = 0; + + buf[pos++] = '['; + + for (i = 0; i < data->param_count; i++) { + zval *zv = &data->param_bind[i].zv; + + if (i > 0) { + buf[pos++] = ','; + } + + /* Ensure space for this param (grow if needed) */ + if (pos + 128 > buf_size) { + buf_size *= 2; + buf = (char *)erealloc(buf, buf_size); + } + + /* Dereference if reference (bind_param uses references) */ + ZVAL_DEREF(zv); + + switch (Z_TYPE_P(zv)) { + case IS_NULL: + memcpy(buf + pos, "null", 4); + pos += 4; + break; + + case IS_LONG: + pos += snprintf(buf + pos, buf_size - pos, "\"%ld\"", + (long)Z_LVAL_P(zv)); + break; + + case IS_DOUBLE: + pos += snprintf(buf + pos, buf_size - pos, "\"%g\"", + Z_DVAL_P(zv)); + break; + +#if PHP_VERSION_ID >= 70000 + case IS_TRUE: + memcpy(buf + pos, "\"1\"", 3); + pos += 3; + break; + + case IS_FALSE: + memcpy(buf + pos, "\"\"", 2); + pos += 2; + break; +#endif + + case IS_STRING: { + char *escaped = profiler_log_escape_json_string( + Z_STRVAL_P(zv), Z_STRLEN_P(zv)); + size_t esc_len = strlen(escaped); + + /* Grow buffer for escaped string */ + if (pos + esc_len + 4 > buf_size) { + buf_size = pos + esc_len + 256; + buf = (char *)erealloc(buf, buf_size); + } + + buf[pos++] = '"'; + memcpy(buf + pos, escaped, esc_len); + pos += esc_len; + buf[pos++] = '"'; + efree(escaped); + break; + } + + default: { + /* Convert unknown types to string */ + zend_string *str = zval_get_string(zv); + char *escaped = profiler_log_escape_json_string( + ZSTR_VAL(str), ZSTR_LEN(str)); + size_t esc_len = strlen(escaped); + + if (pos + esc_len + 4 > buf_size) { + buf_size = pos + esc_len + 256; + buf = (char *)erealloc(buf, buf_size); + } + + buf[pos++] = '"'; + memcpy(buf + pos, escaped, esc_len); + pos += esc_len; + buf[pos++] = '"'; + efree(escaped); + zend_string_release(str); + break; + } + } + } + + buf[pos++] = ']'; + buf[pos] = '\0'; + + return buf; +} +/* }}} */ + +/* {{{ profiler_stmt_execute_hook + * Intercepts prepared statement execution to log query with bound params. */ +static enum_func_status +MYSQLND_METHOD(profiler_stmt, execute)( + MYSQLND_STMT * const stmt) +{ + if (PROFILER_G(enabled) && profiler_job_is_any_active() && PROFILER_G(stmt_queries)) { + zval *entry = zend_hash_index_find( + PROFILER_G(stmt_queries), + (zend_ulong)(uintptr_t)stmt + ); + if (entry && Z_TYPE_P(entry) == IS_STRING) { + char *params_json = profiler_build_params_json(stmt); + profiler_log_query_with_params( + Z_STRVAL_P(entry), Z_STRLEN_P(entry), params_json + ); + if (params_json) { + efree(params_json); + } + } + } + + return orig_stmt_methods->execute(stmt); +} +/* }}} */ + +/* {{{ profiler_stmt_dtor_hook + * Clean up stored query template when statement is destroyed. */ +static enum_func_status +MYSQLND_METHOD(profiler_stmt, dtor)( + MYSQLND_STMT * const stmt, + PROFILER_BOOL_T implicit) +{ + if (PROFILER_G(stmt_queries)) { + zend_hash_index_del( + PROFILER_G(stmt_queries), + (zend_ulong)(uintptr_t)stmt + ); + } + + return orig_stmt_methods->dtor(stmt, implicit); +} +/* }}} */ + +#endif /* PHP_VERSION_ID >= 70000 */ + /* {{{ mariadb_profiler_mysqlnd_plugin_register */ void mariadb_profiler_mysqlnd_plugin_register(void) { @@ -180,5 +362,10 @@ void mariadb_profiler_mysqlnd_plugin_register(void) } stmt_methods->prepare = MYSQLND_METHOD(profiler_stmt, prepare); + +#if PHP_VERSION_ID >= 70000 + stmt_methods->execute = MYSQLND_METHOD(profiler_stmt, execute); + stmt_methods->dtor = MYSQLND_METHOD(profiler_stmt, dtor); +#endif } /* }}} */ diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index 2b61fae..e447d3a 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -12,8 +12,24 @@ data class QueryEntry( @SerialName("k") val jobKey: String = "", val tag: String? = null, + val params: List = emptyList(), val trace: List = emptyList() ) { + /** Whether this query has bound parameters (prepared statement) */ + val hasParams: Boolean + get() = params.isNotEmpty() + + /** Query with ? placeholders replaced by bound values */ + val boundQuery: String? + get() { + if (params.isEmpty()) return null + var result = query + for (param in params) { + val replacement = if (param == null) "NULL" else "'$param'" + result = result.replaceFirst("?", replacement) + } + return result + } /** Tag as list for UI display compatibility */ val tags: List get() = if (tag != null) listOf(tag) else emptyList() diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryDetailPanel.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryDetailPanel.kt index 14f82f1..bf53cae 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryDetailPanel.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/ui/panel/QueryDetailPanel.kt @@ -29,6 +29,21 @@ class QueryDetailPanel(private val project: Project) : JPanel(BorderLayout()) { border = BorderFactory.createEmptyBorder(8, 8, 8, 8) } + private val paramsArea = JTextArea().apply { + isEditable = false + font = Font("JetBrains Mono", Font.PLAIN, 12).let { f -> + if (f.family == "JetBrains Mono") f else Font(Font.MONOSPACED, Font.PLAIN, 12) + } + lineWrap = true + wrapStyleWord = true + border = BorderFactory.createEmptyBorder(4, 8, 4, 8) + foreground = JBColor(0x6A1B9A, 0xCE93D8) + } + private val paramsPanel = JPanel(BorderLayout()).apply { + border = BorderFactory.createTitledBorder("Bound Parameters") + add(JBScrollPane(paramsArea).apply { preferredSize = Dimension(0, 60) }) + } + private val tablesLabel = JBLabel() private val tagsLabel = JBLabel() private val timestampLabel = JBLabel() @@ -87,10 +102,17 @@ class QueryDetailPanel(private val project: Project) : JPanel(BorderLayout()) { add(JBScrollPane(backtracePanel).apply { preferredSize = Dimension(0, 200) }) } + // Stack SQL + params + meta vertically, then backtrace at bottom + val topPanel = JPanel().apply { + layout = BoxLayout(this, BoxLayout.Y_AXIS) + add(sqlPanel) + add(paramsPanel) + add(metaPanel) + } + contentPanel.apply { - add(sqlPanel, BorderLayout.NORTH) - add(metaPanel, BorderLayout.CENTER) - add(btPanel, BorderLayout.SOUTH) + add(topPanel, BorderLayout.NORTH) + add(btPanel, BorderLayout.CENTER) } cardPanel.add(emptyLabel, "empty") @@ -105,8 +127,22 @@ class QueryDetailPanel(private val project: Project) : JPanel(BorderLayout()) { return } - sqlArea.text = entry.query + // Show bound query (with params interpolated) if available, otherwise template + sqlArea.text = entry.boundQuery ?: entry.query sqlArea.caretPosition = 0 + + // Show params panel only for prepared statements with bound values + if (entry.hasParams) { + val paramLines = entry.params.mapIndexed { i, v -> + " ?${i + 1} = ${v ?: "NULL"}" + } + paramsArea.text = paramLines.joinToString("\n") + paramsArea.caretPosition = 0 + paramsPanel.isVisible = true + } else { + paramsPanel.isVisible = false + } + timestampLabel.text = entry.formattedTimestamp tablesLabel.text = entry.tables.joinToString(", ").ifEmpty { "-" } tagsLabel.text = entry.tags.joinToString(", ").ifEmpty { "-" } From 42861741e81b192c1f12588d191e2d6395211504 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 15 Feb 2026 12:58:07 +0000 Subject: [PATCH 2/8] Harden param capture and fix boundQuery placeholder substitution C extension: - Guard stmt->data->param_bind/param_count access with PROFILER_MYSQLND_PARAM_ACCESS_SAFE compile-time check (MYSQLND_VERSION_ID 70000..80499). Unknown future mysqlnd versions gracefully degrade to NULL (no params) instead of risking ABI mismatch crashes. - Remove redundant inner #if PHP_VERSION_ID >= 70000 around IS_TRUE/IS_FALSE cases (already inside outer PHP 7.0+ block). JetBrains plugin (QueryEntry.boundQuery): - Scan query character-by-character tracking single-quoted SQL string literal state; only substitute ? outside literals. - Handle '' (doubled quote escape) inside literals correctly. - Escape single quotes in param values by doubling them (O'Brien -> 'O''Brien'). https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- .../profiler_mysqlnd_plugin.c | 17 ++++- .../plugin/model/QueryEntry.kt | 64 +++++++++++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index 7f51ed5..1f5f114 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -162,11 +162,21 @@ MYSQLND_METHOD(profiler_stmt, prepare)( #if PHP_VERSION_ID >= 70000 +/* + * MYSQLND_VERSION_ID equals PHP_VERSION_ID. The internal layout of + * st_mysqlnd_stmt_data (fields param_bind, param_count) has been stable + * from PHP 7.0 through 8.4. Guard direct access so that an unknown + * future mysqlnd silently degrades to "no params" instead of crashing. + */ +#define PROFILER_MYSQLND_PARAM_ACCESS_SAFE \ + (MYSQLND_VERSION_ID >= 70000 && MYSQLND_VERSION_ID < 80500) + /* {{{ profiler_build_params_json * Build JSON array string from stmt's bound parameter values. * Returns emalloc'd string or NULL if no params. Caller must efree. */ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) { +#if PROFILER_MYSQLND_PARAM_ACCESS_SAFE MYSQLND_STMT_DATA *data = stmt->data; unsigned int i; size_t buf_size, pos; @@ -214,7 +224,6 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) Z_DVAL_P(zv)); break; -#if PHP_VERSION_ID >= 70000 case IS_TRUE: memcpy(buf + pos, "\"1\"", 3); pos += 3; @@ -224,7 +233,6 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) memcpy(buf + pos, "\"\"", 2); pos += 2; break; -#endif case IS_STRING: { char *escaped = profiler_log_escape_json_string( @@ -272,6 +280,11 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) buf[pos] = '\0'; return buf; +#else + /* Unknown mysqlnd version – skip param capture to avoid ABI issues */ + (void)stmt; + return NULL; +#endif /* PROFILER_MYSQLND_PARAM_ACCESS_SAFE */ } /* }}} */ diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index e447d3a..1d2184e 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -19,16 +19,68 @@ data class QueryEntry( val hasParams: Boolean get() = params.isNotEmpty() - /** Query with ? placeholders replaced by bound values */ + /** + * Query with `?` placeholders replaced by bound values. + * + * Only replaces `?` characters that appear **outside** single-quoted SQL + * string literals. Doubled single-quotes (`''`) inside a literal are + * treated as an escaped quote and do not toggle the "inside string" state. + * + * Param values are wrapped in single quotes with internal single-quotes + * doubled (`O'Brien` → `'O''Brien'`); NULL params are emitted bare. + */ val boundQuery: String? get() { if (params.isEmpty()) return null - var result = query - for (param in params) { - val replacement = if (param == null) "NULL" else "'$param'" - result = result.replaceFirst("?", replacement) + + val sb = StringBuilder(query.length + params.size * 8) + var paramIdx = 0 + var inString = false + var i = 0 + + while (i < query.length) { + val ch = query[i] + + if (inString) { + if (ch == '\'') { + /* '' inside a literal is an escaped quote – stay in string */ + if (i + 1 < query.length && query[i + 1] == '\'') { + sb.append("''") + i += 2 + continue + } + /* closing quote */ + inString = false + sb.append(ch) + } else { + sb.append(ch) + } + } else { + when (ch) { + '\'' -> { + inString = true + sb.append(ch) + } + '?' -> { + if (paramIdx < params.size) { + val v = params[paramIdx++] + if (v == null) { + sb.append("NULL") + } else { + sb.append('\'') + sb.append(v.replace("'", "''")) + sb.append('\'') + } + } else { + sb.append(ch) // more ?'s than params – keep as-is + } + } + else -> sb.append(ch) + } + } + i++ } - return result + return sb.toString() } /** Tag as list for UI display compatibility */ val tags: List From db81d24153d5e707d04dcc172d24771dc3b8b816 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 13:27:07 +0000 Subject: [PATCH 3/8] Initial plan From a578acc3c2fe889c49493631558b6f596275a268 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 15 Feb 2026 13:30:58 +0000 Subject: [PATCH 4/8] Add backslash escape handling to boundQuery SQL parser Co-authored-by: 39ff <7544687+39ff@users.noreply.github.com> --- .../plugin/model/QueryEntry.kt | 17 ++- .../plugin/model/QueryEntryTest.kt | 106 ++++++++++++++++++ 2 files changed, 120 insertions(+), 3 deletions(-) diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index 1d2184e..211955a 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -23,8 +23,9 @@ data class QueryEntry( * Query with `?` placeholders replaced by bound values. * * Only replaces `?` characters that appear **outside** single-quoted SQL - * string literals. Doubled single-quotes (`''`) inside a literal are - * treated as an escaped quote and do not toggle the "inside string" state. + * string literals. Handles both doubled single-quotes (`''`) and backslash + * escapes (`\'`, `\\`, etc.) inside literals, matching MySQL/MariaDB default + * behavior (when `NO_BACKSLASH_ESCAPES` is not set). * * Param values are wrapped in single quotes with internal single-quotes * doubled (`O'Brien` → `'O''Brien'`); NULL params are emitted bare. @@ -42,7 +43,17 @@ data class QueryEntry( val ch = query[i] if (inString) { - if (ch == '\'') { + if (ch == '\\') { + /* backslash escape: \' \\ etc. – copy both chars, stay in string */ + if (i + 1 < query.length) { + sb.append(ch) + sb.append(query[i + 1]) + i += 2 + continue + } + /* trailing backslash – just append it */ + sb.append(ch) + } else if (ch == '\'') { /* '' inside a literal is an escaped quote – stay in string */ if (i + 1 < query.length && query[i + 1] == '\'') { sb.append("''") diff --git a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt index a7f7a1a..dba0d33 100644 --- a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt +++ b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt @@ -88,4 +88,110 @@ class QueryEntryTest { assertEquals(42, entry.backtrace[0].line) assertEquals("UserController.php:42", entry.sourceFile) } + + @Test + fun `boundQuery replaces placeholders with params`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = ? AND age = ?", + params = listOf("John", "25") + ) + assertEquals("SELECT * FROM users WHERE name = 'John' AND age = '25'", entry.boundQuery) + } + + @Test + fun `boundQuery handles NULL params`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = ? AND email = ?", + params = listOf("John", null) + ) + assertEquals("SELECT * FROM users WHERE name = 'John' AND email = NULL", entry.boundQuery) + } + + @Test + fun `boundQuery escapes single quotes in params`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = ?", + params = listOf("O'Brien") + ) + assertEquals("SELECT * FROM users WHERE name = 'O''Brien'", entry.boundQuery) + } + + @Test + fun `boundQuery does not replace placeholders inside single-quoted strings`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = 'test?' AND age = ?", + params = listOf("25") + ) + assertEquals("SELECT * FROM users WHERE name = 'test?' AND age = '25'", entry.boundQuery) + } + + @Test + fun `boundQuery handles doubled single-quotes inside strings`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = 'O''Brien' AND age = ?", + params = listOf("25") + ) + assertEquals("SELECT * FROM users WHERE name = 'O''Brien' AND age = '25'", entry.boundQuery) + } + + @Test + fun `boundQuery handles backslash-escaped single quotes`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = 'O\\'Brien' AND age = ?", + params = listOf("25") + ) + assertEquals("SELECT * FROM users WHERE name = 'O\\'Brien' AND age = '25'", entry.boundQuery) + } + + @Test + fun `boundQuery handles backslash-escaped backslashes`() { + val entry = QueryEntry( + query = "SELECT * FROM paths WHERE path = 'C:\\\\Users\\\\test' AND id = ?", + params = listOf("1") + ) + assertEquals("SELECT * FROM paths WHERE path = 'C:\\\\Users\\\\test' AND id = '1'", entry.boundQuery) + } + + @Test + fun `boundQuery handles mixed escape sequences`() { + val entry = QueryEntry( + query = "SELECT * FROM data WHERE value = 'test\\nline' AND name = ?", + params = listOf("John") + ) + assertEquals("SELECT * FROM data WHERE value = 'test\\nline' AND name = 'John'", entry.boundQuery) + } + + @Test + fun `boundQuery handles placeholder at end of backslash-escaped string`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = 'can\\'t' AND status = ?", + params = listOf("active") + ) + assertEquals("SELECT * FROM users WHERE name = 'can\\'t' AND status = 'active'", entry.boundQuery) + } + + @Test + fun `boundQuery returns null when no params`() { + val entry = QueryEntry(query = "SELECT * FROM users") + assertEquals(null, entry.boundQuery) + } + + @Test + fun `boundQuery handles more placeholders than params`() { + val entry = QueryEntry( + query = "SELECT * FROM users WHERE name = ? AND age = ? AND email = ?", + params = listOf("John", "25") + ) + // Should replace first two, leave third as-is + assertEquals("SELECT * FROM users WHERE name = 'John' AND age = '25' AND email = ?", entry.boundQuery) + } + + @Test + fun `boundQuery handles complex nested strings`() { + val entry = QueryEntry( + query = "SELECT * FROM logs WHERE msg = 'User said \\'hello\\'' AND id = ?", + params = listOf("123") + ) + assertEquals("SELECT * FROM logs WHERE msg = 'User said \\'hello\\'' AND id = '123'", entry.boundQuery) + } } From 80d47ca1f273d307a02233a316daba8877677fa3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 15 Feb 2026 14:36:32 +0000 Subject: [PATCH 5/8] Fix SQL parser robustness and snprintf safety from PR review - Fix snprintf truncation in profiler_build_params_json: check return value and grow+retry when output was truncated, handle negative return values (encoding errors) - Enhance boundQuery SQL parser to handle double-quoted string literals, backtick-quoted identifiers, line comments (--), and block comments (/* */) so placeholders inside these contexts are not replaced - Escape backslashes in parameter values before interpolation to produce accurate SQL text for debugging - Document MYSQLND_VERSION_ID upper bound as a defensive estimate requiring verification when PHP 8.5 is released - Add comprehensive unit tests for boundQuery covering: no params, single/multiple placeholders, NULL params, param count mismatches, quoted strings, escaped quotes, backslash escaping, double-quoted strings, backtick identifiers, line/block comments, unclosed comments https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- .../profiler_mysqlnd_plugin.c | 38 +++- .../plugin/model/QueryEntry.kt | 133 ++++++++----- .../plugin/model/QueryEntryTest.kt | 175 ++++++++++++++++-- 3 files changed, 280 insertions(+), 66 deletions(-) diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index 1f5f114..17b1020 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -167,6 +167,10 @@ MYSQLND_METHOD(profiler_stmt, prepare)( * st_mysqlnd_stmt_data (fields param_bind, param_count) has been stable * from PHP 7.0 through 8.4. Guard direct access so that an unknown * future mysqlnd silently degrades to "no params" instead of crashing. + * + * NOTE: The upper bound (80500) is a defensive estimate – it should be + * updated once PHP 8.5 is released and its mysqlnd ABI has been verified. + * If the ABI remains unchanged, simply bump the upper bound. */ #define PROFILER_MYSQLND_PARAM_ACCESS_SAFE \ (MYSQLND_VERSION_ID >= 70000 && MYSQLND_VERSION_ID < 80500) @@ -214,15 +218,41 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) pos += 4; break; - case IS_LONG: - pos += snprintf(buf + pos, buf_size - pos, "\"%ld\"", + case IS_LONG: { + int written = snprintf(buf + pos, buf_size - pos, "\"%ld\"", (long)Z_LVAL_P(zv)); + if (written < 0) { + break; /* encoding error */ + } + if ((size_t)written >= buf_size - pos) { + /* Truncated – grow and retry */ + buf_size = pos + (size_t)written + 64; + buf = (char *)erealloc(buf, buf_size); + pos += snprintf(buf + pos, buf_size - pos, "\"%ld\"", + (long)Z_LVAL_P(zv)); + } else { + pos += (size_t)written; + } break; + } - case IS_DOUBLE: - pos += snprintf(buf + pos, buf_size - pos, "\"%g\"", + case IS_DOUBLE: { + int written = snprintf(buf + pos, buf_size - pos, "\"%g\"", Z_DVAL_P(zv)); + if (written < 0) { + break; /* encoding error */ + } + if ((size_t)written >= buf_size - pos) { + /* Truncated – grow and retry */ + buf_size = pos + (size_t)written + 64; + buf = (char *)erealloc(buf, buf_size); + pos += snprintf(buf + pos, buf_size - pos, "\"%g\"", + Z_DVAL_P(zv)); + } else { + pos += (size_t)written; + } break; + } case IS_TRUE: memcpy(buf + pos, "\"1\"", 3); diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index 211955a..1cea401 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -22,13 +22,18 @@ data class QueryEntry( /** * Query with `?` placeholders replaced by bound values. * - * Only replaces `?` characters that appear **outside** single-quoted SQL - * string literals. Handles both doubled single-quotes (`''`) and backslash - * escapes (`\'`, `\\`, etc.) inside literals, matching MySQL/MariaDB default - * behavior (when `NO_BACKSLASH_ESCAPES` is not set). + * Replaces `?` characters that appear **outside** quoted contexts and + * SQL comments. The parser recognises: + * - Single-quoted string literals (`'...'`) with `''` and backslash + * escapes (`\'`, `\\`, etc.), matching MySQL/MariaDB default behavior + * (when `NO_BACKSLASH_ESCAPES` is not set) + * - Double-quoted string literals (`"..."`) with `""` as escaped quote + * - Backtick-quoted identifiers (`` `...` ``) + * - Line comments (`-- ...`) + * - Block comments (`/* ... */`) * - * Param values are wrapped in single quotes with internal single-quotes - * doubled (`O'Brien` → `'O''Brien'`); NULL params are emitted bare. + * Param values are wrapped in single quotes with internal backslashes + * doubled and single-quotes doubled; NULL params are emitted bare. */ val boundQuery: String? get() { @@ -36,59 +41,93 @@ data class QueryEntry( val sb = StringBuilder(query.length + params.size * 8) var paramIdx = 0 - var inString = false var i = 0 while (i < query.length) { val ch = query[i] - if (inString) { - if (ch == '\\') { - /* backslash escape: \' \\ etc. – copy both chars, stay in string */ - if (i + 1 < query.length) { - sb.append(ch) - sb.append(query[i + 1]) - i += 2 - continue - } - /* trailing backslash – just append it */ - sb.append(ch) - } else if (ch == '\'') { - /* '' inside a literal is an escaped quote – stay in string */ - if (i + 1 < query.length && query[i + 1] == '\'') { - sb.append("''") - i += 2 - continue - } - /* closing quote */ - inString = false - sb.append(ch) + // -- line comment: copy through to end of line + if (ch == '-' && i + 1 < query.length && query[i + 1] == '-') { + val eol = query.indexOf('\n', i) + if (eol == -1) { + sb.append(query, i, query.length) + i = query.length } else { - sb.append(ch) + sb.append(query, i, eol + 1) + i = eol + 1 } - } else { - when (ch) { - '\'' -> { - inString = true - sb.append(ch) - } - '?' -> { - if (paramIdx < params.size) { - val v = params[paramIdx++] - if (v == null) { - sb.append("NULL") - } else { - sb.append('\'') - sb.append(v.replace("'", "''")) - sb.append('\'') - } + continue + } + + // /* block comment */: copy through to closing */ + if (ch == '/' && i + 1 < query.length && query[i + 1] == '*') { + val close = query.indexOf("*/", i + 2) + if (close == -1) { + sb.append(query, i, query.length) + i = query.length + } else { + sb.append(query, i, close + 2) + i = close + 2 + } + continue + } + + // Quoted context: single-quote, double-quote, or backtick + if (ch == '\'' || ch == '"' || ch == '`') { + val quote = ch + sb.append(ch) + i++ + while (i < query.length) { + val qch = query[i] + // Backslash escape inside single- and double-quoted strings + if (qch == '\\' && (quote == '\'' || quote == '"')) { + sb.append(qch) + if (i + 1 < query.length) { + sb.append(query[i + 1]) + i += 2 } else { - sb.append(ch) // more ?'s than params – keep as-is + i++ } + continue + } + if (qch == quote) { + // doubled-quote escape ('' / "" inside their respective literals) + if (i + 1 < query.length && query[i + 1] == quote) { + sb.append(quote) + sb.append(quote) + i += 2 + continue + } + // closing quote + sb.append(quote) + i++ + break + } + sb.append(qch) + i++ + } + continue + } + + // Parameter placeholder + if (ch == '?') { + if (paramIdx < params.size) { + val v = params[paramIdx++] + if (v == null) { + sb.append("NULL") + } else { + sb.append('\'') + sb.append(v.replace("\\", "\\\\").replace("'", "''")) + sb.append('\'') } - else -> sb.append(ch) + } else { + sb.append(ch) // more ?'s than params – keep as-is } + i++ + continue } + + sb.append(ch) i++ } return sb.toString() diff --git a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt index dba0d33..34c4e6f 100644 --- a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt +++ b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt @@ -3,6 +3,7 @@ package com.mariadbprofiler.plugin.model import kotlinx.serialization.json.Json import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertNull import kotlin.test.assertTrue class QueryEntryTest { @@ -69,6 +70,165 @@ class QueryEntryTest { assertEquals("SELECT * FROM users", entry.shortSql) } + // ---- boundQuery tests ---- + + @Test + fun `boundQuery returns null when no params`() { + val entry = QueryEntry(query = "SELECT 1") + assertNull(entry.boundQuery) + } + + @Test + fun `boundQuery replaces single placeholder`() { + val entry = QueryEntry(query = "SELECT * FROM users WHERE id = ?", params = listOf("42")) + assertEquals("SELECT * FROM users WHERE id = '42'", entry.boundQuery) + } + + @Test + fun `boundQuery replaces multiple placeholders`() { + val entry = QueryEntry( + query = "INSERT INTO t (a, b, c) VALUES (?, ?, ?)", + params = listOf("x", "y", "z") + ) + assertEquals("INSERT INTO t (a, b, c) VALUES ('x', 'y', 'z')", entry.boundQuery) + } + + @Test + fun `boundQuery handles NULL params`() { + val entry = QueryEntry( + query = "INSERT INTO t (a, b) VALUES (?, ?)", + params = listOf(null, "val") + ) + assertEquals("INSERT INTO t (a, b) VALUES (NULL, 'val')", entry.boundQuery) + } + + @Test + fun `boundQuery keeps extra placeholders when fewer params than placeholders`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE a = ? AND b = ? AND c = ?", + params = listOf("1") + ) + assertEquals("SELECT * FROM t WHERE a = '1' AND b = ? AND c = ?", entry.boundQuery) + } + + @Test + fun `boundQuery ignores extra params when more params than placeholders`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE a = ?", + params = listOf("1", "2", "3") + ) + assertEquals("SELECT * FROM t WHERE a = '1'", entry.boundQuery) + } + + @Test + fun `boundQuery does not replace placeholder inside single-quoted string`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE a = '?' AND b = ?", + params = listOf("val") + ) + assertEquals("SELECT * FROM t WHERE a = '?' AND b = 'val'", entry.boundQuery) + } + + @Test + fun `boundQuery handles escaped single quotes in SQL literal`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE a = 'it''s' AND b = ?", + params = listOf("x") + ) + assertEquals("SELECT * FROM t WHERE a = 'it''s' AND b = 'x'", entry.boundQuery) + } + + @Test + fun `boundQuery escapes single quotes in param values`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE name = ?", + params = listOf("O'Brien") + ) + assertEquals("SELECT * FROM t WHERE name = 'O''Brien'", entry.boundQuery) + } + + @Test + fun `boundQuery escapes backslashes in param values`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE path = ?", + params = listOf("C:\\tmp") + ) + assertEquals("SELECT * FROM t WHERE path = 'C:\\\\tmp'", entry.boundQuery) + } + + @Test + fun `boundQuery escapes backslash and quote together`() { + val entry = QueryEntry( + query = "SELECT * FROM t WHERE v = ?", + params = listOf("a\\nb's") + ) + assertEquals("SELECT * FROM t WHERE v = 'a\\\\nb''s'", entry.boundQuery) + } + + @Test + fun `boundQuery does not replace placeholder inside double-quoted string`() { + val entry = QueryEntry( + query = """SELECT * FROM t WHERE a = "?" AND b = ?""", + params = listOf("val") + ) + assertEquals("""SELECT * FROM t WHERE a = "?" AND b = 'val'""", entry.boundQuery) + } + + @Test + fun `boundQuery handles escaped double quotes inside double-quoted string`() { + val entry = QueryEntry( + query = """SELECT * FROM t WHERE a = "say""what" AND b = ?""", + params = listOf("x") + ) + assertEquals("""SELECT * FROM t WHERE a = "say""what" AND b = 'x'""", entry.boundQuery) + } + + @Test + fun `boundQuery does not replace placeholder inside backtick-quoted identifier`() { + val entry = QueryEntry( + query = "SELECT `?` FROM t WHERE id = ?", + params = listOf("1") + ) + assertEquals("SELECT `?` FROM t WHERE id = '1'", entry.boundQuery) + } + + @Test + fun `boundQuery skips placeholder in line comment`() { + val entry = QueryEntry( + query = "SELECT * FROM t -- where id = ?\nWHERE name = ?", + params = listOf("test") + ) + assertEquals("SELECT * FROM t -- where id = ?\nWHERE name = 'test'", entry.boundQuery) + } + + @Test + fun `boundQuery skips placeholder in block comment`() { + val entry = QueryEntry( + query = "SELECT * FROM t /* ? */ WHERE id = ?", + params = listOf("42") + ) + assertEquals("SELECT * FROM t /* ? */ WHERE id = '42'", entry.boundQuery) + } + + @Test + fun `boundQuery handles block comment with quote inside`() { + val entry = QueryEntry( + query = "SELECT * FROM t /* it's ? */ WHERE id = ?", + params = listOf("1") + ) + assertEquals("SELECT * FROM t /* it's ? */ WHERE id = '1'", entry.boundQuery) + } + + @Test + fun `boundQuery handles unclosed block comment`() { + val entry = QueryEntry( + query = "SELECT * /* unclosed ? ", + params = listOf("x") + ) + // ? is inside unclosed comment, no replacement + assertEquals("SELECT * /* unclosed ? ", entry.boundQuery) + } + @Test fun `parse entry with backtrace`() { val jsonStr = """ @@ -98,15 +258,6 @@ class QueryEntryTest { assertEquals("SELECT * FROM users WHERE name = 'John' AND age = '25'", entry.boundQuery) } - @Test - fun `boundQuery handles NULL params`() { - val entry = QueryEntry( - query = "SELECT * FROM users WHERE name = ? AND email = ?", - params = listOf("John", null) - ) - assertEquals("SELECT * FROM users WHERE name = 'John' AND email = NULL", entry.boundQuery) - } - @Test fun `boundQuery escapes single quotes in params`() { val entry = QueryEntry( @@ -170,12 +321,6 @@ class QueryEntryTest { assertEquals("SELECT * FROM users WHERE name = 'can\\'t' AND status = 'active'", entry.boundQuery) } - @Test - fun `boundQuery returns null when no params`() { - val entry = QueryEntry(query = "SELECT * FROM users") - assertEquals(null, entry.boundQuery) - } - @Test fun `boundQuery handles more placeholders than params`() { val entry = QueryEntry( From 6aea79077ec82e25b603e633dd857c8c6da5f5f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 15 Feb 2026 14:57:17 +0000 Subject: [PATCH 6/8] Add execution status to query logs and fix SQL comment parsing - Move logging after execute in all mysqlnd hooks (query, send_query, stmt_execute) so the result status is available at log time - Add "s" field ("ok"/"err") to JSONL output and [ok]/[err] to raw log - Update all log function signatures to accept status parameter - Add status field with @SerialName("s") to QueryEntry.kt - Add @SerialName annotations to params/tag/trace for consistency - Fix -- line comment detection to require trailing whitespace per MySQL/MariaDB spec (prevents misparse of expressions like 1--?) - Add # hash comment support in boundQuery placeholder substitution - Add tests for status parsing, hash comments, and -- edge cases https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- ext/mariadb_profiler/php_mariadb_profiler.h | 8 +-- ext/mariadb_profiler/profiler_log.c | 41 +++++++----- .../profiler_mysqlnd_plugin.c | 47 ++++++++++---- .../plugin/model/QueryEntry.kt | 26 +++++++- .../plugin/model/QueryEntryTest.kt | 65 +++++++++++++++++++ 5 files changed, 150 insertions(+), 37 deletions(-) diff --git a/ext/mariadb_profiler/php_mariadb_profiler.h b/ext/mariadb_profiler/php_mariadb_profiler.h index a74bcc9..ca432f5 100644 --- a/ext/mariadb_profiler/php_mariadb_profiler.h +++ b/ext/mariadb_profiler/php_mariadb_profiler.h @@ -99,13 +99,13 @@ void profiler_job_free_active_jobs(void); int profiler_job_is_any_active(void); char **profiler_job_get_active_list(int *count); -/* Logging */ -void profiler_log_query(const char *query, size_t query_len); +/* Logging – status is "ok" or "err" (NULL treated as "ok") */ +void profiler_log_query(const char *query, size_t query_len, const char *status); void profiler_log_query_with_params(const char *query, size_t query_len, - const char *params_json); + const char *params_json, const char *status); void profiler_log_raw(const char *job_key, const char *query, size_t query_len, const char *tag, const char *trace_json, - const char *params_json); + const char *params_json, const char *status); void profiler_log_init(void); void profiler_log_shutdown(void); diff --git a/ext/mariadb_profiler/profiler_log.c b/ext/mariadb_profiler/profiler_log.c index 67b59e7..bac853b 100644 --- a/ext/mariadb_profiler/profiler_log.c +++ b/ext/mariadb_profiler/profiler_log.c @@ -108,10 +108,10 @@ static double profiler_log_get_microtime(void) /* {{{ profiler_log_raw * Write raw query to job's raw log file. - * tag, trace_json, and params_json may be NULL. */ + * tag, trace_json, params_json, and status may be NULL. */ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, const char *tag, const char *trace_json, - const char *params_json) + const char *params_json, const char *status) { char *filepath; FILE *fp; @@ -133,9 +133,11 @@ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, timestamp = profiler_log_get_timestamp(); if (tag) { - fprintf(fp, "[%s] [%s] %.*s\n", timestamp, tag, (int)query_len, query); + fprintf(fp, "[%s] [%s] [%s] %.*s\n", timestamp, + status ? status : "ok", tag, (int)query_len, query); } else { - fprintf(fp, "[%s] %.*s\n", timestamp, (int)query_len, query); + fprintf(fp, "[%s] [%s] %.*s\n", timestamp, + status ? status : "ok", (int)query_len, query); } /* Append bound parameter values if present */ @@ -202,11 +204,11 @@ void profiler_log_raw(const char *job_key, const char *query, size_t query_len, /* {{{ profiler_log_jsonl * Write JSON line to job's parsed log file. - * tag, trace_json, and params_json may be NULL. + * tag, trace_json, params_json, and status may be NULL. * SQL parsing (table/column extraction) is done by the CLI tool. */ static void profiler_log_jsonl(const char *job_key, const char *query, size_t query_len, const char *tag, const char *trace_json, - const char *params_json) + const char *params_json, const char *status) { char *filepath; FILE *fp; @@ -251,6 +253,10 @@ static void profiler_log_jsonl(const char *job_key, const char *query, size_t qu fprintf(fp, ",\"trace\":%s", trace_json); } + if (status) { + fprintf(fp, ",\"s\":\"%s\"", status); + } + fprintf(fp, ",\"ts\":%.6f}\n", ts); efree(escaped_query); @@ -265,10 +271,11 @@ static void profiler_log_jsonl(const char *job_key, const char *query, size_t qu /* }}} */ /* {{{ profiler_log_query_internal - * Internal: log a query to all active jobs with optional params. + * Internal: log a query to all active jobs with optional params and status. * Captures the current context tag and PHP trace once, shared across all jobs. */ static void profiler_log_query_internal(const char *query, size_t query_len, - const char *params_json) + const char *params_json, + const char *status) { char **jobs; int job_count; @@ -289,11 +296,11 @@ static void profiler_log_query_internal(const char *query, size_t query_len, for (i = 0; i < job_count; i++) { /* Write JSONL entry */ - profiler_log_jsonl(jobs[i], query, query_len, tag, trace_json, params_json); + profiler_log_jsonl(jobs[i], query, query_len, tag, trace_json, params_json, status); /* Write raw log if enabled */ if (PROFILER_G(raw_log)) { - profiler_log_raw(jobs[i], query, query_len, tag, trace_json, params_json); + profiler_log_raw(jobs[i], query, query_len, tag, trace_json, params_json, status); } } @@ -304,19 +311,21 @@ static void profiler_log_query_internal(const char *query, size_t query_len, /* }}} */ /* {{{ profiler_log_query - * Main entry point: log a query (without params) to all active jobs. */ -void profiler_log_query(const char *query, size_t query_len) + * Main entry point: log a query (without params) to all active jobs. + * status is "ok" or "err" (NULL treated as "ok"). */ +void profiler_log_query(const char *query, size_t query_len, const char *status) { - profiler_log_query_internal(query, query_len, NULL); + profiler_log_query_internal(query, query_len, NULL, status); } /* }}} */ /* {{{ profiler_log_query_with_params - * Log a prepared statement query with bound parameter values to all active jobs. */ + * Log a prepared statement query with bound parameter values to all active jobs. + * status is "ok" or "err" (NULL treated as "ok"). */ void profiler_log_query_with_params(const char *query, size_t query_len, - const char *params_json) + const char *params_json, const char *status) { - profiler_log_query_internal(query, query_len, params_json); + profiler_log_query_internal(query, query_len, params_json, status); } /* }}} */ diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index 17b1020..85adb79 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -61,13 +61,17 @@ MYSQLND_METHOD(profiler_conn, query)( const char *query, PROFILER_QUERY_LEN_T query_len TSRMLS_DC) { - /* Log the query if any job is active */ + enum_func_status result; + + /* Call the original method first */ + result = orig_conn_data_methods->query(conn, query, query_len TSRMLS_CC); + + /* Log the query with execution status */ if (PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len); + profiler_log_query(query, query_len, result == PASS ? "ok" : "err"); } - /* Call the original method */ - return orig_conn_data_methods->query(conn, query, query_len TSRMLS_CC); + return result; } /* }}} */ @@ -87,10 +91,12 @@ MYSQLND_METHOD(profiler_conn, send_query)( zval *read_cb, zval *err_cb) { + enum_func_status result; + result = orig_conn_data_methods->send_query(conn, query, query_len, read_cb, err_cb); if (PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len); + profiler_log_query(query, query_len, result == PASS ? "ok" : "err"); } - return orig_conn_data_methods->send_query(conn, query, query_len, read_cb, err_cb); + return result; } #elif PHP_VERSION_ID >= 70000 /* PHP 7.0-8.0: no TSRMLS, size_t, with type param */ @@ -103,10 +109,12 @@ MYSQLND_METHOD(profiler_conn, send_query)( zval *read_cb, zval *err_cb) { + enum_func_status result; + result = orig_conn_data_methods->send_query(conn, query, query_len, type, read_cb, err_cb); if (PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len); + profiler_log_query(query, query_len, result == PASS ? "ok" : "err"); } - return orig_conn_data_methods->send_query(conn, query, query_len, type, read_cb, err_cb); + return result; } #else /* PHP 5.3-5.6: simple signature (conn, query, query_len TSRMLS_DC) */ @@ -116,10 +124,12 @@ MYSQLND_METHOD(profiler_conn, send_query)( const char *query, unsigned int query_len TSRMLS_DC) { + enum_func_status result; + result = orig_conn_data_methods->send_query(conn, query, query_len TSRMLS_CC); if (PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len); + profiler_log_query(query, query_len, result == PASS ? "ok" : "err"); } - return orig_conn_data_methods->send_query(conn, query, query_len TSRMLS_CC); + return result; } #endif /* }}} */ @@ -152,7 +162,7 @@ MYSQLND_METHOD(profiler_stmt, prepare)( #else /* PHP 5.x: log template at prepare time (no param support) */ if (result == PASS && PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len); + profiler_log_query(query, query_len, "ok"); } #endif @@ -319,11 +329,19 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) /* }}} */ /* {{{ profiler_stmt_execute_hook - * Intercepts prepared statement execution to log query with bound params. */ + * Intercepts prepared statement execution to log query with bound params. + * Logging is performed after execute so the result status can be recorded. + * param_bind remains valid after execute (freed only on stmt dtor / rebind). */ static enum_func_status MYSQLND_METHOD(profiler_stmt, execute)( MYSQLND_STMT * const stmt) { + enum_func_status result; + + /* Call the original method first */ + result = orig_stmt_methods->execute(stmt); + + /* Log with status and params after execution */ if (PROFILER_G(enabled) && profiler_job_is_any_active() && PROFILER_G(stmt_queries)) { zval *entry = zend_hash_index_find( PROFILER_G(stmt_queries), @@ -332,7 +350,8 @@ MYSQLND_METHOD(profiler_stmt, execute)( if (entry && Z_TYPE_P(entry) == IS_STRING) { char *params_json = profiler_build_params_json(stmt); profiler_log_query_with_params( - Z_STRVAL_P(entry), Z_STRLEN_P(entry), params_json + Z_STRVAL_P(entry), Z_STRLEN_P(entry), params_json, + result == PASS ? "ok" : "err" ); if (params_json) { efree(params_json); @@ -340,7 +359,7 @@ MYSQLND_METHOD(profiler_stmt, execute)( } } - return orig_stmt_methods->execute(stmt); + return result; } /* }}} */ diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index 1cea401..ca04e31 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -11,8 +11,13 @@ data class QueryEntry( val timestamp: Double = 0.0, @SerialName("k") val jobKey: String = "", + @SerialName("s") + val status: String? = null, + @SerialName("tag") val tag: String? = null, + @SerialName("params") val params: List = emptyList(), + @SerialName("trace") val trace: List = emptyList() ) { /** Whether this query has bound parameters (prepared statement) */ @@ -29,7 +34,8 @@ data class QueryEntry( * (when `NO_BACKSLASH_ESCAPES` is not set) * - Double-quoted string literals (`"..."`) with `""` as escaped quote * - Backtick-quoted identifiers (`` `...` ``) - * - Line comments (`-- ...`) + * - Line comments (`-- ...` where `--` is followed by whitespace) + * - Hash comments (`# ...`) * - Block comments (`/* ... */`) * * Param values are wrapped in single quotes with internal backslashes @@ -46,8 +52,22 @@ data class QueryEntry( while (i < query.length) { val ch = query[i] - // -- line comment: copy through to end of line - if (ch == '-' && i + 1 < query.length && query[i + 1] == '-') { + // -- line comment (MySQL/MariaDB requires space/tab/newline after --) + if (ch == '-' && i + 1 < query.length && query[i + 1] == '-' + && i + 2 < query.length && (query[i + 2] == ' ' || query[i + 2] == '\t' || query[i + 2] == '\n')) { + val eol = query.indexOf('\n', i) + if (eol == -1) { + sb.append(query, i, query.length) + i = query.length + } else { + sb.append(query, i, eol + 1) + i = eol + 1 + } + continue + } + + // # line comment (MySQL/MariaDB): copy through to end of line + if (ch == '#') { val eol = query.indexOf('\n', i) if (eol == -1) { sb.append(query, i, query.length) diff --git a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt index 34c4e6f..01e206a 100644 --- a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt +++ b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt @@ -339,4 +339,69 @@ class QueryEntryTest { ) assertEquals("SELECT * FROM logs WHERE msg = 'User said \\'hello\\'' AND id = '123'", entry.boundQuery) } + + // ---- status field tests ---- + + @Test + fun `parse entry with status ok`() { + val jsonStr = """{"k":"job1","q":"SELECT 1","s":"ok","ts":1705970401.0}""" + val entry = json.decodeFromString(jsonStr) + assertEquals("ok", entry.status) + } + + @Test + fun `parse entry with status err`() { + val jsonStr = """{"k":"job1","q":"SELECT bad","s":"err","ts":1705970401.0}""" + val entry = json.decodeFromString(jsonStr) + assertEquals("err", entry.status) + } + + @Test + fun `parse entry without status defaults to null`() { + val jsonStr = """{"k":"job1","q":"SELECT 1","ts":1705970401.0}""" + val entry = json.decodeFromString(jsonStr) + assertNull(entry.status) + } + + // ---- hash comment tests ---- + + @Test + fun `boundQuery skips placeholder in hash comment`() { + val entry = QueryEntry( + query = "SELECT * FROM t # where id = ?\nWHERE name = ?", + params = listOf("test") + ) + assertEquals("SELECT * FROM t # where id = ?\nWHERE name = 'test'", entry.boundQuery) + } + + @Test + fun `boundQuery skips placeholder in hash comment at end of query`() { + val entry = QueryEntry( + query = "SELECT 1 # comment ?", + params = listOf("unused") + ) + assertEquals("SELECT 1 # comment ?", entry.boundQuery) + } + + // ---- double-dash comment spec tests ---- + + @Test + fun `boundQuery does not treat double-dash without trailing space as comment`() { + // MySQL/MariaDB requires whitespace after -- for it to be a line comment. + // "1--?" is arithmetic, not a comment. + val entry = QueryEntry( + query = "SELECT 1--?", + params = listOf("2") + ) + assertEquals("SELECT 1--'2'", entry.boundQuery) + } + + @Test + fun `boundQuery treats double-dash with tab as comment`() { + val entry = QueryEntry( + query = "SELECT * FROM t --\twhere id = ?\nWHERE name = ?", + params = listOf("test") + ) + assertEquals("SELECT * FROM t --\twhere id = ?\nWHERE name = 'test'", entry.boundQuery) + } } From f3f67ac365821796970d1d9e0e303d975fc699f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Feb 2026 00:41:48 +0000 Subject: [PATCH 7/8] Fix prepare-hook logging, bind-type param formatting, and \r in comment detection - Log failed prepare() with status "err" on PHP 7+ (previously lost since no execute() follows a failed prepare) - Rewrite profiler_build_params_json to dispatch on the declared bind type (MYSQL_TYPE_LONG/DOUBLE/VAR_STRING/LONG_BLOB) instead of the zval's runtime type, matching the coercion mysqlnd applies before sending to the server - Extract string-writing helper to reduce duplication in param builder - Add \r to the -- line-comment whitespace check in QueryEntry.kt so that --\r\n line endings are recognised as comments - Add test for CRLF line-comment edge case https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- .../profiler_mysqlnd_plugin.c | 161 ++++++++++-------- .../plugin/model/QueryEntry.kt | 2 +- .../plugin/model/QueryEntryTest.kt | 10 ++ 3 files changed, 101 insertions(+), 72 deletions(-) diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index 85adb79..c9748e7 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -149,15 +149,20 @@ MYSQLND_METHOD(profiler_stmt, prepare)( result = orig_stmt_methods->prepare(stmt, query, query_len TSRMLS_CC); #if PHP_VERSION_ID >= 70000 - /* Store query template on success - will be logged at execute() with bound params */ - if (result == PASS && PROFILER_G(enabled) && PROFILER_G(stmt_queries)) { - zval zv; - ZVAL_STRINGL(&zv, query, query_len); - zend_hash_index_update( - PROFILER_G(stmt_queries), - (zend_ulong)(uintptr_t)stmt, - &zv - ); + if (PROFILER_G(enabled)) { + if (result == PASS && PROFILER_G(stmt_queries)) { + /* Store query template on success - will be logged at execute() with bound params */ + zval zv; + ZVAL_STRINGL(&zv, query, query_len); + zend_hash_index_update( + PROFILER_G(stmt_queries), + (zend_ulong)(uintptr_t)stmt, + &zv + ); + } else if (result != PASS && profiler_job_is_any_active()) { + /* Failed prepare has no subsequent execute(); log immediately with err status */ + profiler_log_query(query, query_len, "err"); + } } #else /* PHP 5.x: log template at prepare time (no param support) */ @@ -185,8 +190,36 @@ MYSQLND_METHOD(profiler_stmt, prepare)( #define PROFILER_MYSQLND_PARAM_ACCESS_SAFE \ (MYSQLND_VERSION_ID >= 70000 && MYSQLND_VERSION_ID < 80500) +/* {{{ profiler_build_params_json_write_string + * Helper: write a JSON-escaped string value into the buffer. + * Grows the buffer as needed. Returns updated position. */ +static size_t profiler_build_params_json_write_string( + char **buf_ptr, size_t buf_size, size_t pos, + const char *str, size_t str_len) +{ + char *buf = *buf_ptr; + char *escaped = profiler_log_escape_json_string(str, str_len); + size_t esc_len = strlen(escaped); + + if (pos + esc_len + 4 > buf_size) { + buf_size = pos + esc_len + 256; + buf = (char *)erealloc(buf, buf_size); + *buf_ptr = buf; + } + + buf[pos++] = '"'; + memcpy(buf + pos, escaped, esc_len); + pos += esc_len; + buf[pos++] = '"'; + efree(escaped); + return pos; +} + /* {{{ profiler_build_params_json * Build JSON array string from stmt's bound parameter values. + * Formats each value according to the declared bind type (MYSQL_TYPE_*) + * rather than the zval's runtime type, matching the coercion mysqlnd + * performs before sending data to the server. * Returns emalloc'd string or NULL if no params. Caller must efree. */ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) { @@ -208,6 +241,7 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) for (i = 0; i < data->param_count; i++) { zval *zv = &data->param_bind[i].zv; + zend_uchar bind_type = data->param_bind[i].type; if (i > 0) { buf[pos++] = ','; @@ -222,95 +256,80 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) /* Dereference if reference (bind_param uses references) */ ZVAL_DEREF(zv); - switch (Z_TYPE_P(zv)) { - case IS_NULL: - memcpy(buf + pos, "null", 4); - pos += 4; - break; + /* NULL zval is always serialized as JSON null regardless of bind type */ + if (Z_TYPE_P(zv) == IS_NULL) { + memcpy(buf + pos, "null", 4); + pos += 4; + continue; + } - case IS_LONG: { - int written = snprintf(buf + pos, buf_size - pos, "\"%ld\"", - (long)Z_LVAL_P(zv)); + /* + * Format according to the declared bind type rather than zval type, + * matching the coercion mysqlnd performs before sending to the server. + * 'i' -> MYSQL_TYPE_LONG 'd' -> MYSQL_TYPE_DOUBLE + * 's' -> MYSQL_TYPE_VAR_STRING 'b' -> MYSQL_TYPE_LONG_BLOB + */ + switch (bind_type) { + case MYSQL_TYPE_LONG: + case MYSQL_TYPE_LONGLONG: { + zend_long val = (Z_TYPE_P(zv) == IS_LONG) + ? Z_LVAL_P(zv) : zval_get_long(zv); + int written = snprintf(buf + pos, buf_size - pos, + "\"%ld\"", (long)val); if (written < 0) { - break; /* encoding error */ + break; } if ((size_t)written >= buf_size - pos) { - /* Truncated – grow and retry */ buf_size = pos + (size_t)written + 64; buf = (char *)erealloc(buf, buf_size); - pos += snprintf(buf + pos, buf_size - pos, "\"%ld\"", - (long)Z_LVAL_P(zv)); + pos += snprintf(buf + pos, buf_size - pos, + "\"%ld\"", (long)val); } else { pos += (size_t)written; } break; } - case IS_DOUBLE: { - int written = snprintf(buf + pos, buf_size - pos, "\"%g\"", - Z_DVAL_P(zv)); + case MYSQL_TYPE_DOUBLE: + case MYSQL_TYPE_FLOAT: { + double val = (Z_TYPE_P(zv) == IS_DOUBLE) + ? Z_DVAL_P(zv) : zval_get_double(zv); + int written = snprintf(buf + pos, buf_size - pos, + "\"%g\"", val); if (written < 0) { - break; /* encoding error */ + break; } if ((size_t)written >= buf_size - pos) { - /* Truncated – grow and retry */ buf_size = pos + (size_t)written + 64; buf = (char *)erealloc(buf, buf_size); - pos += snprintf(buf + pos, buf_size - pos, "\"%g\"", - Z_DVAL_P(zv)); + pos += snprintf(buf + pos, buf_size - pos, + "\"%g\"", val); } else { pos += (size_t)written; } break; } - case IS_TRUE: - memcpy(buf + pos, "\"1\"", 3); - pos += 3; + case MYSQL_TYPE_LONG_BLOB: + /* Blob data sent via send_long_data – log placeholder */ + memcpy(buf + pos, "\"[BLOB]\"", 8); + pos += 8; break; - case IS_FALSE: - memcpy(buf + pos, "\"\"", 2); - pos += 2; - break; - - case IS_STRING: { - char *escaped = profiler_log_escape_json_string( - Z_STRVAL_P(zv), Z_STRLEN_P(zv)); - size_t esc_len = strlen(escaped); - - /* Grow buffer for escaped string */ - if (pos + esc_len + 4 > buf_size) { - buf_size = pos + esc_len + 256; - buf = (char *)erealloc(buf, buf_size); - } - - buf[pos++] = '"'; - memcpy(buf + pos, escaped, esc_len); - pos += esc_len; - buf[pos++] = '"'; - efree(escaped); - break; - } - default: { - /* Convert unknown types to string */ - zend_string *str = zval_get_string(zv); - char *escaped = profiler_log_escape_json_string( - ZSTR_VAL(str), ZSTR_LEN(str)); - size_t esc_len = strlen(escaped); - - if (pos + esc_len + 4 > buf_size) { - buf_size = pos + esc_len + 256; - buf = (char *)erealloc(buf, buf_size); + /* String types (MYSQL_TYPE_VAR_STRING, etc.) and any + * unrecognised bind type: coerce to string */ + if (Z_TYPE_P(zv) == IS_STRING) { + pos = profiler_build_params_json_write_string( + &buf, buf_size, pos, + Z_STRVAL_P(zv), Z_STRLEN_P(zv)); + } else { + zend_string *str = zval_get_string(zv); + pos = profiler_build_params_json_write_string( + &buf, buf_size, pos, + ZSTR_VAL(str), ZSTR_LEN(str)); + zend_string_release(str); } - - buf[pos++] = '"'; - memcpy(buf + pos, escaped, esc_len); - pos += esc_len; - buf[pos++] = '"'; - efree(escaped); - zend_string_release(str); break; } } diff --git a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt index ca04e31..430801e 100644 --- a/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt +++ b/jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt @@ -54,7 +54,7 @@ data class QueryEntry( // -- line comment (MySQL/MariaDB requires space/tab/newline after --) if (ch == '-' && i + 1 < query.length && query[i + 1] == '-' - && i + 2 < query.length && (query[i + 2] == ' ' || query[i + 2] == '\t' || query[i + 2] == '\n')) { + && i + 2 < query.length && (query[i + 2] == ' ' || query[i + 2] == '\t' || query[i + 2] == '\n' || query[i + 2] == '\r')) { val eol = query.indexOf('\n', i) if (eol == -1) { sb.append(query, i, query.length) diff --git a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt index 01e206a..2f75aa9 100644 --- a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt +++ b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt @@ -404,4 +404,14 @@ class QueryEntryTest { ) assertEquals("SELECT * FROM t --\twhere id = ?\nWHERE name = 'test'", entry.boundQuery) } + + @Test + fun `boundQuery treats double-dash with CRLF as comment`() { + val entry = QueryEntry( + query = "SELECT * FROM t --\r\nwhere id = ?\r\nAND name = ?", + params = listOf("test") + ) + // -- followed by \r starts a comment through to \n, second ? is outside comment + assertEquals("SELECT * FROM t --\r\nwhere id = ?\r\nAND name = 'test'", entry.boundQuery) + } } From a1ea11165994eb34aa12e9b33fd0f2b05a08cf56 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Feb 2026 00:57:31 +0000 Subject: [PATCH 8/8] Fix CRLF test expectation, buffer capacity propagation, and PHP 5 prepare logging - Fix CRLF comment test: --\r\n comment ends at \n, so text on the next line is outside the comment and its ? placeholder gets replaced - Fix profiler_build_params_json_write_string to propagate updated buf_size back to caller via pointer, preventing stale capacity after erealloc that could cause out-of-bounds writes on subsequent params - Log failed prepare() on PHP 5.x branch (previously only logged on PASS, making syntax/permission errors invisible) https://claude.ai/code/session_01RetwzD4CVW4uBT9hS2aSPw --- ext/mariadb_profiler/profiler_mysqlnd_plugin.c | 15 +++++++++------ .../plugin/model/QueryEntryTest.kt | 5 +++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c index c9748e7..c9f57c1 100644 --- a/ext/mariadb_profiler/profiler_mysqlnd_plugin.c +++ b/ext/mariadb_profiler/profiler_mysqlnd_plugin.c @@ -166,8 +166,8 @@ MYSQLND_METHOD(profiler_stmt, prepare)( } #else /* PHP 5.x: log template at prepare time (no param support) */ - if (result == PASS && PROFILER_G(enabled) && profiler_job_is_any_active()) { - profiler_log_query(query, query_len, "ok"); + if (PROFILER_G(enabled) && profiler_job_is_any_active()) { + profiler_log_query(query, query_len, result == PASS ? "ok" : "err"); } #endif @@ -192,12 +192,14 @@ MYSQLND_METHOD(profiler_stmt, prepare)( /* {{{ profiler_build_params_json_write_string * Helper: write a JSON-escaped string value into the buffer. - * Grows the buffer as needed. Returns updated position. */ + * Grows the buffer as needed via *buf_ptr / *buf_size_ptr. + * Returns updated write position. */ static size_t profiler_build_params_json_write_string( - char **buf_ptr, size_t buf_size, size_t pos, + char **buf_ptr, size_t *buf_size_ptr, size_t pos, const char *str, size_t str_len) { char *buf = *buf_ptr; + size_t buf_size = *buf_size_ptr; char *escaped = profiler_log_escape_json_string(str, str_len); size_t esc_len = strlen(escaped); @@ -205,6 +207,7 @@ static size_t profiler_build_params_json_write_string( buf_size = pos + esc_len + 256; buf = (char *)erealloc(buf, buf_size); *buf_ptr = buf; + *buf_size_ptr = buf_size; } buf[pos++] = '"'; @@ -321,12 +324,12 @@ static char *profiler_build_params_json(MYSQLND_STMT * const stmt) * unrecognised bind type: coerce to string */ if (Z_TYPE_P(zv) == IS_STRING) { pos = profiler_build_params_json_write_string( - &buf, buf_size, pos, + &buf, &buf_size, pos, Z_STRVAL_P(zv), Z_STRLEN_P(zv)); } else { zend_string *str = zval_get_string(zv); pos = profiler_build_params_json_write_string( - &buf, buf_size, pos, + &buf, &buf_size, pos, ZSTR_VAL(str), ZSTR_LEN(str)); zend_string_release(str); } diff --git a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt index 2f75aa9..ec43976 100644 --- a/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt +++ b/jetbrains-plugin/src/test/kotlin/com/mariadbprofiler/plugin/model/QueryEntryTest.kt @@ -407,11 +407,12 @@ class QueryEntryTest { @Test fun `boundQuery treats double-dash with CRLF as comment`() { + // --\r\n is a comment that ends at the newline; "where id = ?" is on the + // next line so its ? is outside the comment and consumes the param. val entry = QueryEntry( query = "SELECT * FROM t --\r\nwhere id = ?\r\nAND name = ?", params = listOf("test") ) - // -- followed by \r starts a comment through to \n, second ? is outside comment - assertEquals("SELECT * FROM t --\r\nwhere id = ?\r\nAND name = 'test'", entry.boundQuery) + assertEquals("SELECT * FROM t --\r\nwhere id = 'test'\r\nAND name = ?", entry.boundQuery) } }