In: skip ? inside SQL comments and string literals

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

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",