matchers: Parse Matcher now expects consistent enclosing with quotes. (#2632)

Fixes https://github.com/prometheus/alertmanager/issues/2630

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

pkg/labels/parse.go

@@ -22,11 +22,9 @@ import (
 )
 
 var (
-	re = regexp.MustCompile(
+	// '=~' has to come before '=' because otherwise only the '='
-		// '=~' has to come before '=' because otherwise only the '='
+	// will be consumed, and the '~' will be part of the 3rd token.
-		// will be consumed, and the '~' will be part of the 3rd token.
+	re      = regexp.MustCompile(`^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`)
-		`^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`,
-	)
 	typeMap = map[string]MatchType{
 		"=":  MatchEqual,
 		"!=": MatchNotEqual,
@@ -116,20 +114,26 @@ func ParseMatchers(s string) ([]*Matcher, error) {
 // character). However, literal line feed characters are tolerated, as are
 // single '\' characters not followed by '\', 'n', or '"'. They act as a literal
 // backslash in that case.
-func ParseMatcher(s string) (*Matcher, error) {
+func ParseMatcher(s string) (_ *Matcher, err error) {
 	ms := re.FindStringSubmatch(s)
 	if len(ms) == 0 {
 		return nil, errors.Errorf("bad matcher format: %s", s)
 	}
 
 	var (
-		rawValue = strings.TrimPrefix(ms[3], "\"")
+		rawValue            = ms[3]
-		value    strings.Builder
+		value               strings.Builder
-		escaped  bool
+		escaped             bool
+		expectTrailingQuote bool
 	)
 
+	if rawValue[0] == '"' {
+		rawValue = strings.TrimPrefix(rawValue, "\"")
+		expectTrailingQuote = true
+	}
+
 	if !utf8.ValidString(rawValue) {
-		return nil, errors.Errorf("matcher value not valid UTF-8: %s", rawValue)
+		return nil, errors.Errorf("matcher value not valid UTF-8: %s", ms[3])
 	}
 
 	// Unescape the rawValue:
@@ -157,15 +161,18 @@ func ParseMatcher(s string) (*Matcher, error) {
 			// '\' encountered as last byte. Treat it as literal.
 			value.WriteByte('\\')
 		case '"':
-			if i < len(rawValue)-1 { // Otherwise this is a trailing quote.
+			if !expectTrailingQuote || i < len(rawValue)-1 {
-				return nil, errors.Errorf(
+				return nil, errors.Errorf("matcher value contains unescaped double quote: %s", ms[3])
-					"matcher value contains unescaped double quote: %s", rawValue,
-				)
 			}
+			expectTrailingQuote = false
 		default:
 			value.WriteRune(r)
 		}
 	}
 
+	if expectTrailingQuote {
+		return nil, errors.Errorf("matcher value contains unescaped double quote: %s", ms[3])
+	}
+
 	return NewMatcher(typeMap[ms[2]], ms[1], value.String())
 }

pkg/labels/parse_test.go

@@ -19,7 +19,7 @@ import (
 )
 
 func TestMatchers(t *testing.T) {
-	testCases := []struct {
+	for _, tc := range []struct {
 		input string
 		want  []*Matcher
 		err   string
@@ -173,7 +173,7 @@ func TestMatchers(t *testing.T) {
 			}(),
 		},
 		{
-			input: `trickier==\\=\=\""`,
+			input: `trickier==\\=\=\"`,
 			want: func() []*Matcher {
 				ms := []*Matcher{}
 				m, _ := NewMatcher(MatchEqual, "trickier", `=\=\="`)
@@ -189,6 +189,18 @@ func TestMatchers(t *testing.T) {
 				return append(ms, m, m2)
 			}(),
 		},
+		{
+			input: `job="value`,
+			err:   `matcher value contains unescaped double quote: "value`,
+		},
+		{
+			input: `job=value"`,
+			err:   `matcher value contains unescaped double quote: value"`,
+		},
+		{
+			input: `trickier==\\=\=\""`,
+			err:   `matcher value contains unescaped double quote: =\\=\=\""`,
+		},
 		{
 			input: `contains_unescaped_quote = foo"bar`,
 			err:   `matcher value contains unescaped double quote: foo"bar`,
@@ -201,22 +213,47 @@ func TestMatchers(t *testing.T) {
 			input: `{foo=~"invalid[regexp"}`,
 			err:   "error parsing regexp: missing closing ]: `[regexp)$`",
 		},
-	}
+		// Double escaped strings.
-
+		{
-	for i, tc := range testCases {
+			input: `"{foo=\"bar"}`,
-		got, err := ParseMatchers(tc.input)
+			err:   `bad matcher format: "{foo=\"bar"`,
-		if err != nil && tc.err == "" {
+		},
-			t.Fatalf("got error where none expected (i=%d): %v", i, err)
+		{
-		}
+			input: `"foo=\"bar"`,
-		if err == nil && tc.err != "" {
+			err:   `bad matcher format: "foo=\"bar"`,
-			t.Fatalf("expected error but got none (i=%d): %v", i, tc.err)
+		},
-		}
+		{
-		if err != nil && err.Error() != tc.err {
+			input: `"foo=\"bar\""`,
-			t.Fatalf("error not equal (i=%d):\ngot  %v\nwant %v", i, err, tc.err)
+			err:   `bad matcher format: "foo=\"bar\""`,
-		}
+		},
-		if !reflect.DeepEqual(got, tc.want) {
+		{
-			t.Fatalf("labels not equal (i=%d):\ngot  %v\nwant %v", i, got, tc.want)
+			input: `"foo=\"bar\"`,
-		}
+			err:   `bad matcher format: "foo=\"bar\"`,
+		},
+		{
+			input: `"{foo=\"bar\"}"`,
+			err:   `bad matcher format: "{foo=\"bar\"}"`,
+		},
+		{
+			input: `"foo="bar""`,
+			err:   `bad matcher format: "foo="bar""`,
+		},
+	} {
+		t.Run(tc.input, func(t *testing.T) {
+			got, err := ParseMatchers(tc.input)
+			if err != nil && tc.err == "" {
+				t.Fatalf("got error where none expected: %v", err)
+			}
+			if err == nil && tc.err != "" {
+				t.Fatalf("expected error but got none: %v", tc.err)
+			}
+			if err != nil && err.Error() != tc.err {
+				t.Fatalf("error not equal:\ngot  %v\nwant %v", err, tc.err)
+			}
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Fatalf("labels not equal:\ngot  %v\nwant %v", got, tc.want)
+			}
+		})
 	}
 
 }