Skip to content

Commit 7d0e217

Browse files
dmcgowandmcgowan
committed
Fix filter errors
Prevent error messages from being output to stderr. Return illegal token when a quoted string is invalid and capture the error. Signed-off-by: Derek McGowan <[email protected]> (cherry picked from commit 3af3a76) Signed-off-by: Derek McGowan <[email protected]>
1 parent 095a1af commit 7d0e217

5 files changed

Lines changed: 103 additions & 20 deletions

File tree

‎filters/filter_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,16 @@ func TestFilters(t *testing.T) {
272272
input: "image~=,id?=?fbaq",
273273
errString: `filters: parse error: [image~= >|,|< id?=?fbaq]: expected value or quoted`,
274274
},
275+
{
276+
name: "FieldQuotedLiteralNotTerminated",
277+
input: "labels.ns/key==value",
278+
errString: `filters: parse error: [labels.ns >|/|< key==value]: quoted literal not terminated`,
279+
},
280+
{
281+
name: "ValueQuotedLiteralNotTerminated",
282+
input: "labels.key==/value",
283+
errString: `filters: parse error: [labels.key== >|/|< value]: quoted literal not terminated`,
284+
},
275285
} {
276286
t.Run(testcase.name, func(t *testing.T) {
277287
filter, err := Parse(testcase.input)

‎filters/parser.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ func (p *parser) field() (string, error) {
209209
return s, nil
210210
case tokenQuoted:
211211
return p.unquote(pos, s, false)
212+
case tokenIllegal:
213+
return "", p.mkerr(pos, p.scanner.err)
212214
}
213215

214216
return "", p.mkerr(pos, "expected field or quoted")
@@ -228,6 +230,8 @@ func (p *parser) operator() (operator, error) {
228230
default:
229231
return 0, p.mkerr(pos, "unsupported operator %q", s)
230232
}
233+
case tokenIllegal:
234+
return 0, p.mkerr(pos, p.scanner.err)
231235
}
232236

233237
return 0, p.mkerr(pos, `expected an operator ("=="|"!="|"~=")`)
@@ -241,6 +245,8 @@ func (p *parser) value(allowAltQuotes bool) (string, error) {
241245
return s, nil
242246
case tokenQuoted:
243247
return p.unquote(pos, s, allowAltQuotes)
248+
case tokenIllegal:
249+
return "", p.mkerr(pos, p.scanner.err)
244250
}
245251

246252
return "", p.mkerr(pos, "expected value or quoted")

‎filters/scanner.go‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package filters
1818

1919
import (
20-
"fmt"
2120
"unicode"
2221
"unicode/utf8"
2322
)
@@ -64,6 +63,7 @@ type scanner struct {
6463
pos int
6564
ppos int // bounds the current rune in the string
6665
value bool
66+
err string
6767
}
6868

6969
func (s *scanner) init(input string) {
@@ -82,12 +82,14 @@ func (s *scanner) next() rune {
8282
s.ppos += w
8383
if r == utf8.RuneError {
8484
if w > 0 {
85+
s.error("rune error")
8586
return tokenIllegal
8687
}
8788
return tokenEOF
8889
}
8990

9091
if r == 0 {
92+
s.error("unexpected null")
9193
return tokenIllegal
9294
}
9395

@@ -114,7 +116,9 @@ chomp:
114116
case ch == tokenEOF:
115117
case ch == tokenIllegal:
116118
case isQuoteRune(ch):
117-
s.scanQuoted(ch)
119+
if !s.scanQuoted(ch) {
120+
return pos, tokenIllegal, s.input[pos:s.ppos]
121+
}
118122
return pos, tokenQuoted, s.input[pos:s.ppos]
119123
case isSeparatorRune(ch):
120124
s.value = false
@@ -172,54 +176,64 @@ func (s *scanner) scanValue() {
172176
}
173177
}
174178

175-
func (s *scanner) scanQuoted(quote rune) {
179+
func (s *scanner) scanQuoted(quote rune) bool {
180+
var illegal bool
176181
ch := s.next() // read character after quote
177182
for ch != quote {
178183
if ch == '\n' || ch < 0 {
179-
s.error("literal not terminated")
180-
return
184+
s.error("quoted literal not terminated")
185+
return false
181186
}
182187
if ch == '\\' {
183-
ch = s.scanEscape(quote)
188+
var legal bool
189+
ch, legal = s.scanEscape(quote)
190+
if !legal {
191+
illegal = true
192+
}
184193
} else {
185194
ch = s.next()
186195
}
187196
}
197+
return !illegal
188198
}
189199

190-
func (s *scanner) scanEscape(quote rune) rune {
191-
ch := s.next() // read character after '/'
200+
func (s *scanner) scanEscape(quote rune) (ch rune, legal bool) {
201+
ch = s.next() // read character after '/'
192202
switch ch {
193203
case 'a', 'b', 'f', 'n', 'r', 't', 'v', '\\', quote:
194204
// nothing to do
195205
ch = s.next()
206+
legal = true
196207
case '0', '1', '2', '3', '4', '5', '6', '7':
197-
ch = s.scanDigits(ch, 8, 3)
208+
ch, legal = s.scanDigits(ch, 8, 3)
198209
case 'x':
199-
ch = s.scanDigits(s.next(), 16, 2)
210+
ch, legal = s.scanDigits(s.next(), 16, 2)
200211
case 'u':
201-
ch = s.scanDigits(s.next(), 16, 4)
212+
ch, legal = s.scanDigits(s.next(), 16, 4)
202213
case 'U':
203-
ch = s.scanDigits(s.next(), 16, 8)
214+
ch, legal = s.scanDigits(s.next(), 16, 8)
204215
default:
205-
s.error("illegal char escape")
216+
s.error("illegal escape sequence")
206217
}
207-
return ch
218+
return
208219
}
209220

210-
func (s *scanner) scanDigits(ch rune, base, n int) rune {
221+
func (s *scanner) scanDigits(ch rune, base, n int) (rune, bool) {
211222
for n > 0 && digitVal(ch) < base {
212223
ch = s.next()
213224
n--
214225
}
215226
if n > 0 {
216-
s.error("illegal char escape")
227+
s.error("illegal numeric escape sequence")
228+
return ch, false
217229
}
218-
return ch
230+
return ch, true
219231
}
220232

221233
func (s *scanner) error(msg string) {
222-
fmt.Println("error fixme", msg)
234+
if s.err == "" {
235+
s.err = msg
236+
}
223237
}
224238

225239
func digitVal(ch rune) int {

‎filters/scanner_test.go‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@ type tokenResult struct {
2525
pos int
2626
token token
2727
text string
28+
err string
2829
}
2930

3031
func (tr tokenResult) String() string {
32+
if tr.err != "" {
33+
return fmt.Sprintf("{pos: %v, token: %v, text: %q, err: %q}", tr.pos, tr.token, tr.text, tr.err)
34+
}
3135
return fmt.Sprintf("{pos: %v, token: %v, text: %q}", tr.pos, tr.token, tr.text)
3236
}
3337

@@ -171,7 +175,7 @@ func TestScanner(t *testing.T) {
171175
input: "foo\x00bar",
172176
expected: []tokenResult{
173177
{pos: 0, token: tokenField, text: "foo"},
174-
{pos: 3, token: tokenIllegal},
178+
{pos: 3, token: tokenIllegal, err: "unexpected null"},
175179
{pos: 4, token: tokenField, text: "bar"},
176180
{pos: 7, token: tokenEOF},
177181
},
@@ -271,6 +275,51 @@ func TestScanner(t *testing.T) {
271275
{pos: 23, token: tokenEOF},
272276
},
273277
},
278+
{
279+
name: "IllegalQuoted",
280+
input: "labels.containerd.io/key==value",
281+
expected: []tokenResult{
282+
{pos: 0, token: tokenField, text: "labels"},
283+
{pos: 6, token: tokenSeparator, text: "."},
284+
{pos: 7, token: tokenField, text: "containerd"},
285+
{pos: 17, token: tokenSeparator, text: "."},
286+
{pos: 18, token: tokenField, text: "io"},
287+
{pos: 20, token: tokenIllegal, text: "/key==value", err: "quoted literal not terminated"},
288+
{pos: 31, token: tokenEOF},
289+
},
290+
},
291+
{
292+
name: "IllegalQuotedWithNewLine",
293+
input: "labels.\"containerd.io\nkey\"==value",
294+
expected: []tokenResult{
295+
{pos: 0, token: tokenField, text: "labels"},
296+
{pos: 6, token: tokenSeparator, text: "."},
297+
{pos: 7, token: tokenIllegal, text: "\"containerd.io\n", err: "quoted literal not terminated"},
298+
{pos: 22, token: tokenField, text: "key"},
299+
{pos: 25, token: tokenIllegal, text: "\"==value", err: "quoted literal not terminated"},
300+
{pos: 33, token: tokenEOF},
301+
},
302+
},
303+
{
304+
name: "IllegalEscapeSequence",
305+
input: `labels."\g"`,
306+
expected: []tokenResult{
307+
{pos: 0, token: tokenField, text: "labels"},
308+
{pos: 6, token: tokenSeparator, text: "."},
309+
{pos: 7, token: tokenIllegal, text: `"\g"`, err: "illegal escape sequence"},
310+
{pos: 11, token: tokenEOF},
311+
},
312+
},
313+
{
314+
name: "IllegalNumericEscapeSequence",
315+
input: `labels."\xaz"`,
316+
expected: []tokenResult{
317+
{pos: 0, token: tokenField, text: "labels"},
318+
{pos: 6, token: tokenSeparator, text: "."},
319+
{pos: 7, token: tokenIllegal, text: `"\xaz"`, err: "illegal numeric escape sequence"},
320+
{pos: 13, token: tokenEOF},
321+
},
322+
},
274323
} {
275324
t.Run(testcase.name, func(t *testing.T) {
276325
var sc scanner
@@ -296,6 +345,9 @@ func TestScanner(t *testing.T) {
296345
if i >= len(testcase.expected) {
297346
t.Fatalf("too many tokens parsed")
298347
}
348+
if tok == tokenIllegal {
349+
tokv.err = sc.err
350+
}
299351

300352
if tokv != testcase.expected[i] {
301353
t.Fatalf("token unexpected: %v != %v", tokv, testcase.expected[i])
@@ -305,6 +357,7 @@ func TestScanner(t *testing.T) {
305357
if tok == tokenEOF {
306358
break
307359
}
360+
308361
}
309362

310363
// make sure we've eof'd

‎services/content/contentserver/contentserver.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func (s *service) List(req *api.ListContentRequest, session api.Content_ListServ
115115

116116
return nil
117117
}, req.Filters...); err != nil {
118-
return err
118+
return errdefs.ToGRPC(err)
119119
}
120120

121121
if len(buffer) > 0 {

0 commit comments

Comments
 (0)