Skip to content

Commit c34ed70

Browse files
authored
Merge pull request #51 from ShiftLeftSecurity/preetam/support-escaped-placeholders
Support escaped '?', which may be JSONB operators
2 parents e63d7d8 + d3d16ca commit c34ed70

File tree

3 files changed

+76
-6
lines changed

3 files changed

+76
-6
lines changed

CONTRIBUTORS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
Horacio Duran <[email protected]>
22
William Cody Laeder <[email protected]>
3-
Alex Hornbake <[email protected]>
3+
Alex Hornbake <[email protected]>
4+
Preetam Jinka <[email protected]>

db/chain/placeholders.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,25 @@ func MarksToPlaceholders(q string, args []interface{}) (string, []interface{}, e
7676
args = otherArgs
7777

7878
// TODO: make this a bit less ugly
79-
// TODO: identify escaped question marks
8079
// TODO: use an actual parser <3
8180
// TODO: structure query segments around SQL-Standard AST
8281
queryWithArgs := &strings.Builder{}
8382
argCounter := 1
8483
argPositioner := 0
8584
expandedArgs := []interface{}{}
86-
for _, queryChar := range q {
85+
skip := false
86+
for i, queryChar := range q {
87+
if skip {
88+
skip = false
89+
continue
90+
}
91+
92+
if queryChar == '\\' && i < len(q)-1 && q[i+1] == '?' {
93+
// Escaped '?'
94+
queryWithArgs.WriteRune('?')
95+
skip = true
96+
continue
97+
}
8798
if queryChar == '?' {
8899
arg := args[argPositioner]
89100
switch reflect.TypeOf(arg).Kind() {
@@ -126,7 +137,6 @@ func MarksToPlaceholders(q string, args []interface{}) (string, []interface{}, e
126137

127138
// PlaceholdersToPositional converts ? in a query into $<argument number> which postgres expects
128139
func PlaceholdersToPositional(q *strings.Builder, argCount int) (*strings.Builder, int, error) {
129-
// TODO: identify escaped question marks
130140
// TODO: use an actual parser <3
131141
// TODO: structure query segments around SQL-Standard AST
132142
newQ := &strings.Builder{}
@@ -136,8 +146,22 @@ func PlaceholdersToPositional(q *strings.Builder, argCount int) (*strings.Builde
136146
newQ.Grow(renderedLength - newQ.Len())
137147
}
138148

149+
queryString := q.String()
139150
argCounter := 1
140-
for _, queryChar := range q.String() {
151+
skip := false
152+
for i, queryChar := range queryString {
153+
if skip {
154+
skip = false
155+
continue
156+
}
157+
158+
if queryChar == '\\' && i < len(queryString)-1 && queryString[i+1] == '?' {
159+
// Escaped '?'
160+
newQ.WriteRune('?')
161+
skip = true
162+
continue
163+
}
164+
141165
if queryChar == '?' {
142166
newQ.WriteRune('$')
143167
newQ.WriteString(strconv.Itoa(argCounter))

db/chain/placeholders_test.go

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package chain
22

3-
import "testing"
3+
import (
4+
"fmt"
5+
"testing"
6+
)
47

58
func Test_digitSize(t *testing.T) {
69
type args struct {
@@ -56,3 +59,45 @@ func Test_digitSize(t *testing.T) {
5659
})
5760
}
5861
}
62+
63+
func TestPlaceholderEscaping(t *testing.T) {
64+
tests := []struct {
65+
q string
66+
want string
67+
args []interface{}
68+
}{
69+
{
70+
q: "? = 1",
71+
want: "$1 = 1",
72+
args: []interface{}{1},
73+
},
74+
{
75+
q: "\\? = 1",
76+
want: "? = 1",
77+
args: []interface{}{},
78+
},
79+
{
80+
q: "? = ? AND \\? = 1",
81+
want: "$1 = $2 AND ? = 1",
82+
args: []interface{}{1, 1},
83+
},
84+
{
85+
q: `'["a", "b"]'::jsonb \?& array['a', 'b']`,
86+
want: `'["a", "b"]'::jsonb ?& array['a', 'b']`,
87+
args: []interface{}{},
88+
},
89+
{
90+
q: `'["a", "b"]'::jsonb \?& array[?]`,
91+
want: `'["a", "b"]'::jsonb ?& array[$1]`,
92+
args: []interface{}{"a"},
93+
},
94+
}
95+
for i, tt := range tests {
96+
t.Run(fmt.Sprint(i), func(t *testing.T) {
97+
result, _, _ := MarksToPlaceholders(tt.q, tt.args)
98+
if result != tt.want {
99+
t.Errorf("got %v, want %v", result, tt.want)
100+
}
101+
})
102+
}
103+
}

0 commit comments

Comments
 (0)