From 02346e4e493f56d699643726842fc9e780a2b7b6 Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Wed, 23 Jun 2021 11:05:49 +0200 Subject: [PATCH] matchers: Parse Matcher now expects consistent enclosing with quotes. (#2632) Fixes https://github.com/prometheus/alertmanager/issues/2630 Signed-off-by: Bartlomiej Plotka --- pkg/labels/parse.go | 35 +++++++++++-------- pkg/labels/parse_test.go | 73 ++++++++++++++++++++++++++++++---------- 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/pkg/labels/parse.go b/pkg/labels/parse.go index 907056fe..0a242d50 100644 --- a/pkg/labels/parse.go +++ b/pkg/labels/parse.go @@ -22,11 +22,9 @@ import ( ) var ( - re = regexp.MustCompile( - // '=~' has to come before '=' because otherwise only the '=' - // will be consumed, and the '~' will be part of the 3rd token. - `^\s*([a-zA-Z_:][a-zA-Z0-9_:]*)\s*(=~|=|!=|!~)\s*((?s).*?)\s*$`, - ) + // '=~' has to come before '=' because otherwise only the '=' + // 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*$`) 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], "\"") - value strings.Builder - escaped bool + rawValue = ms[3] + value strings.Builder + 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. - return nil, errors.Errorf( - "matcher value contains unescaped double quote: %s", rawValue, - ) + if !expectTrailingQuote || i < len(rawValue)-1 { + return nil, errors.Errorf("matcher value contains unescaped double quote: %s", ms[3]) } + 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()) } diff --git a/pkg/labels/parse_test.go b/pkg/labels/parse_test.go index 7adca325..d7847e76 100644 --- a/pkg/labels/parse_test.go +++ b/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)$`", }, - } - - for i, tc := range testCases { - got, err := ParseMatchers(tc.input) - if err != nil && tc.err == "" { - t.Fatalf("got error where none expected (i=%d): %v", i, err) - } - if err == nil && tc.err != "" { - t.Fatalf("expected error but got none (i=%d): %v", i, tc.err) - } - if err != nil && err.Error() != tc.err { - t.Fatalf("error not equal (i=%d):\ngot %v\nwant %v", i, err, tc.err) - } - if !reflect.DeepEqual(got, tc.want) { - t.Fatalf("labels not equal (i=%d):\ngot %v\nwant %v", i, got, tc.want) - } + // Double escaped strings. + { + 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\""`, + }, + { + 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) + } + }) } }