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 <derek@mcgstyle.net>

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)

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")

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")
+			s.error("quoted literal not terminated")
-			return
+			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 {
+func (s *scanner) scanEscape(quote rune) (ch rune, legal bool) {
-	ch := s.next() // read character after '/'
+	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 {

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

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 {