feat: Use dlclark/regexp2 over standard library's package

Signed-off-by: Pranshu Srivastava <[email protected]>

Author: rexagod

Diff for: go.mod

@@ -5,6 +5,7 @@ go 1.23.0
 require (
 	github.com/KimMachineGun/automemlimit v0.7.0
 	github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0
+	github.com/dlclark/regexp2 v1.11.5
 	github.com/fsnotify/fsnotify v1.8.0
 	github.com/go-logr/logr v1.4.2
 	github.com/gobuffalo/flect v1.0.3

Diff for: go.sum

@@ -16,6 +16,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0 h1:0wH6nO9QEa02Qx8sIQGw6ieKdz+BXjpccSOo9vXNl4U=
 github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0/go.mod h1:4hKCXuwrJoYvHZxJ86+bRVTOMyJ0Ej+RqfSm8mHi6KA=
+github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ=
+github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
 github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=

Diff for: pkg/allowdenylist/allowdenylist.go

@@ -18,13 +18,23 @@ package allowdenylist
 
 import (
 	"errors"
-	"regexp"
 	"strings"
+	"time"
+
+	regexp "github.com/dlclark/regexp2"
+	"k8s.io/klog/v2"
 
 	generator "k8s.io/kube-state-metrics/v2/pkg/metric_generator"
 )
 
-// AllowDenyList encapsulates the logic needed to filter based on a string.
+// Use ECMAScript as the default regexp spec to support lookarounds (#2594).
+var regexpDefaultSpec regexp.RegexOptions = regexp.ECMAScript
+
+func init() {
+	regexp.DefaultMatchTimeout = time.Minute
+}
+
+// AllowDenyList namespaceencapsulates the logic needed to filter based on a string.
 type AllowDenyList struct {
 	list        map[string]struct{}
 	rList       []*regexp.Regexp
@@ -62,7 +72,7 @@ func New(allow, deny map[string]struct{}) (*AllowDenyList, error) {
 func (l *AllowDenyList) Parse() error {
 	regexes := make([]*regexp.Regexp, 0, len(l.list))
 	for item := range l.list {
-		r, err := regexp.Compile(item)
+		r, err := regexp.Compile(item, regexpDefaultSpec)
 		if err != nil {
 			return err
 		}
@@ -99,25 +109,36 @@ func (l *AllowDenyList) Exclude(items []string) {
 }
 
 // IsIncluded returns if the given item is included.
-func (l *AllowDenyList) IsIncluded(item string) bool {
-	var matched bool
+func (l *AllowDenyList) IsIncluded(item string) (bool, error) {
+	var (
+		matched bool
+		err     error
+	)
 	for _, r := range l.rList {
-		matched = r.MatchString(item)
+		matched, err = r.MatchString(item)
+		if err != nil {
+			return false, err
+		}
 		if matched {
 			break
 		}
 	}
 
 	if l.isAllowList {
-		return matched
+		return matched, nil
 	}
 
-	return !matched
+	return !matched, nil
 }
 
 // IsExcluded returns if the given item is excluded.
-func (l *AllowDenyList) IsExcluded(item string) bool {
-	return !l.IsIncluded(item)
+func (l *AllowDenyList) IsExcluded(item string) (bool, error) {
+	isIncluded, err := l.IsIncluded(item)
+	if err != nil {
+		return false, err
+	}
+
+	return !isIncluded, nil
 }
 
 // Status returns the status of the AllowDenyList that can e.g. be passed into
@@ -137,7 +158,13 @@ func (l *AllowDenyList) Status() string {
 
 // Test returns if the given family generator passes (is included in) the AllowDenyList
 func (l *AllowDenyList) Test(generator generator.FamilyGenerator) bool {
-	return l.IsIncluded(generator.Name)
+	isIncluded, err := l.IsIncluded(generator.Name)
+	if err != nil {
+		klog.ErrorS(err, "Error while processing allow-deny entries for generator", "generator", generator.Name)
+		return false
+	}
+
+	return isIncluded
 }
 
 func copyList(l map[string]struct{}) map[string]struct{} {

Diff for: pkg/allowdenylist/allowdenylist_test.go

@@ -17,8 +17,12 @@ limitations under the License.
 package allowdenylist
 
 import (
-	"regexp"
+	"fmt"
+	"strings"
 	"testing"
+	"time"
+
+	regexp "github.com/dlclark/regexp2"
 )
 
 func TestNew(t *testing.T) {
@@ -76,7 +80,11 @@ func TestInclude(t *testing.T) {
 			t.Fatal("expected Parse() to not fail")
 		}
 
-		if !allowlist.IsIncluded("item1") {
+		isIncluded, err := allowlist.IsIncluded("item1")
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -93,7 +101,11 @@ func TestInclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if !denylist.IsIncluded(item1) {
+		isIncluded, err := denylist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -109,7 +121,11 @@ func TestInclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if !allowlist.IsIncluded("kube_secret_info") {
+		isIncluded, err := allowlist.IsIncluded("kube_secret_info")
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -130,16 +146,32 @@ func TestInclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if denylist.IsExcluded(item1) {
+		isExcluded, err := denylist.IsExcluded(item1)
+		if err != nil {
+			t.Fatal("expected IsExcluded() to not fail")
+		}
+		if isExcluded {
 			t.Fatalf("expected included %s to be included", item1)
 		}
-		if denylist.IsIncluded(item2) {
+		isIncluded, err := denylist.IsIncluded(item2)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatalf("expected included %s to be excluded", item2)
 		}
-		if denylist.IsIncluded(item3) {
+		isIncluded, err = denylist.IsIncluded(item3)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatalf("expected included %s to be excluded", item3)
 		}
-		if denylist.IsExcluded(item4) {
+		isExcluded, err = denylist.IsExcluded(item4)
+		if err != nil {
+			t.Fatal("expected IsExcluded() to not fail")
+		}
+		if isExcluded {
 			t.Fatalf("expected included %s to be included", item4)
 		}
 	})
@@ -159,7 +191,11 @@ func TestExclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if allowlist.IsIncluded(item1) {
+		isIncluded, err := allowlist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatal("expected excluded item to be excluded")
 		}
 	})
@@ -176,7 +212,11 @@ func TestExclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if denylist.IsIncluded(item1) {
+		isIncluded, err := denylist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatal("expected excluded item to be excluded")
 		}
 	})
@@ -224,7 +264,8 @@ func TestStatus(t *testing.T) {
 		allowlist, _ := New(map[string]struct{}{item1: {}, item2: {}}, map[string]struct{}{})
 		actualStatusString := allowlist.Status()
 		expectedRegexPattern := `^Including the following lists that were on allowlist: (item1|item2), (item2|item1)$`
-		matched, _ := regexp.MatchString(expectedRegexPattern, actualStatusString)
+		re := regexp.MustCompile(expectedRegexPattern, regexpDefaultSpec)
+		matched, _ := re.MatchString(actualStatusString)
 		if !matched {
 			t.Errorf("expected status %q but got %q", expectedRegexPattern, actualStatusString)
 		}
@@ -244,9 +285,38 @@ func TestStatus(t *testing.T) {
 		denylist, _ := New(map[string]struct{}{}, map[string]struct{}{item1: {}, item2: {}})
 		actualStatusString := denylist.Status()
 		expectedRegexPattern := `^Excluding the following lists that were on denylist: (item1|item2), (item2|item1)$`
-		matched, _ := regexp.MatchString(expectedRegexPattern, actualStatusString)
+		re := regexp.MustCompile(expectedRegexPattern, regexpDefaultSpec)
+		matched, _ := re.MatchString(actualStatusString)
 		if !matched {
 			t.Errorf("expected status %q but got %q", expectedRegexPattern, actualStatusString)
 		}
 	})
 }
+
+func TestCatastrophicBacktrackTimeout(t *testing.T) {
+	r, err := regexp.Compile("(.+)*\\?", 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var exp = "Lorem ipsum dolor sit amet, consectetur adipiscing elit"
+	exp = strings.Repeat(exp, 2^10)
+
+	timeout := regexp.DefaultMatchTimeout
+	t.Logf("regexp.DefaultMatchTimeout set to: %v", timeout)
+	buffer := 500 * time.Millisecond
+	t.Run(fmt.Sprint(timeout), func(t *testing.T) {
+		r.MatchTimeout = timeout
+		start := time.Now()
+		_, err = r.FindStringMatch(exp)
+		if err != nil && !strings.HasPrefix(err.Error(), "match timeout") {
+			t.Fatal(err)
+		}
+		if err == nil {
+			t.Fatal("expected catastrophic backtracking error")
+		}
+		elapsed := time.Since(start)
+		if elapsed > timeout+buffer {
+			t.Fatalf("timeout %v exceeded: %v", timeout, elapsed)
+		}
+	})
+}