From 986ca1a16a16bcc1a4285cd8f0d0b37572df55f0 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Thu, 7 Dec 2023 11:34:45 +0100 Subject: [PATCH 1/2] Ensure manipulating the returned FastRegexMatcher.SetMatches() doesn't pollute the matcher's internal state Signed-off-by: Marco Pracucci --- model/labels/regexp.go | 8 +++++++- model/labels/regexp_test.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/model/labels/regexp.go b/model/labels/regexp.go index 5486a642e6..4ea178f7c9 100644 --- a/model/labels/regexp.go +++ b/model/labels/regexp.go @@ -339,7 +339,13 @@ 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. + ret := make([]string, 0, len(m.setMatches)) + for _, value := range m.setMatches { + ret = append(ret, value) + } + return ret } func (m *FastRegexMatcher) GetRegexString() string { diff --git a/model/labels/regexp_test.go b/model/labels/regexp_test.go index 582418a8a6..8b8525b6c9 100644 --- a/model/labels/regexp_test.go +++ b/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() From 0a92839843f36d2d246fa7e73203c236408432de Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Thu, 7 Dec 2023 11:50:08 +0100 Subject: [PATCH 2/2] Use slices.Clone() Signed-off-by: Marco Pracucci --- model/labels/regexp.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/model/labels/regexp.go b/model/labels/regexp.go index 4ea178f7c9..17c9e5fcfe 100644 --- a/model/labels/regexp.go +++ b/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 ( @@ -341,11 +342,7 @@ func (m *FastRegexMatcher) MatchString(s string) bool { func (m *FastRegexMatcher) SetMatches() []string { // IMPORTANT: always return a copy, otherwise if the caller manipulate this slice it will // also get manipulated in the cached FastRegexMatcher instance. - ret := make([]string, 0, len(m.setMatches)) - for _, value := range m.setMatches { - ret = append(ret, value) - } - return ret + return slices.Clone(m.setMatches) } func (m *FastRegexMatcher) GetRegexString() string {