Fix regexp param anchoring (#105)

* fix: anchor user regexp with non-capturing group to bound alternation

* refactor!(matcher): rename Regex to Value and return raw source string

* refactor(simplelru): use range over int in Resize loop

* docs(matcher): clarify Value godoc on regexp matchers

Author: tigerwill90

fox_test.go

@@ -1546,6 +1546,38 @@ func TestWildcardSuffix(t *testing.T) {
 	}
 }
 
+func TestRegexpParamAlternationPrecedence(t *testing.T) {
+	r := MustRouter(AllowRegexpParam(true))
+	require.NoError(t, onlyError(r.Add(MethodGet, "/role/{role:admin|user|guest}", pathHandler)))
+	require.NoError(t, onlyError(r.Add(MethodGet, "/scope/{scope:read|write}/items", pathHandler)))
+
+	cases := []struct {
+		path string
+		want int
+	}{
+		{"/role/admin", http.StatusOK},
+		{"/role/user", http.StatusOK},
+		{"/role/guest", http.StatusOK},
+		{"/role/adminBYPASS", http.StatusNotFound},
+		{"/role/EVILguest", http.StatusNotFound},
+		{"/role/userX", http.StatusNotFound},
+		{"/role/Xuser", http.StatusNotFound},
+		{"/scope/read/items", http.StatusOK},
+		{"/scope/write/items", http.StatusOK},
+		{"/scope/readBYPASS/items", http.StatusNotFound},
+		{"/scope/EVILwrite/items", http.StatusNotFound},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.path, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.path, nil)
+			w := httptest.NewRecorder()
+			r.ServeHTTP(w, req)
+			assert.Equal(t, tc.want, w.Code)
+		})
+	}
+}
+
 func TestInsertUpdateAndDeleteWithHostname(t *testing.T) {
 	cases := []struct {
 		name   string

internal/simplelru/lru.go

@@ -154,7 +154,7 @@ func (c *LRU[K, V]) Cap() int {
 // Resize changes the cache size.
 func (c *LRU[K, V]) Resize(size int) (evicted int) {
 	diff := max(c.Len()-size, 0)
-	for i := 0; i < diff; i++ {
+	for range diff {
 		c.removeOldest()
 	}
 	c.size = size

matcher.go

@@ -85,7 +85,7 @@ func MatchQueryRegexp(key, expr string) (QueryRegexpMatcher, error) {
 	if key == "" {
 		return QueryRegexpMatcher{}, errors.New("empty query key")
 	}
-	regex, err := regexp.Compile("^" + expr + "$")
+	regex, err := regexp.Compile("^(?:" + expr + ")$")
 	if err != nil {
 		return QueryRegexpMatcher{}, err
 	}
@@ -108,14 +108,15 @@ func (m QueryRegexpMatcher) Key() string {
 	return m.key
 }
 
-// Regex returns a copy of the compiled regular expression used to evaluate query values.
-func (m QueryRegexpMatcher) Regex() *regexp.Regexp {
-	return new(*m.regex)
+// Value returns the regular expression matching the query parameter.
+func (m QueryRegexpMatcher) Value() string {
+	expr := m.regex.String()
+	return expr[4 : len(expr)-2]
 }
 
 // String returns a textual representation of the matcher in the form "qx:key=expr".
 func (m QueryRegexpMatcher) String() string {
-	return "qx:" + m.key + "=" + m.regex.String()
+	return "qx:" + m.key + "=" + m.Value()
 }
 
 // Match reports whether the request URL contains the configured key with any value matching the configured regular
@@ -198,7 +199,7 @@ func MatchHeaderRegexp(key, expr string) (HeaderRegexpMatcher, error) {
 	if key == "" {
 		return HeaderRegexpMatcher{}, errors.New("empty header key")
 	}
-	regex, err := regexp.Compile("^" + expr + "$")
+	regex, err := regexp.Compile("^(?:" + expr + ")$")
 	if err != nil {
 		return HeaderRegexpMatcher{}, err
 	}
@@ -221,9 +222,10 @@ func (m HeaderRegexpMatcher) Key() string {
 	return m.canonicalKey
 }
 
-// Regex returns a copy of the compiled regular expression used to evaluate header values.
-func (m HeaderRegexpMatcher) Regex() *regexp.Regexp {
-	return new(*m.regex)
+// Value returns the regular expression matching the header.
+func (m HeaderRegexpMatcher) Value() string {
+	expr := m.regex.String()
+	return expr[4 : len(expr)-2]
 }
 
 // Match reports whether the request contains the configured header with any value matching the configured regular
@@ -247,7 +249,7 @@ func (m HeaderRegexpMatcher) Equal(matcher Matcher) bool {
 
 // String returns a textual representation of the matcher in the form "hx:key=expr".
 func (m HeaderRegexpMatcher) String() string {
-	return "hx:" + m.canonicalKey + "=" + m.regex.String()
+	return "hx:" + m.canonicalKey + "=" + m.Value()
 }
 
 // MatchClientIP returns a [ClientIPMatcher] that matches when the resolved client IP belongs to the given CIDR range.

matcher_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestQueryMatcher_Match(t *testing.T) {
@@ -622,7 +623,7 @@ func TestMatchQueryRegexp(t *testing.T) {
 			}
 			assert.NoError(t, err)
 			assert.Equal(t, tc.wantKey, m.Key())
-			assert.NotNil(t, m.Regex())
+			assert.Equal(t, tc.expr, m.Value())
 		})
 	}
 }
@@ -727,7 +728,50 @@ func TestMatchHeaderRegexp(t *testing.T) {
 			}
 			assert.NoError(t, err)
 			assert.Equal(t, tc.wantKey, m.Key())
-			assert.NotNil(t, m.Regex())
+			assert.Equal(t, tc.expr, m.Value())
+		})
+	}
+}
+
+func TestRegexpMatcherAlternationPrecedence(t *testing.T) {
+	mq, err := MatchQueryRegexp("scope", "read|write")
+	require.NoError(t, err)
+	mh, err := MatchHeaderRegexp("X-Role", "admin|user|guest")
+	require.NoError(t, err)
+
+	queryCases := []struct {
+		url  string
+		want bool
+	}{
+		{"/?scope=read", true},
+		{"/?scope=write", true},
+		{"/?scope=readBYPASS", false},
+		{"/?scope=EVILwrite", false},
+	}
+	for _, tc := range queryCases {
+		t.Run("query "+tc.url, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tc.url, nil)
+			c := NewTestContextOnly(httptest.NewRecorder(), req)
+			assert.Equal(t, tc.want, mq.Match(c))
+		})
+	}
+
+	headerCases := []struct {
+		value string
+		want  bool
+	}{
+		{"admin", true},
+		{"user", true},
+		{"guest", true},
+		{"adminBYPASS", false},
+		{"EVILguest", false},
+	}
+	for _, tc := range headerCases {
+		t.Run("header "+tc.value, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			req.Header.Set("X-Role", tc.value)
+			c := NewTestContextOnly(httptest.NewRecorder(), req)
+			assert.Equal(t, tc.want, mh.Match(c))
 		})
 	}
 }

