Merge pull request #571 from grafana/fix-possible-set-matches-manipulation

Ensure manipulating the returned FastRegexMatcher.SetMatches() doesn't pollute the matcher's internal state

model/labels/regexp.go

@@ -21,6 +21,7 @@ import (
 	"github.com/dgraph-io/ristretto"
 	"github.com/grafana/regexp"
 	"github.com/grafana/regexp/syntax"
+	"golang.org/x/exp/slices"
 )
 
 const (
@@ -339,7 +340,9 @@ func (m *FastRegexMatcher) MatchString(s string) bool {
 }
 
 func (m *FastRegexMatcher) SetMatches() []string {
-	return m.setMatches
+	// IMPORTANT: always return a copy, otherwise if the caller manipulate this slice it will
+	// also get manipulated in the cached FastRegexMatcher instance.
+	return slices.Clone(m.setMatches)
 }
 
 func (m *FastRegexMatcher) GetRegexString() string {

model/labels/regexp_test.go

@@ -344,6 +344,20 @@ func TestFindSetMatches(t *testing.T) {
 	}
 }
 
+func TestFastRegexMatcher_SetMatches_ShouldReturnACopy(t *testing.T) {
+	m, err := newFastRegexMatcherWithoutCache("a|b")
+	require.NoError(t, err)
+	require.Equal(t, []string{"a", "b"}, m.SetMatches())
+
+	// Manipulate the returned slice.
+	matches := m.SetMatches()
+	matches[0] = "xxx"
+	matches[1] = "yyy"
+
+	// Ensure that if we call SetMatches() again we get the original one.
+	require.Equal(t, []string{"a", "b"}, m.SetMatches())
+}
+
 func BenchmarkFastRegexMatcher(b *testing.B) {
 	texts := generateRandomValues()