From 986ca1a16a16bcc1a4285cd8f0d0b37572df55f0 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Thu, 7 Dec 2023 11:34:45 +0100 Subject: [PATCH] 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()