More strict filter syntax checking

Current filter factory rules are very relaxed, anything that doesn't match filter syntax perfectly is passed down to the fuzzy filter, that happily accepts every input and doesn't complain. This change makes syntax checks more strict, we explicitly look for roughly <anything><any of the chars used as operators><anything> (in reality we match the first <anything> used filter specific regex, but that can still be any regex) and only if evaluated filter expression doesn't match this regex we pass it to the fuzzy filter. Once we have a match we check if operator is supported by the filter and we're more strict there, in case of a===b we currently accept = as the operator and ==b as the value, now operator will be === (as we match for any of the operator chars in use) and fail once that doesn't match.

filters/filter.go

@@ -3,7 +3,6 @@ package filters
 import (
 	"fmt"
 	"regexp"
-	"strings"
 
 	"github.com/cloudflare/unsee/models"
 )
@@ -55,10 +54,13 @@ type newFilterFactory func() FilterT
 // expression will be parsed and best filter implementation and value matcher
 // will be selected
 func NewFilter(expression string) FilterT {
-	for _, fc := range AllFilters {
+	invalid := alwaysInvalidFilter{}
+	invalid.init("", nil, expression, false, expression)
 
-		reOperators := strings.Join(fc.SupportedOperators, "|")
-		reExp := fmt.Sprintf("^(?P<matched>(%s))(?P<operator>(%s))(?P<value>(.+))", fc.Label, reOperators)
+	for _, fc := range AllFilters {
+		f := fc.Factory()
+
+		reExp := fmt.Sprintf("^(?P<matched>(%s))(?P<operator>(%s))(?P<value>(.*))", fc.Label, matcherRegex)
 		re := regexp.MustCompile(reExp)
 		match := re.FindStringSubmatch(expression)
 		result := make(map[string]string)
@@ -68,33 +70,44 @@ func NewFilter(expression string) FilterT {
 			}
 		}
 
-		if matched, found := result["matched"]; found {
-
-			f := fc.Factory()
-			if fc.IsSimple {
-				matcher, err := newMatcher(regexpOperator)
-				if err != nil {
-					f.init("", nil, expression, true, expression)
-				} else {
-					f.init("", &matcher, expression, true, expression)
-				}
-				return f
-			} else if operator, found := result["operator"]; found {
-				var err error
-				matcher, err := newMatcher(operator)
-				if err != nil {
-					f.init(matched, nil, expression, false, "")
-				} else {
-					if value, found := result["value"]; found {
-						f.init(matched, &matcher, expression, true, value)
-						return f
-					}
-					f.init(matched, &matcher, expression, false, "")
-				}
+		matched, found := result["matched"]
+		if !found && fc.IsSimple {
+			matcher, err := newMatcher(regexpOperator)
+			if err != nil {
+				f.init("", nil, expression, false, expression)
+			} else {
+				f.init("", &matcher, expression, true, expression)
 			}
+			return f
 		}
+		if !found {
+			continue
+		}
+		if value, ok := result["value"]; !ok || value == "" {
+			// value group not found in the expression
+			// example: 'label=''
+			return &invalid
+		}
+		operator, found := result["operator"]
+		if !found {
+			// used operator is not supported by the filter
+			// example: @limit=~0
+			return &invalid
+		}
+		if !stringInSlice(fc.SupportedOperators, operator) {
+			return &invalid
+		}
+		matcher, err := newMatcher(operator)
+		if err != nil {
+			f.init(matched, nil, expression, false, "")
+		} else {
+			if value, found := result["value"]; found {
+				f.init(matched, &matcher, expression, true, value)
+				return f
+			}
+			f.init(matched, &matcher, expression, false, "")
+		}
+
 	}
-	f := alwaysInvalidFilter{}
-	f.init("", nil, expression, false, expression)
-	return &f
+	return &invalid
 }

filters/registry.go

@@ -9,6 +9,12 @@ const (
 	negativeRegexOperator string = "!~"
 )
 
+// this needs to be hand crafted because any of the supported operator chars
+// should be considered part of the operator expression
+// this is needed to catch errors in operators, for example:
+// a===b should yield an error
+var matcherRegex = "[=!<>~]+"
+
 var matcherConfig = map[string]matcherT{
 	equalOperator:         &equalMatcher{},
 	notEqualOperator:      &notEqualMatcher{},

filters/slices.go

@@ -0,0 +1,10 @@
+package filters
+
+func stringInSlice(stringArray []string, value string) bool {
+	for _, s := range stringArray {
+		if s == value {
+			return true
+		}
+	}
+	return false
+}