From 2e0ecc013f3e490ba459b1ed165ec5dfd2d7aa6f Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 1 Mar 2023 12:18:30 +0100 Subject: [PATCH] Fix containsStringMatcher() when the text contains multiple occurrences of a substring (#431) Signed-off-by: Marco Pracucci --- model/labels/regexp.go | 37 +++++++++++++++++++++++-------------- model/labels/regexp_test.go | 27 +++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/model/labels/regexp.go b/model/labels/regexp.go index 680da68b85..721cb247c5 100644 --- a/model/labels/regexp.go +++ b/model/labels/regexp.go @@ -397,27 +397,36 @@ type containsStringMatcher struct { func (m *containsStringMatcher) Matches(s string) bool { for _, substr := range m.substrings { if m.right != nil && m.left != nil { - pos := strings.Index(s, substr) - if pos < 0 { - continue + searchStartPos := 0 + + for { + pos := strings.Index(s[searchStartPos:], substr) + if pos < 0 { + break + } + + // Since we started searching from searchStartPos, we have to add that offset + // to get the actual position of the substring inside the text. + pos += searchStartPos + + // If both the left and right matchers match, then we can stop searching because + // we've found a match. + if m.left.Matches(s[:pos]) && m.right.Matches(s[pos+len(substr):]) { + return true + } + + // Continue searching for another occurrence of the substring inside the text. + searchStartPos = pos + 1 } - if m.left.Matches(s[:pos]) && m.right.Matches(s[pos+len(substr):]) { - return true - } - continue - } - // If we have to check for characters on the left then we need to match a suffix. - if m.left != nil { + } else if m.left != nil { + // If we have to check for characters on the left then we need to match a suffix. if strings.HasSuffix(s, substr) && m.left.Matches(s[:len(s)-len(substr)]) { return true } - continue - } - if m.right != nil { + } else if m.right != nil { if strings.HasPrefix(s, substr) && m.right.Matches(s[len(substr):]) { return true } - continue } } return false diff --git a/model/labels/regexp_test.go b/model/labels/regexp_test.go index 7e30409092..5f8d9bea90 100644 --- a/model/labels/regexp_test.go +++ b/model/labels/regexp_test.go @@ -19,7 +19,6 @@ import ( "testing" "time" - "github.com/grafana/regexp" "github.com/grafana/regexp/syntax" "github.com/stretchr/testify/require" ) @@ -60,6 +59,7 @@ var ( "foo", " foo bar", "bar", "buzz\nbar", "bar foo", "bfoo", "\n", "\nfoo", "foo\n", "hello foo world", "hello foo\n world", "", "FOO", "Foo", "OO", "Oo", "\nfoo\n", strings.Repeat("f", 20), "prometheus", "prometheus_api_v1", "prometheus_api_v1_foo", "10.0.1.20", "10.0.2.10", "10.0.3.30", "10.0.4.40", + "foofoo0", "foofoo", } ) @@ -72,9 +72,7 @@ func TestNewFastRegexMatcher(t *testing.T) { t.Parallel() m, err := NewFastRegexMatcher(r) require.NoError(t, err) - re, err := regexp.Compile("^(?:" + r + ")$") - require.NoError(t, err) - require.Equal(t, re.MatchString(v), m.MatchString(v)) + require.Equal(t, m.re.MatchString(v), m.MatchString(v)) }) } @@ -329,3 +327,24 @@ func RandStringRunes(n int) string { } return string(b) } + +func FuzzFastRegexMatcher_WithStaticallyDefinedRegularExpressions(f *testing.F) { + // Create all matchers. + matchers := make([]*FastRegexMatcher, 0, len(regexes)) + for _, re := range regexes { + m, err := NewFastRegexMatcher(re) + require.NoError(f, err) + matchers = append(matchers, m) + } + + // Add known values to seed corpus. + for _, v := range values { + f.Add(v) + } + + f.Fuzz(func(t *testing.T, text string) { + for _, m := range matchers { + require.Equalf(t, m.re.MatchString(text), m.MatchString(text), "regexp: %s text: %s", m.re.String(), text) + } + }) +}