Merge pull request #577 from grafana/fix-regexp-optimization-with-dou…

…ble-capture-group Fix FastRegexMatcher to skip nested capture groups
grafana · Dec 20, 2023 · 12d2c10 · 12d2c10
2 parents ab26771 + b894301
commit 12d2c10
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 1 deletion.
diff --git a/model/labels/regexp.go b/model/labels/regexp.go
@@ -284,7 +284,8 @@ func findSetMatchesFromAlternate(re *syntax.Regexp, base string) (matches []stri
 // clearCapture removes capture operation as they are not used for matching.
 func clearCapture(regs ...*syntax.Regexp) {
 	for _, r := range regs {
-		if r.Op == syntax.OpCapture {
+		// Iterate on the regexp because capture groups could be nested.
+		for r.Op == syntax.OpCapture {
 			*r = *r.Sub[0]
 		}
 	}

diff --git a/model/labels/regexp_test.go b/model/labels/regexp_test.go
@@ -286,6 +286,8 @@ func TestFindSetMatches(t *testing.T) {
 		// Simple sets alternate and concat and alternates with empty matches
 		// parsed as  b(ar|(?:)|uzz) where b(?:) means literal b.
 		{"bar|b|buzz", []string{"bar", "b", "buzz"}, true},
+		// Skip nested capture groups.
+		{"^((bar|b|buzz))$", []string{"bar", "b", "buzz"}, true},
 		// Skip outer anchors (it's enforced anyway at the root).
 		{"^(bar|b|buzz)$", []string{"bar", "b", "buzz"}, true},
 		{"^(?:prod|production)$", []string{"prod", "production"}, true},
@@ -395,6 +397,8 @@ func TestStringMatcherFromRegexp(t *testing.T) {
 		{"^foo$", &equalStringMatcher{s: "foo", caseSensitive: true}},
 		{"^(?i:foo)$", &equalStringMatcher{s: "FOO", caseSensitive: false}},
 		{"^((?i:foo)|(bar))$", orStringMatcher([]StringMatcher{&equalStringMatcher{s: "FOO", caseSensitive: false}, &equalStringMatcher{s: "bar", caseSensitive: true}})},
+		{`(?i:((foo|bar)))`, orStringMatcher([]StringMatcher{&equalStringMatcher{s: "FOO", caseSensitive: false}, &equalStringMatcher{s: "BAR", caseSensitive: false}})},
+		{`(?i:((foo1|foo2|bar)))`, orStringMatcher([]StringMatcher{orStringMatcher([]StringMatcher{&equalStringMatcher{s: "FOO1", caseSensitive: false}, &equalStringMatcher{s: "FOO2", caseSensitive: false}}), &equalStringMatcher{s: "BAR", caseSensitive: false}})},
 		{"^((?i:foo|oo)|(bar))$", orStringMatcher([]StringMatcher{&equalStringMatcher{s: "FOO", caseSensitive: false}, &equalStringMatcher{s: "OO", caseSensitive: false}, &equalStringMatcher{s: "bar", caseSensitive: true}})},
 		{"(?i:(foo1|foo2|bar))", orStringMatcher([]StringMatcher{orStringMatcher([]StringMatcher{&equalStringMatcher{s: "FOO1", caseSensitive: false}, &equalStringMatcher{s: "FOO2", caseSensitive: false}}), &equalStringMatcher{s: "BAR", caseSensitive: false}})},
 		{".*foo.*", &containsStringMatcher{substrings: []string{"foo"}, left: anyStringWithoutNewlineMatcher{}, right: anyStringWithoutNewlineMatcher{}}},