Enhance interpolateParams to correctly handle placeholders in queries with comments, strings, and backticks.

* Add `findParamPositions` to identify real parameter positions * Update and expand related tests.

Author: rusher

connection.go

@@ -172,7 +172,7 @@ func (mc *mysqlConn) close(){
 }
 
 // Closes the network connection and unsets internal variables. Do not call this
-// function after successfully authentication, call Close instead. This function
+// function after successful authentication, call Close instead. This function
 // is called before auth or on auth failure because MySQL will have already
 // closed the network connection.
 func (mc*mysqlConn) cleanup(){
@@ -245,9 +245,106 @@ func (mc *mysqlConn) Prepare(query string) (driver.Stmt, error){
 returnstmt, err
 }
 
+// findParamPositions returns the positions of real parameter holders ('?') in the query, ignoring those in comments, strings, or backticks.
+funcfindParamPositions(querystring) []int{
+const (
+stateNormal=iota
+stateString
+stateEscape
+stateEOLComment
+stateSlashStarComment
+stateBacktick
+ )
+
+var (
+QUOTE_BYTE=byte('\'')
+DBL_QUOTE_BYTE=byte('"')
+BACKSLASH_BYTE=byte('\\')
+QUESTION_MARK_BYTE=byte('?')
+SLASH_BYTE=byte('/')
+STAR_BYTE=byte('*')
+HASH_BYTE=byte('#')
+MINUS_BYTE=byte('-')
+LINE_FEED_BYTE=byte('\n')
+RADICAL_BYTE=byte('`')
+ )
+
+paramPositions:=make([]int, 0)
+state:=stateNormal
+singleQuotes:=false
+lastChar:=byte(0)
+lenq:=len(query)
+fori:=0; i<lenq; i++{
+currentChar:=query[i]
+ifstate==stateEscape&&!((currentChar==QUOTE_BYTE&&singleQuotes) || (currentChar==DBL_QUOTE_BYTE&&!singleQuotes)){
+state=stateString
+lastChar=currentChar
+continue
+ }
+switchcurrentChar{
+caseSTAR_BYTE:
+ifstate==stateNormal&&lastChar==SLASH_BYTE{
+state=stateSlashStarComment
+ }
+caseSLASH_BYTE:
+ifstate==stateSlashStarComment&&lastChar==STAR_BYTE{
+state=stateNormal
+ } elseifstate==stateNormal&&lastChar==SLASH_BYTE{
+state=stateEOLComment
+ }
+caseHASH_BYTE:
+ifstate==stateNormal{
+state=stateEOLComment
+ }
+caseMINUS_BYTE:
+ifstate==stateNormal&&lastChar==MINUS_BYTE{
+state=stateEOLComment
+ }
+caseLINE_FEED_BYTE:
+ifstate==stateEOLComment{
+state=stateNormal
+ }
+caseDBL_QUOTE_BYTE:
+ifstate==stateNormal{
+state=stateString
+singleQuotes=false
+ } elseifstate==stateString&&!singleQuotes{
+state=stateNormal
+ } elseifstate==stateEscape{
+state=stateString
+ }
+caseQUOTE_BYTE:
+ifstate==stateNormal{
+state=stateString
+singleQuotes=true
+ } elseifstate==stateString&&singleQuotes{
+state=stateNormal
+ } elseifstate==stateEscape{
+state=stateString
+ }
+caseBACKSLASH_BYTE:
+ifstate==stateString{
+state=stateEscape
+ }
+caseQUESTION_MARK_BYTE:
+ifstate==stateNormal{
+paramPositions=append(paramPositions, i)
+ }
+caseRADICAL_BYTE:
+ifstate==stateBacktick{
+state=stateNormal
+ } elseifstate==stateNormal{
+state=stateBacktick
+ }
+ }
+lastChar=currentChar
+ }
+returnparamPositions
+}
+
 func (mc*mysqlConn) interpolateParams(querystring, args []driver.Value) (string, error){
-// Number of ? should be same to len(args)
-ifstrings.Count(query, "?") !=len(args){
+paramPositions:=findParamPositions(query)
+iflen(paramPositions) !=len(args){
 return"", driver.ErrSkip
  }
 
@@ -261,21 +358,16 @@ func (mc *mysqlConn) interpolateParams(query string, args []driver.Value) (strin
  }
 buf=buf[:0]
 argPos:=0
+lastIdx:=0
 
-fori:=0; i<len(query); i++{
-q:=strings.IndexByte(query[i:], '?')
-ifq==-1{
-buf=append(buf, query[i:]...)
-break
- }
-buf=append(buf, query[i:i+q]...)
-i+=q
-
+for_, qmIdx:=rangeparamPositions{
+buf=append(buf, query[lastIdx:qmIdx]...)
 arg:=args[argPos]
 argPos++
 
 ifarg==nil{
 buf=append(buf, "NULL"...)
+lastIdx=qmIdx+1
 continue
  }
 
@@ -339,7 +431,9 @@ func (mc *mysqlConn) interpolateParams(query string, args []driver.Value) (strin
 iflen(buf)+4>mc.maxAllowedPacket{
 return"", driver.ErrSkip
  }
+lastIdx=qmIdx+1
  }
+buf=append(buf, query[lastIdx:]...)
 ifargPos!=len(args){
 return"", driver.ErrSkip
  }

connection_test.go

@@ -79,24 +79,6 @@ func TestInterpolateParamsTooManyPlaceholders(t *testing.T){
  }
 }
 
-// We don't support placeholder in string literal for now.
-// https://github.com/go-sql-driver/mysql/pull/490
-funcTestInterpolateParamsPlaceholderInString(t*testing.T){
-mc:=&mysqlConn{
-buf: newBuffer(),
-maxAllowedPacket: maxPacketSize,
-cfg: &Config{
-InterpolateParams: true,
- },
- }
-
-q, err:=mc.interpolateParams("SELECT 'abc?xyz',?", []driver.Value{int64(42)})
-// When InterpolateParams support string literal, this should return `"SELECT 'abc?xyz', 42`
-iferr!=driver.ErrSkip{
-t.Errorf("Expected err=driver.ErrSkip, got err=%#v, q=%#v", err, q)
- }
-}
-
 funcTestInterpolateParamsUint64(t*testing.T){
 mc:=&mysqlConn{
 buf: newBuffer(),
@@ -204,3 +186,51 @@ func (bc badConnection) Write(b []byte) (n int, err error){
 func (bcbadConnection) Close() error{
 returnnil
 }
+
+funcTestInterpolateParamsWithComments(t*testing.T){
+mc:=&mysqlConn{
+buf: newBuffer(),
+maxAllowedPacket: maxPacketSize,
+cfg: &Config{
+InterpolateParams: true,
+ },
+ }
+
+tests:= []struct{
+querystring
+args []driver.Value
+expectedstring
+shouldSkipbool
+ }{
+// ? in single-line comment (--) should not be replaced
+{"SELECT 1 -- ?\n, ?", []driver.Value{int64(42)}, "SELECT 1 -- ?\n, 42", false},
+// ? in single-line comment (#) should not be replaced
+{"SELECT 1 # ?\n, ?", []driver.Value{int64(42)}, "SELECT 1 # ?\n, 42", false},
+// ? in multi-line comment should not be replaced
+{"SELECT /* ? */ ?", []driver.Value{int64(42)}, "SELECT /* ? */ 42", false},
+// ? in string literal should not be replaced
+{"SELECT '?', ?", []driver.Value{int64(42)}, "SELECT '?', 42", false},
+// ? in backtick identifier should not be replaced
+{"SELECT `?`, ?", []driver.Value{int64(42)}, "SELECT `?`, 42", false},
+// Multiple comments and real placeholders
+{"SELECT ? -- comment ?\n, ? /* ? */ , ? # ?\n, ?", []driver.Value{int64(1), int64(2), int64(3)}, "SELECT 1 -- comment ?\n, 2 /* ? */ , 3 # ?\n, ?", true},
+ }
+
+fori, test:=rangetests{
+
+q, err:=mc.interpolateParams(test.query, test.args)
+iftest.shouldSkip{
+iferr!=driver.ErrSkip{
+t.Errorf("Test %d: Expected driver.ErrSkip, got err=%#v, q=%#v", i, err, q)
+ }
+continue
+ }
+iferr!=nil{
+t.Errorf("Test %d: Expected err=nil, got %#v", i, err)
+continue
+ }
+ifq!=test.expected{
+t.Errorf("Test %d: Expected: %q\nGot: %q", i, test.expected, q)
+ }
+ }
+}