parser.go

@@ -391,7 +391,7 @@ func (fox *Router) compileParamRegexp(rawRegex string) (*regexp.Regexp, *Pattern
 		return nil, newPatternError("regexp", 0, 0, "missing expression")
 	}
 
-	re, err := regexp.Compile("^" + rawRegex + "$")
+	re, err := regexp.Compile("^(?:" + rawRegex + ")$")
 	if err != nil {
 		return nil, &PatternError{
 			Reason: "regexp",

parser_test.go

@@ -30,7 +30,7 @@ func Test_parsePattern(t *testing.T) {
 			typ:   nodeParam,
 		}
 		if reg != "" {
-			tk.regexp = regexp.MustCompile("^" + reg + "$")
+			tk.regexp = regexp.MustCompile("^(?:" + reg + ")$")
 		}
 		return tk
 	}
@@ -41,7 +41,7 @@ func Test_parsePattern(t *testing.T) {
 			typ:   nodeWildcard,
 		}
 		if reg != "" {
-			tk.regexp = regexp.MustCompile("^" + reg + "$")
+			tk.regexp = regexp.MustCompile("^(?:" + reg + ")$")
 		}
 		return tk
 	}

tree.go

@@ -953,8 +953,9 @@ func concat(a, b string) string {
 // placeholder ("?" for params, "*" for catch-alls).
 func canonicalKey(tk token) string {
 	if tk.regexp != nil {
+		// Strip the surrounding ^(?: and )$ added by compileParamRegexp.
 		expr := tk.regexp.String()
-		return expr[1 : len(expr)-1]
+		return expr[4 : len(expr)-2]
 	}
 	switch tk.typ {
 	case nodeParam: