From 661ba814af6748ccdc555e87352ebbcb5a101f32 Mon Sep 17 00:00:00 2001 From: Stiwar0098 Date: Mon, 22 Jun 2026 02:37:19 -0500 Subject: [PATCH 1/2] fix(mysql): route routine DDL through text protocol --- .gitignore | 2 ++ src-tauri/src/drivers/mysql/mod.rs | 18 +++++++++++--- src-tauri/src/drivers/mysql/tests.rs | 35 +++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 420447e3..0d87623b 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,5 @@ coverage plugins/**/target .gitnexus .pi/ +.atl/ +.opencode/ diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index a01e5591..45798b53 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -879,11 +879,16 @@ async fn acquire_mysql_conn( /// statement protocol yet"). `sqlx::query()` always goes through /// `COM_STMT_PREPARE` + `COM_STMT_EXECUTE`, so these have to be routed /// through `sqlx::raw_sql()` which uses `COM_QUERY` (text protocol) -/// instead. Without this, explicit transactions inside a multi-statement -/// script (`BEGIN; … COMMIT;`) silently fail — which would defeat the -/// point of `execute_batch` even after sharing a single connection. +/// instead. This currently covers transaction/lock control plus routine +/// DDL that MySQL rejects when prepared. Without this, explicit +/// transactions inside a multi-statement script (`BEGIN; … COMMIT;`) +/// silently fail — which would defeat the point of `execute_batch` even +/// after sharing a single connection. fn is_text_protocol_stmt(query: &str) -> bool { let head = crate::drivers::common::strip_leading_sql_comments(query).to_uppercase(); + let is_create_definer_routine = head.starts_with("CREATE DEFINER") + && (head.contains(" PROCEDURE") || head.contains(" FUNCTION")); + head.starts_with("BEGIN") || head.starts_with("START TRANSACTION") || head.starts_with("COMMIT") @@ -892,6 +897,13 @@ fn is_text_protocol_stmt(query: &str) -> bool { || head.starts_with("RELEASE SAVEPOINT") || head.starts_with("LOCK TABLES") || head.starts_with("UNLOCK TABLES") + || head.starts_with("DROP PROCEDURE") + || head.starts_with("CREATE PROCEDURE") + || head.starts_with("ALTER PROCEDURE") + || head.starts_with("DROP FUNCTION") + || head.starts_with("CREATE FUNCTION") + || head.starts_with("ALTER FUNCTION") + || is_create_definer_routine } /// Executes one statement on an already-acquired connection. Used by both diff --git a/src-tauri/src/drivers/mysql/tests.rs b/src-tauri/src/drivers/mysql/tests.rs index e26dc835..70ea8e83 100644 --- a/src-tauri/src/drivers/mysql/tests.rs +++ b/src-tauri/src/drivers/mysql/tests.rs @@ -1,5 +1,5 @@ use super::explain::{parse_analyze_actual, parse_mysql_analyze_text, parse_mysql_query_block}; -use super::MysqlDriver; +use super::{is_text_protocol_stmt, MysqlDriver}; use crate::drivers::driver_trait::DatabaseDriver; use crate::models::ExplainNode; use crate::models::{ConnectionParams, DatabaseSelection}; @@ -601,3 +601,36 @@ fn parse_mysql_analyze_text_reports_total_time_for_looped_node() { "expected ~2646ms total for index lookup, got {total}" ); } + +#[test] +fn routes_mysql_routine_ddl_through_text_protocol() { + for sql in [ + "DROP PROCEDURE IF EXISTS sociedades_close;", + "CREATE PROCEDURE sociedades_close() SELECT 1;", + "CREATE DEFINER=`root`@`localhost` PROCEDURE sociedades_close() SELECT 1;", + "ALTER PROCEDURE sociedades_close COMMENT 'patched';", + "DROP FUNCTION IF EXISTS sociedades_total;", + "CREATE FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE DEFINER=`root`@`localhost` FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "ALTER FUNCTION sociedades_total COMMENT 'patched';", + ] { + assert!( + is_text_protocol_stmt(sql), + "expected text protocol routing for {sql}" + ); + } +} + +#[test] +fn keeps_regular_dml_out_of_text_protocol_classifier() { + for sql in [ + "SELECT * FROM routines", + "INSERT INTO routines(name) VALUES ('sociedades_close')", + "DROP TABLE IF EXISTS routines_backup", + ] { + assert!( + !is_text_protocol_stmt(sql), + "did not expect text protocol routing for {sql}" + ); + } +} From 3ee7d3914dd2e603c9df17c32934d4594beecc80 Mon Sep 17 00:00:00 2001 From: Stiwar0098 Date: Mon, 22 Jun 2026 16:16:51 -0500 Subject: [PATCH 2/2] fix(mysql): cover create or replace routine DDL --- src-tauri/src/drivers/mysql/mod.rs | 189 ++++++++++++++++++++++++++- src-tauri/src/drivers/mysql/tests.rs | 67 ++++++++++ 2 files changed, 254 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index 45798b53..914ce4a1 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -884,10 +884,192 @@ async fn acquire_mysql_conn( /// transactions inside a multi-statement script (`BEGIN; … COMMIT;`) /// silently fail — which would defeat the point of `execute_batch` even /// after sharing a single connection. +/// +/// For `CREATE [OR REPLACE] DEFINER = …` statements the object-type +/// keyword (`PROCEDURE`/`FUNCTION`/`VIEW`/`TRIGGER`/`EVENT`/`TABLE`) +/// follows the definer clause. Returns the slice starting at that +/// keyword, or `None` if the definer clause cannot be skipped. +/// +/// The definer value is NOT always a single token: MySQL accepts spaced +/// quoted forms such as `'root' @ 'localhost'`, backtick-quoted +/// identifiers like `` `root`@`localhost` ``, bare `user@host`, and +/// `CURRENT_USER` / `CURRENT_USER()`. Splitting on the first whitespace +/// would stop inside a spaced definer and mistake the host part for the +/// object keyword, so we scan the remainder character by character — +/// tracking single-quote, double-quote, backtick and paren state — and +/// stop at the first top-level object-type keyword that appears outside +/// any quoting and outside parentheses. A keyword is only accepted when +/// followed by whitespace or end-of-string, so a username like +/// `procedure@localhost` is not mistaken for the `PROCEDURE` keyword. +fn after_definer_clause(head: &str) -> Option<&str> { + // `head` is already uppercased and known to start with + // `CREATE [OR REPLACE] DEFINER`, so a plain `find` is sufficient + // here — there is no risk of matching `DEFINER` inside a quoted + // body before the clause itself. + let definer_idx = head.find("DEFINER")?; + let after_eq = head[definer_idx + "DEFINER".len()..] + .trim_start() + .strip_prefix('=')? + .trim_start(); + find_first_top_level_object_keyword(after_eq) +} + +/// Scans `s` character by character, tracking single-quote, double-quote, +/// backtick and paren depth, and returns the slice starting at the first +/// top-level object-type keyword (`PROCEDURE`, `FUNCTION`, `VIEW`, +/// `TRIGGER`, `EVENT`, `TABLE`) that is followed by whitespace or +/// end-of-string. Returns `None` when no such keyword appears at the top +/// level before the string ends. This keeps the scan focused on the +/// statement head and avoids re-introducing the full-statement +/// `contains` overmatching that would fire on `PROCEDURE`/`FUNCTION` +/// words appearing inside a VIEW's `SELECT` body. +fn find_first_top_level_object_keyword(s: &str) -> Option<&str> { + const KEYWORDS: &[&str] = &[ + "PROCEDURE", "FUNCTION", "VIEW", "TRIGGER", "EVENT", "TABLE", + ]; + let bytes = s.as_bytes(); + let mut i = 0; + let mut in_single = false; + let mut in_double = false; + let mut in_backtick = false; + let mut paren: u32 = 0; + let mut word_start: Option = None; + + while i < bytes.len() { + let c = bytes[i]; + + // Inside quotes: skip everything until the matching close quote. + if in_single { + if c == b'\'' { + in_single = false; + } + i += 1; + continue; + } + if in_double { + if c == b'"' { + in_double = false; + } + i += 1; + continue; + } + if in_backtick { + if c == b'`' { + in_backtick = false; + } + i += 1; + continue; + } + + // Inside parentheses (e.g. `CURRENT_USER()`): skip, but keep + // tracking nested quotes/parens so a `)` inside a quoted string + // does not close the group early. + if paren > 0 { + match c { + b'\'' => in_single = true, + b'"' => in_double = true, + b'`' => in_backtick = true, + b'(' => paren += 1, + b')' => paren -= 1, + _ => {} + } + i += 1; + continue; + } + + // Top level: track entry into quotes/parens. + match c { + b'\'' => { + in_single = true; + i += 1; + continue; + } + b'"' => { + in_double = true; + i += 1; + continue; + } + b'`' => { + in_backtick = true; + i += 1; + continue; + } + b'(' => { + paren += 1; + i += 1; + continue; + } + b')' => { + i += 1; + continue; + } + _ => {} + } + + // Accumulate word characters at the top level. + if c.is_ascii_alphanumeric() || c == b'_' { + if word_start.is_none() { + word_start = Some(i); + } + i += 1; + continue; + } + + // Non-word character at top level: close any open word and + // check whether it is an object-type keyword followed by a + // whitespace/end-of-string boundary. + if let Some(start) = word_start.take() { + let word = &s[start..i]; + if KEYWORDS.contains(&word) && is_keyword_boundary(bytes, i) { + return Some(&s[start..]); + } + } + i += 1; + } + + // End of string: close any trailing word (end-of-string is a valid + // boundary for an object-type keyword). + if let Some(start) = word_start.take() { + let word = &s[start..]; + if KEYWORDS.contains(&word) { + return Some(&s[start..]); + } + } + None +} + +/// Returns `true` when the byte at `idx` is a valid terminator for an +/// object-type keyword (whitespace or end-of-string). This prevents +/// matching a username like `procedure@localhost` as the `PROCEDURE` +/// keyword, since `@` is not a keyword boundary. +fn is_keyword_boundary(bytes: &[u8], idx: usize) -> bool { + idx >= bytes.len() || bytes[idx].is_ascii_whitespace() +} + +/// Returns `true` when a `CREATE [OR REPLACE] DEFINER = …` statement's +/// object-type keyword — the first top-level object keyword after the +/// definer clause — is `PROCEDURE` or `FUNCTION`. This is tighter than +/// `contains` over the full statement, which would overmatch when a +/// VIEW's `SELECT` body mentions ` PROCEDURE`/` FUNCTION`, and it +/// tolerates spaced definer forms such as `'root' @ 'localhost'`. +fn definer_stmt_is_routine(head: &str) -> bool { + matches!( + after_definer_clause(head).and_then(|rest| rest.split_whitespace().next()), + Some("PROCEDURE" | "FUNCTION") + ) +} + fn is_text_protocol_stmt(query: &str) -> bool { let head = crate::drivers::common::strip_leading_sql_comments(query).to_uppercase(); - let is_create_definer_routine = head.starts_with("CREATE DEFINER") - && (head.contains(" PROCEDURE") || head.contains(" FUNCTION")); + let is_create_definer_routine = + head.starts_with("CREATE DEFINER") && definer_stmt_is_routine(&head); + // MariaDB allows `CREATE OR REPLACE [DEFINER = …] PROCEDURE|FUNCTION`, + // which is likewise rejected by the prepared-statement protocol. MySQL + // does NOT support `OR REPLACE` for routines, but the same text-protocol + // routing applies whenever such a statement is submitted against a + // MariaDB backend. + let is_create_or_replace_routine = + head.starts_with("CREATE OR REPLACE DEFINER") && definer_stmt_is_routine(&head); head.starts_with("BEGIN") || head.starts_with("START TRANSACTION") @@ -903,7 +1085,10 @@ fn is_text_protocol_stmt(query: &str) -> bool { || head.starts_with("DROP FUNCTION") || head.starts_with("CREATE FUNCTION") || head.starts_with("ALTER FUNCTION") + || head.starts_with("CREATE OR REPLACE PROCEDURE") + || head.starts_with("CREATE OR REPLACE FUNCTION") || is_create_definer_routine + || is_create_or_replace_routine } /// Executes one statement on an already-acquired connection. Used by both diff --git a/src-tauri/src/drivers/mysql/tests.rs b/src-tauri/src/drivers/mysql/tests.rs index 70ea8e83..024c001e 100644 --- a/src-tauri/src/drivers/mysql/tests.rs +++ b/src-tauri/src/drivers/mysql/tests.rs @@ -608,10 +608,18 @@ fn routes_mysql_routine_ddl_through_text_protocol() { "DROP PROCEDURE IF EXISTS sociedades_close;", "CREATE PROCEDURE sociedades_close() SELECT 1;", "CREATE DEFINER=`root`@`localhost` PROCEDURE sociedades_close() SELECT 1;", + "CREATE DEFINER=`root`@`localhost` PROCEDURE sociedades_close() SELECT 1;", + "CREATE OR REPLACE PROCEDURE sociedades_close() SELECT 1;", + "CREATE OR REPLACE DEFINER=`root`@`localhost` PROCEDURE sociedades_close() SELECT 1;", + "CREATE OR REPLACE DEFINER=`root`@`localhost` PROCEDURE sociedades_close() SELECT 1;", "ALTER PROCEDURE sociedades_close COMMENT 'patched';", "DROP FUNCTION IF EXISTS sociedades_total;", "CREATE FUNCTION sociedades_total() RETURNS INT RETURN 1;", "CREATE DEFINER=`root`@`localhost` FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE DEFINER=`root`@`localhost` FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE OR REPLACE FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE OR REPLACE DEFINER=`root`@`localhost` FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE OR REPLACE DEFINER=`root`@`localhost` FUNCTION sociedades_total() RETURNS INT RETURN 1;", "ALTER FUNCTION sociedades_total COMMENT 'patched';", ] { assert!( @@ -627,6 +635,9 @@ fn keeps_regular_dml_out_of_text_protocol_classifier() { "SELECT * FROM routines", "INSERT INTO routines(name) VALUES ('sociedades_close')", "DROP TABLE IF EXISTS routines_backup", + // `CREATE OR REPLACE` is also valid for non-routine objects such as + // VIEW that are not part of this routing rule — must not match. + "CREATE OR REPLACE VIEW routines_view AS SELECT 1", ] { assert!( !is_text_protocol_stmt(sql), @@ -634,3 +645,59 @@ fn keeps_regular_dml_out_of_text_protocol_classifier() { ); } } + +#[test] +fn definer_view_with_routine_words_in_body_is_not_text_protocol() { + // `CREATE [OR REPLACE] DEFINER … VIEW … AS SELECT …` must not be + // classified as a routine even when the SELECT body mentions + // `PROCEDURE`/`FUNCTION`. Regression for the loose `contains`-based + // matching that searched the full statement. + for sql in [ + "CREATE DEFINER=`root`@`localhost` VIEW v AS SELECT 'call PROCEDURE foo' AS col", + "CREATE OR REPLACE DEFINER=`root`@`localhost` VIEW v AS SELECT name FROM routines WHERE note LIKE '%FUNCTION%'", + "CREATE OR REPLACE DEFINER=CURRENT_USER() VIEW v AS SELECT 'PROCEDURE' AS word UNION SELECT 'FUNCTION' AS word", + ] { + assert!( + !is_text_protocol_stmt(sql), + "DEFINER … VIEW with routine words in body must not route through text protocol: {sql}" + ); + } +} + +#[test] +fn spaced_definer_routes_routines_through_text_protocol() { + // MySQL accepts spaced definer forms such as `'root' @ 'localhost'` + // where the value contains internal whitespace. The classifier must + // skip past the whole definer value and find the real object keyword + // instead of stopping at the first space inside the definer. + for sql in [ + "CREATE DEFINER = 'root' @ 'localhost' PROCEDURE sociedades_close() SELECT 1;", + "CREATE DEFINER = 'root' @ 'localhost' PROCEDURE sociedades_close() SELECT 1;", + "CREATE OR REPLACE DEFINER = 'root' @ 'localhost' FUNCTION sociedades_total() RETURNS INT RETURN 1;", + "CREATE OR REPLACE DEFINER = 'root' @ 'localhost' FUNCTION sociedades_total() RETURNS INT RETURN 1;", + ] { + assert!( + is_text_protocol_stmt(sql), + "expected text protocol routing for spaced definer routine: {sql}" + ); + } +} + +#[test] +fn spaced_definer_view_with_routine_words_in_body_is_not_text_protocol() { + // A spaced definer must not let `PROCEDURE`/`FUNCTION` words that + // appear inside a VIEW body route the statement through text + // protocol — only the actual object-type keyword after the definer + // clause counts, and the scan must stop at `VIEW` before reaching + // the body. + for sql in [ + "CREATE DEFINER = 'root' @ 'localhost' VIEW v AS SELECT 'call PROCEDURE foo' AS col", + "CREATE OR REPLACE DEFINER = 'root' @ 'localhost' VIEW v AS SELECT name FROM routines WHERE note LIKE '%FUNCTION%'", + "CREATE DEFINER = 'root' @ 'localhost' VIEW v AS SELECT 'PROCEDURE' AS word UNION SELECT 'FUNCTION' AS word", + ] { + assert!( + !is_text_protocol_stmt(sql), + "spaced definer VIEW with routine words in body must not route through text protocol: {sql}" + ); + } +}