From 44f892fddca72e496e13e947cf4c28e2348bd2ba Mon Sep 17 00:00:00 2001
From: antialiasis <antialiasis@noreply.gitea.io>
Date: Sat, 26 Jun 2021 19:19:13 +0800
Subject: [PATCH] Ignore comments when deciding when to replace question marks.
 #1954 (#1955)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This should solve #1954 and adds some tests for it. I will note I'm not 100% clear on whether there are other edge cases that should be covered here. From what I understand the only standard SQL way to escape single quotes is to double them, which shouldn't cause any problems with this, but if some SQL flavors allow other kinds of escaping, for instance, that would probably need to be covered too for ideal results.

Co-authored-by: Hlín Önnudóttir <hlin@nanitor.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-on: https://gitea.com/xorm/xorm/pulls/1955
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: antialiasis <antialiasis@noreply.gitea.io>
Co-committed-by: antialiasis <antialiasis@noreply.gitea.io>
---
 dialects/filter.go      | 36 ++++++++++++++++++++++++--
 dialects/filter_test.go | 57 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/dialects/filter.go b/dialects/filter.go
index 2a36a731..bfe2e93e 100644
--- a/dialects/filter.go
+++ b/dialects/filter.go
@@ -23,13 +23,45 @@ type SeqFilter struct {
 func convertQuestionMark(sql, prefix string, start int) string {
 	var buf strings.Builder
 	var beginSingleQuote bool
+	var isLineComment bool
+	var isComment bool
+	var isMaybeLineComment bool
+	var isMaybeComment bool
+	var isMaybeCommentEnd bool
 	var index = start
 	for _, c := range sql {
-		if !beginSingleQuote && c == '?' {
+		if !beginSingleQuote && !isLineComment && !isComment && c == '?' {
 			buf.WriteString(fmt.Sprintf("%s%v", prefix, index))
 			index++
 		} else {
-			if c == '\'' {
+			if isMaybeLineComment {
+				if c == '-' {
+					isLineComment = true
+				}
+				isMaybeLineComment = false
+			} else if isMaybeComment {
+				if c == '*' {
+					isComment = true
+				}
+				isMaybeComment = false
+			} else if isMaybeCommentEnd {
+				if c == '/' {
+					isComment = false
+				}
+				isMaybeCommentEnd = false
+			} else if isLineComment {
+				if c == '\n' {
+					isLineComment = false
+				}
+			} else if isComment {
+				if c == '*' {
+					isMaybeCommentEnd = true
+				}
+			} else if !beginSingleQuote && c == '-' {
+				isMaybeLineComment = true
+			} else if !beginSingleQuote && c == '/' {
+				isMaybeComment = true
+			} else if c == '\'' {
 				beginSingleQuote = !beginSingleQuote
 			}
 			buf.WriteRune(c)
diff --git a/dialects/filter_test.go b/dialects/filter_test.go
index 7e2ef0a2..15050656 100644
--- a/dialects/filter_test.go
+++ b/dialects/filter_test.go
@@ -19,3 +19,60 @@ func TestSeqFilter(t *testing.T) {
 		assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1))
 	}
 }
+
+func TestSeqFilterLineComment(t *testing.T) {
+	var kases = map[string]string{
+		`SELECT *
+		FROM TABLE1
+		WHERE foo='bar'
+		AND a=? -- it's a comment
+		AND b=?`: `SELECT *
+		FROM TABLE1
+		WHERE foo='bar'
+		AND a=$1 -- it's a comment
+		AND b=$2`,
+		`SELECT *
+		FROM TABLE1
+		WHERE foo='bar'
+		AND a=? -- it's a comment?
+		AND b=?`: `SELECT *
+		FROM TABLE1
+		WHERE foo='bar'
+		AND a=$1 -- it's a comment?
+		AND b=$2`,
+		`SELECT *
+		FROM TABLE1
+		WHERE a=? -- it's a comment? and that's okay?
+		AND b=?`: `SELECT *
+		FROM TABLE1
+		WHERE a=$1 -- it's a comment? and that's okay?
+		AND b=$2`,
+	}
+	for sql, result := range kases {
+		assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1))
+	}
+}
+
+func TestSeqFilterComment(t *testing.T) {
+	var kases = map[string]string{
+		`SELECT *
+		FROM TABLE1
+		WHERE a=? /* it's a comment */
+		AND b=?`: `SELECT *
+		FROM TABLE1
+		WHERE a=$1 /* it's a comment */
+		AND b=$2`,
+		`SELECT /* it's a comment * ?
+		More comment on the next line! */ *
+		FROM TABLE1
+		WHERE a=? /**/
+		AND b=?`: `SELECT /* it's a comment * ?
+		More comment on the next line! */ *
+		FROM TABLE1
+		WHERE a=$1 /**/
+		AND b=$2`,
+	}
+	for sql, result := range kases {
+		assert.EqualValues(t, result, convertQuestionMark(sql, "$", 1))
+	}
+}