diff --git a/filters/filter_test.go b/filters/filter_test.go index d24ebb6e5..46c7d55e7 100644 --- a/filters/filter_test.go +++ b/filters/filter_test.go @@ -272,6 +272,16 @@ func TestFilters(t *testing.T) { input: "image~=,id?=?fbaq", errString: `filters: parse error: [image~= >|,|< id?=?fbaq]: expected value or quoted`, }, + { + name: "FieldQuotedLiteralNotTerminated", + input: "labels.ns/key==value", + errString: `filters: parse error: [labels.ns >|/|< key==value]: quoted literal not terminated`, + }, + { + name: "ValueQuotedLiteralNotTerminated", + input: "labels.key==/value", + errString: `filters: parse error: [labels.key== >|/|< value]: quoted literal not terminated`, + }, } { t.Run(testcase.name, func(t *testing.T) { filter, err := Parse(testcase.input) diff --git a/filters/parser.go b/filters/parser.go index 2be23574e..0825d668c 100644 --- a/filters/parser.go +++ b/filters/parser.go @@ -209,6 +209,8 @@ func (p *parser) field() (string, error) { return s, nil case tokenQuoted: return p.unquote(pos, s, false) + case tokenIllegal: + return "", p.mkerr(pos, p.scanner.err) } return "", p.mkerr(pos, "expected field or quoted") @@ -228,6 +230,8 @@ func (p *parser) operator() (operator, error) { default: return 0, p.mkerr(pos, "unsupported operator %q", s) } + case tokenIllegal: + return 0, p.mkerr(pos, p.scanner.err) } return 0, p.mkerr(pos, `expected an operator ("=="|"!="|"~=")`) @@ -241,6 +245,8 @@ func (p *parser) value(allowAltQuotes bool) (string, error) { return s, nil case tokenQuoted: return p.unquote(pos, s, allowAltQuotes) + case tokenIllegal: + return "", p.mkerr(pos, p.scanner.err) } return "", p.mkerr(pos, "expected value or quoted") diff --git a/filters/scanner.go b/filters/scanner.go index 45c52606d..6a485467b 100644 --- a/filters/scanner.go +++ b/filters/scanner.go @@ -17,7 +17,6 @@ package filters import ( - "fmt" "unicode" "unicode/utf8" ) @@ -64,6 +63,7 @@ type scanner struct { pos int ppos int // bounds the current rune in the string value bool + err string } func (s *scanner) init(input string) { @@ -82,12 +82,14 @@ func (s *scanner) next() rune { s.ppos += w if r == utf8.RuneError { if w > 0 { + s.error("rune error") return tokenIllegal } return tokenEOF } if r == 0 { + s.error("unexpected null") return tokenIllegal } @@ -114,7 +116,9 @@ chomp: case ch == tokenEOF: case ch == tokenIllegal: case isQuoteRune(ch): - s.scanQuoted(ch) + if !s.scanQuoted(ch) { + return pos, tokenIllegal, s.input[pos:s.ppos] + } return pos, tokenQuoted, s.input[pos:s.ppos] case isSeparatorRune(ch): s.value = false @@ -172,54 +176,64 @@ func (s *scanner) scanValue() { } } -func (s *scanner) scanQuoted(quote rune) { +func (s *scanner) scanQuoted(quote rune) bool { + var illegal bool ch := s.next() // read character after quote for ch != quote { if ch == '\n' || ch < 0 { - s.error("literal not terminated") - return + s.error("quoted literal not terminated") + return false } if ch == '\\' { - ch = s.scanEscape(quote) + var legal bool + ch, legal = s.scanEscape(quote) + if !legal { + illegal = true + } } else { ch = s.next() } } + return !illegal } -func (s *scanner) scanEscape(quote rune) rune { - ch := s.next() // read character after '/' +func (s *scanner) scanEscape(quote rune) (ch rune, legal bool) { + ch = s.next() // read character after '/' switch ch { case 'a', 'b', 'f', 'n', 'r', 't', 'v', '\\', quote: // nothing to do ch = s.next() + legal = true case '0', '1', '2', '3', '4', '5', '6', '7': - ch = s.scanDigits(ch, 8, 3) + ch, legal = s.scanDigits(ch, 8, 3) case 'x': - ch = s.scanDigits(s.next(), 16, 2) + ch, legal = s.scanDigits(s.next(), 16, 2) case 'u': - ch = s.scanDigits(s.next(), 16, 4) + ch, legal = s.scanDigits(s.next(), 16, 4) case 'U': - ch = s.scanDigits(s.next(), 16, 8) + ch, legal = s.scanDigits(s.next(), 16, 8) default: - s.error("illegal char escape") + s.error("illegal escape sequence") } - return ch + return } -func (s *scanner) scanDigits(ch rune, base, n int) rune { +func (s *scanner) scanDigits(ch rune, base, n int) (rune, bool) { for n > 0 && digitVal(ch) < base { ch = s.next() n-- } if n > 0 { - s.error("illegal char escape") + s.error("illegal numeric escape sequence") + return ch, false } - return ch + return ch, true } func (s *scanner) error(msg string) { - fmt.Println("error fixme", msg) + if s.err == "" { + s.err = msg + } } func digitVal(ch rune) int { diff --git a/filters/scanner_test.go b/filters/scanner_test.go index cefc12026..b0e98fd5c 100644 --- a/filters/scanner_test.go +++ b/filters/scanner_test.go @@ -25,9 +25,13 @@ type tokenResult struct { pos int token token text string + err string } func (tr tokenResult) String() string { + if tr.err != "" { + return fmt.Sprintf("{pos: %v, token: %v, text: %q, err: %q}", tr.pos, tr.token, tr.text, tr.err) + } return fmt.Sprintf("{pos: %v, token: %v, text: %q}", tr.pos, tr.token, tr.text) } @@ -171,7 +175,7 @@ func TestScanner(t *testing.T) { input: "foo\x00bar", expected: []tokenResult{ {pos: 0, token: tokenField, text: "foo"}, - {pos: 3, token: tokenIllegal}, + {pos: 3, token: tokenIllegal, err: "unexpected null"}, {pos: 4, token: tokenField, text: "bar"}, {pos: 7, token: tokenEOF}, }, @@ -271,6 +275,51 @@ func TestScanner(t *testing.T) { {pos: 23, token: tokenEOF}, }, }, + { + name: "IllegalQuoted", + input: "labels.containerd.io/key==value", + expected: []tokenResult{ + {pos: 0, token: tokenField, text: "labels"}, + {pos: 6, token: tokenSeparator, text: "."}, + {pos: 7, token: tokenField, text: "containerd"}, + {pos: 17, token: tokenSeparator, text: "."}, + {pos: 18, token: tokenField, text: "io"}, + {pos: 20, token: tokenIllegal, text: "/key==value", err: "quoted literal not terminated"}, + {pos: 31, token: tokenEOF}, + }, + }, + { + name: "IllegalQuotedWithNewLine", + input: "labels.\"containerd.io\nkey\"==value", + expected: []tokenResult{ + {pos: 0, token: tokenField, text: "labels"}, + {pos: 6, token: tokenSeparator, text: "."}, + {pos: 7, token: tokenIllegal, text: "\"containerd.io\n", err: "quoted literal not terminated"}, + {pos: 22, token: tokenField, text: "key"}, + {pos: 25, token: tokenIllegal, text: "\"==value", err: "quoted literal not terminated"}, + {pos: 33, token: tokenEOF}, + }, + }, + { + name: "IllegalEscapeSequence", + input: `labels."\g"`, + expected: []tokenResult{ + {pos: 0, token: tokenField, text: "labels"}, + {pos: 6, token: tokenSeparator, text: "."}, + {pos: 7, token: tokenIllegal, text: `"\g"`, err: "illegal escape sequence"}, + {pos: 11, token: tokenEOF}, + }, + }, + { + name: "IllegalNumericEscapeSequence", + input: `labels."\xaz"`, + expected: []tokenResult{ + {pos: 0, token: tokenField, text: "labels"}, + {pos: 6, token: tokenSeparator, text: "."}, + {pos: 7, token: tokenIllegal, text: `"\xaz"`, err: "illegal numeric escape sequence"}, + {pos: 13, token: tokenEOF}, + }, + }, } { t.Run(testcase.name, func(t *testing.T) { var sc scanner @@ -296,6 +345,9 @@ func TestScanner(t *testing.T) { if i >= len(testcase.expected) { t.Fatalf("too many tokens parsed") } + if tok == tokenIllegal { + tokv.err = sc.err + } if tokv != testcase.expected[i] { t.Fatalf("token unexpected: %v != %v", tokv, testcase.expected[i]) @@ -305,6 +357,7 @@ func TestScanner(t *testing.T) { if tok == tokenEOF { break } + } // make sure we've eof'd diff --git a/services/content/contentserver/contentserver.go b/services/content/contentserver/contentserver.go index 33707303e..7b6efdb3a 100644 --- a/services/content/contentserver/contentserver.go +++ b/services/content/contentserver/contentserver.go @@ -115,7 +115,7 @@ func (s *service) List(req *api.ListContentRequest, session api.Content_ListServ return nil }, req.Filters...); err != nil { - return err + return errdefs.ToGRPC(err) } if len(buffer) > 0 {