From fe113f473849ef0102f8882f4b006aba73789fa9 Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Thu, 23 Jul 2026 19:18:46 +0330 Subject: [PATCH] fix(any): rewrite ? placeholders to $N for Postgres backend (#3000) QueryBuilder::push_bind() always writes '?' placeholders since AnyArguments has no backend context at build time. This adds a SQL-aware rewrite (skipping string literals, quoted identifiers, dollar-quoted strings, and comments) applied on the Postgres leg of the Any driver, right before the query hits the wire protocol in fetch_many/fetch_optional/prepare_with/describe. MySQL and SQLite are unaffected since they already use '?' natively. Fixes #3000 --- sqlx-postgres/src/any.rs | 221 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 220 insertions(+), 1 deletion(-) diff --git a/sqlx-postgres/src/any.rs b/sqlx-postgres/src/any.rs index 62b3dedbac..9a73a1a0f9 100644 --- a/sqlx-postgres/src/any.rs +++ b/sqlx-postgres/src/any.rs @@ -5,7 +5,8 @@ use crate::{ use futures_core::future::BoxFuture; use futures_core::stream::BoxStream; use futures_util::{stream, FutureExt, StreamExt, TryFutureExt, TryStreamExt}; -use sqlx_core::sql_str::SqlStr; +use sqlx_core::sql_str::{AssertSqlSafe, SqlSafeStr, SqlStr}; +use std::borrow::Cow; use std::{future, pin::pin}; use sqlx_core::any::{ @@ -22,6 +23,134 @@ use sqlx_core::transaction::TransactionManager; sqlx_core::declare_driver_with_optional_migrate!(DRIVER = Postgres); +/// Rewrite `?`-style placeholders (as produced by the `Any` driver, e.g. via `QueryBuilder`) +/// into Postgres-style positional placeholders (`$1`, `$2`, ...). +/// +/// `AnyArguments` binds its values in the same left-to-right order that `?` placeholders were +/// written, so numbering them `1..=N` in order of appearance preserves the correct binding. +/// +/// This is SQL-aware to avoid rewriting a literal `?` character that appears inside: +/// - a single-quoted string literal (`'...'`, with `''` as an escaped quote) +/// - a double-quoted identifier (`"..."`, with `""` as an escaped quote) +/// - a dollar-quoted string (`$$...$$` or `$tag$...$tag$`) +/// - a `--` line comment or a `/* */` block comment (not nested) +/// +/// Only MySQL and SQLite use `?` natively, so this rewrite is only needed on the Postgres leg +/// of the `Any` driver; see . +fn rewrite_any_placeholders(sql: &str) -> Cow<'_, str> { + if !sql.contains('?') { + return Cow::Borrowed(sql); + } + + let chars: Vec = sql.chars().collect(); + let mut out = String::with_capacity(sql.len() + 8); + let mut i = 0usize; + let mut placeholder_num = 0usize; + + while i < chars.len() { + let c = chars[i]; + match c { + '?' => { + placeholder_num += 1; + out.push('$'); + out.push_str(&placeholder_num.to_string()); + i += 1; + } + '\'' | '"' => { + // String literal or quoted identifier; the doubled-quote is the escape for both. + let quote = c; + out.push(c); + i += 1; + while i < chars.len() { + let ch = chars[i]; + out.push(ch); + i += 1; + if ch == quote { + if chars.get(i) == Some("e) { + out.push(quote); + i += 1; + continue; + } + break; + } + } + } + '-' if chars.get(i + 1) == Some(&'-') => { + // Line comment: copy through to (but not including) the newline. + while i < chars.len() && chars[i] != '\n' { + out.push(chars[i]); + i += 1; + } + } + '/' if chars.get(i + 1) == Some(&'*') => { + // Block comment (not handling nesting; Postgres nested comments are a rare + // edge case and out of scope for this fix). + out.push(chars[i]); + out.push(chars[i + 1]); + i += 2; + while i < chars.len() { + if chars[i] == '*' && chars.get(i + 1) == Some(&'/') { + out.push('*'); + out.push('/'); + i += 2; + break; + } + out.push(chars[i]); + i += 1; + } + } + '$' => { + // Possible dollar-quote opening tag: `$tag$` where `tag` is `[A-Za-z0-9_]*` + // (empty tag is the common `$$...$$` form). + let mut j = i + 1; + while j < chars.len() && (chars[j].is_alphanumeric() || chars[j] == '_') { + j += 1; + } + if j < chars.len() && chars[j] == '$' { + let tag: String = chars[i + 1..j].iter().collect(); + let open: String = chars[i..=j].iter().collect(); + out.push_str(&open); + i = j + 1; + + let close: Vec = format!("${tag}$").chars().collect(); + let mut found = false; + while i < chars.len() { + if chars[i..].starts_with(&close[..]) { + out.extend(&close); + i += close.len(); + found = true; + break; + } + out.push(chars[i]); + i += 1; + } + // If unterminated, we've already copied through to the end of input; + // nothing further to do (matches Postgres's own eventual parse error). + let _ = found; + } else { + out.push('$'); + i += 1; + } + } + _ => { + out.push(c); + i += 1; + } + } + } + + Cow::Owned(out) +} + +/// Apply [`rewrite_any_placeholders`] to a [`SqlStr`], only allocating a new one if a rewrite +/// was actually needed. +fn maybe_rewrite_any_placeholders(sql: SqlStr) -> SqlStr { + match rewrite_any_placeholders(sql.as_str()) { + Cow::Borrowed(_) => sql, + Cow::Owned(rewritten) => AssertSqlSafe(rewritten).into_sql_str(), + } +} + impl AnyConnectionBackend for PgConnection { fn name(&self) -> &str { ::NAME @@ -84,6 +213,7 @@ impl AnyConnectionBackend for PgConnection { persistent: bool, arguments: Option, ) -> BoxStream<'_, sqlx_core::Result>> { + let query = maybe_rewrite_any_placeholders(query); let persistent = persistent && arguments.is_some(); let arguments = match arguments.map(AnyArguments::convert_into).transpose() { Ok(arguments) => arguments, @@ -110,6 +240,7 @@ impl AnyConnectionBackend for PgConnection { persistent: bool, arguments: Option, ) -> BoxFuture<'_, sqlx_core::Result>> { + let query = maybe_rewrite_any_placeholders(query); let persistent = persistent && arguments.is_some(); let arguments = arguments .map(AnyArguments::convert_into) @@ -133,6 +264,7 @@ impl AnyConnectionBackend for PgConnection { sql: SqlStr, _parameters: &[AnyTypeInfo], ) -> BoxFuture<'c, sqlx_core::Result> { + let sql = maybe_rewrite_any_placeholders(sql); Box::pin(async move { let statement = Executor::prepare_with(self, sql, &[]).await?; let column_names = statement.metadata.column_names.clone(); @@ -145,6 +277,7 @@ impl AnyConnectionBackend for PgConnection { &mut self, sql: SqlStr, ) -> BoxFuture<'_, sqlx_core::Result>> { + let sql = maybe_rewrite_any_placeholders(sql); Box::pin(async move { let describe = Executor::describe(self, sql).await?; @@ -252,3 +385,89 @@ fn map_result(res: PgQueryResult) -> AnyQueryResult { last_insert_id: None, } } + +#[cfg(test)] +mod tests { + use super::rewrite_any_placeholders; + + #[test] + fn no_placeholders_is_borrowed_unchanged() { + let sql = "SELECT * FROM foo"; + assert!(matches!( + rewrite_any_placeholders(sql), + std::borrow::Cow::Borrowed(s) if s == sql + )); + } + + #[test] + fn simple_placeholders_are_numbered_in_order() { + assert_eq!( + rewrite_any_placeholders("SELECT * FROM foo WHERE a = ? AND b = ?"), + "SELECT * FROM foo WHERE a = $1 AND b = $2" + ); + } + + #[test] + fn limit_offset_placeholders() { + assert_eq!( + rewrite_any_placeholders("SELECT * FROM foo LIMIT ? OFFSET ?"), + "SELECT * FROM foo LIMIT $1 OFFSET $2" + ); + } + + #[test] + fn question_mark_in_string_literal_is_untouched() { + assert_eq!( + rewrite_any_placeholders("SELECT * FROM foo WHERE q = 'is this ok?' AND a = ?"), + "SELECT * FROM foo WHERE q = 'is this ok?' AND a = $1" + ); + } + + #[test] + fn escaped_quote_in_string_literal() { + assert_eq!( + rewrite_any_placeholders("SELECT 'it''s a ? test' WHERE a = ?"), + "SELECT 'it''s a ? test' WHERE a = $1" + ); + } + + #[test] + fn question_mark_in_quoted_identifier_is_untouched() { + assert_eq!( + rewrite_any_placeholders(r#"SELECT "weird?col" FROM foo WHERE a = ?"#), + r#"SELECT "weird?col" FROM foo WHERE a = $1"# + ); + } + + #[test] + fn question_mark_in_dollar_quoted_string_is_untouched() { + assert_eq!( + rewrite_any_placeholders("SELECT $$has a ? in it$$ WHERE a = ?"), + "SELECT $$has a ? in it$$ WHERE a = $1" + ); + } + + #[test] + fn question_mark_in_tagged_dollar_quoted_string_is_untouched() { + assert_eq!( + rewrite_any_placeholders("SELECT $tag$has a ? in it$tag$ WHERE a = ?"), + "SELECT $tag$has a ? in it$tag$ WHERE a = $1" + ); + } + + #[test] + fn question_mark_in_line_comment_is_untouched() { + assert_eq!( + rewrite_any_placeholders("SELECT a -- is this ok?\nWHERE a = ?"), + "SELECT a -- is this ok?\nWHERE a = $1" + ); + } + + #[test] + fn question_mark_in_block_comment_is_untouched() { + assert_eq!( + rewrite_any_placeholders("SELECT a /* ok? */ WHERE a = ?"), + "SELECT a /* ok? */ WHERE a = $1" + ); + } +}