From a164a7c3bb4f6466537fc10d438b55d7704e198a Mon Sep 17 00:00:00 2001 From: Charlie Tonneslan Date: Tue, 12 May 2026 12:50:08 -0400 Subject: [PATCH] In: skip ? inside SQL comments and string literals sqlx.In() was using strings.IndexByte to find ? placeholders, which has no concept of SQL syntax. A ? inside a -- line comment, a /* block comment */, or a single-quoted string literal would be counted as a bind variable, causing "number of bindVars exceeds arguments" errors even when the query was correct. Fix adds a nextBindVar helper that walks the query string while tracking comment and string state, returning only the index of a ? that's actually a bind variable. The In function now uses it in place of IndexByte. Closes #961 --- bind.go | 44 +++++++++++++++++++++++++++++++++++++++++- bind_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/bind.go b/bind.go index e6980392b..1a5eb4010 100644 --- a/bind.go +++ b/bind.go @@ -136,6 +136,48 @@ func asSliceForIn(i interface{}) (v reflect.Value, ok bool) { return v, true } +// nextBindVar returns the index of the next '?' in s that is not inside a SQL +// comment (-- line comment or /* block comment */) or a single-quoted string +// literal. Returns -1 if none is found. +func nextBindVar(s string) int { + for i := 0; i < len(s); i++ { + switch s[i] { + case '-': + if i+1 < len(s) && s[i+1] == '-' { + // line comment: skip to end of line + for i < len(s) && s[i] != '\n' { + i++ + } + } + case '/': + if i+1 < len(s) && s[i+1] == '*' { + // block comment: skip to */ + i += 2 + for i+1 < len(s) && !(s[i] == '*' && s[i+1] == '/') { + i++ + } + i++ // skip the closing / + } + case '\'': + // single-quoted string: skip to the closing quote, handling '' + i++ + for i < len(s) { + if s[i] == '\'' { + if i+1 < len(s) && s[i+1] == '\'' { + i++ // escaped quote + } else { + break + } + } + i++ + } + case '?': + return i + } + } + return -1 +} + // In expands slice values in args, returning the modified query string // and a new arg list that can be executed by a database. The `query` should // use the `?` bindVar. The return value uses the `?` bindVar. @@ -198,7 +240,7 @@ func In(query string, args ...interface{}) (string, []interface{}, error) { var arg, offset int - for i := strings.IndexByte(query[offset:], '?'); i != -1; i = strings.IndexByte(query[offset:], '?') { + for i := nextBindVar(query[offset:]); i != -1; i = nextBindVar(query[offset:]) { if arg >= len(meta) { // if an argument wasn't passed, lets return an error; this is // not actually how database/sql Exec/Query works, but since we are diff --git a/bind_test.go b/bind_test.go index dfa590ecf..1237b06d3 100644 --- a/bind_test.go +++ b/bind_test.go @@ -2,6 +2,7 @@ package sqlx import ( "math/rand" + "strings" "testing" ) @@ -40,6 +41,59 @@ BenchmarkBindSpeed/old-4 100000000 11.0 ns/op BenchmarkBindSpeed/new-4 42535839 27.5 ns/op */ +// TestInSkipsCommentBindVars ensures that ? inside SQL comments and string +// literals aren't mistaken for bind variable placeholders by In(). +func TestInSkipsCommentBindVars(t *testing.T) { + type tc struct { + name string + query string + args []interface{} + wantN int // expected number of ? placeholders / args in result + } + + tests := []tc{ + { + name: "line comment with ?", + query: "SELECT * -- should we filter by id?\nFROM foo WHERE id IN (?)", + args: []interface{}{[]int{1, 2, 3}}, + wantN: 3, + }, + { + name: "block comment with ?", + query: "SELECT * /* what about id=? */ FROM foo WHERE id IN (?)", + args: []interface{}{[]int{1, 2}}, + wantN: 2, + }, + { + name: "string literal with ?", + query: "SELECT id, '?' AS q FROM foo WHERE id IN (?)", + args: []interface{}{[]int{10, 20}}, + wantN: 2, + }, + { + name: "multiple comments and real bind var", + query: "SELECT * -- filter?\nFROM foo WHERE x = ? AND id IN (?)", + args: []interface{}{"bar", []int{1, 2}}, + wantN: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + q, a, err := In(tt.query, tt.args...) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(a) != tt.wantN { + t.Errorf("expected %d args, got %d (%v)", tt.wantN, len(a), a) + } + // Count only the real bind vars that In() emitted, not any in comments. + // We can't easily strip comments here, so just verify arg count above. + _ = strings.Count(q, "?") + }) + } +} + func BenchmarkBindSpeed(b *testing.B) { testDrivers := []string{ "postgres", "pgx", "mysql", "sqlite3", "ora", "sqlserver",