[TT-12865] URL matching prefixes/explicit, regex support

[titpetric]

User description

https://tyktech.atlassian.net/browse/TT-12865

---

PR Type

Bug fix, Tests

---

Description

• Enhanced URL path matching to handle both stripped and full URL paths, ensuring backward compatibility.
• Improved error handling and logging for regex matching in ProcessRequest.
• Updated test cases to reflect changes in URL path handling and added new tests for regex matching.
• Improved regex pattern handling by supporting patterns starting with '^' in GetPathRegexp.

---

Changes walkthrough 📝

	Relevant files
Bug fix	mw_granular_access.go Enhance URL path matching for backward compatibility          gateway/mw_granular_access.go Introduced handling for both stripped and full URL paths. Added error handling for regex matching. Refactored logger initialization. +17/-8    mux.go Improve regex pattern handling in path matching                    internal/httputil/mux.go Added handling for patterns starting with '^'. Ensured backward compatibility with existing regex patterns. +3/-0
Tests	mw_granular_access_test.go Update tests for enhanced URL path matching                            gateway/mw_granular_access_test.go Updated test cases to include regex matching for listen paths. Adjusted paths in test cases to reflect new listen path. Added new test cases for regex matching. +46/-27  mux_test.go Add test for regex pattern handling improvement                    internal/httputil/mux_test.go Added test case for regex pattern starting with '^'. +1/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The error handling in the loop for matching URL paths might skip valid URLs if an error occurs early in the iterations. Consider breaking out of the loop or handling this scenario differently to ensure all paths are evaluated correctly. Logging Clarity The logging within the URL matching loop logs an error for each failed match attempt, which could lead to log flooding and may obscure actual errors. Consider logging only unexpected errors or summarizing match attempts.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Handle potential errors after the loop to ensure they do not propagate unmanaged Consider handling the case where err is not nil after the loop in lines 48-54. Currently, if err is not nil, the loop breaks and the error is not handled, potentially leading to further issues down the execution path. gateway/mw_granular_access.go [48-54] for _, urlPath := range urlPaths { match, err = httputil.MatchEndpoint(url, urlPath) - if err != nil || !match { + if err != nil { logger.WithError(err).Errorf("error matching path regex: %q, skipping", url) + return err, http.StatusInternalServerError + } + if !match { continue } break } Suggestion importance[1-10]: 9 Why: The suggestion addresses a potential bug where an error is not handled after the loop, which could lead to unexpected behavior or failures. Handling the error properly by returning it and an appropriate HTTP status code improves the robustness of the code.	9
Performance	Optimize logger usage by reducing redundancy and potential performance overhead Refactor the logging inside the loop to reduce redundancy and potential performance issues due to repeated logger setup. gateway/mw_granular_access.go [41-60] logger := m.Logger().WithField("path", r.URL.Path) ... -logger.WithError(err).Errorf("error matching path regex: %q, skipping", url) -... -logger.WithField("pattern", url).WithField("match", match).Debug("checking allowed url") +logEntry := logger.WithField("pattern", url) +if err != nil { + logEntry.WithError(err).Errorf("error matching path regex: %q, skipping", url) +} else if match { + logEntry.WithField("match", match).Debug("checking allowed url") +} Suggestion importance[1-10]: 7 Why: The suggestion improves performance by reducing redundant logger setup and enhances code readability. While not critical, it optimizes logger usage, which can be beneficial in high-load scenarios.	7

[github-actions]

API Changes

--- prev.txt	2024-09-12 05:51:02.492792205 +0000
+++ current.txt	2024-09-12 05:50:59.316758992 +0000
@@ -5905,6 +5905,49 @@
 	// Regular expressions and parameterized routes will be left alone regardless of this setting.
 	EnableStrictRoutes bool `json:"enable_strict_routes"`
 
+	// EnablePrefixMatching changes the URL matching from wildcard mode to prefix mode.
+	// For example, `/json` matches `*/json*` by current default behaviour.
+	// If prefix matching is enabled, the match will be performed as a prefix match (`/json*`).
+	//
+	// The `/json` url would be matched as `^/json` against the following paths:
+	//
+	// - Full listen path and versioning URL (`/listen-path/v4/json`)
+	// - Stripped listen path URL (`/v4/json`)
+	// - Stripped version information (`/json`) - match.
+	//
+	// If versioning is disabled then the following URLs are considered:
+	//
+	// - Full listen path and endpoint (`/listen-path/json`)
+	// - Stripped listen path (`/json`) - match.
+	//
+	// For inputs that start with `/`, a prefix match is ensured by prepending
+	// the start of string `^` caret.
+	//
+	// For all other cases, the pattern remains unmodified.
+	//
+	// Combine this option with EnableSuffixMatching to achieve strict
+	// url matching with `/json` being evaluated as `^/json$`.
+	EnablePrefixMatching bool `json:"enable_prefix_matching"`
+
+	// EnableSuffixMatching changes the URL matching to match as a suffix.
+	// For example: `/json` is matched as `/json$` against the following paths:
+	//
+	// - Full listen path and versioning URL (`/listen-path/v4/json`)
+	// - Stripped listen path URL (`/v4/json`)
+	// - Stripped version information (`/json`) - match.
+	//
+	// If versioning is disabled then the following URLs are considered:
+	//
+	// - Full listen path and endpoint (`/listen-path/json`)
+	// - Stripped listen path (`/json`) - match.
+	//
+	// If the input pattern already ends with a `$` (`/json$`) then
+	// the pattern remains unmodified.
+	//
+	// Combine this option with EnablePrefixMatching to achieve strict
+	// url matching with `/json` being evaluated as `^/json$`.
+	EnableSuffixMatching bool `json:"enable_suffix_matching"`
+
 	// Disable TLS verification. Required if you are using self-signed certificates.
 	SSLInsecureSkipVerify bool `json:"ssl_insecure_skip_verify"`
 
@@ -7884,6 +7927,12 @@
 func (a *APISpec) StopSessionManagerPool()
 
 func (a *APISpec) StripListenPath(reqPath string) string
+    StripListenPath will strip the listen path from the URL, keeping version in
+    tact.
+
+func (a *APISpec) StripVersionPath(reqPath string) string
+    StripVersionPath will strip the version from the URL. The input URL should
+    already have listen path stripped.
 
 func (a *APISpec) URLAllowedAndIgnored(r *http.Request, rxPaths []URLSpec, whiteListStatus bool) (RequestStatus, interface{})
     URLAllowedAndIgnored checks if a url is allowed and ignored.
@@ -10194,6 +10243,8 @@
     sessionFailReason if session limits have been exceeded. Key values to manage
     rate are Rate and Per, e.g. Rate of 10 messages Per 10 seconds
 
+func (l *SessionLimiter) RateLimitInfo(r *http.Request, api *APISpec, endpoints user.Endpoints) (*user.EndpointRateLimitInfo, bool)
+
 func (l *SessionLimiter) RedisQuotaExceeded(r *http.Request, session *user.SessionState, quotaKey, scope string, limit *user.APILimit, store storage.Handler, hashKeys bool) bool
     RedisQuotaExceeded returns true if the request should be blocked as over
     quota.
@@ -10524,7 +10575,6 @@
     system, return an error to have the chain fail
 
 type URLSpec struct {
-	Spec                      *regexp.Regexp
 	Status                    URLStatus
 	MethodActions             map[string]apidef.EndpointMethodMeta
 	Whitelist                 apidef.EndPointMeta
@@ -10553,6 +10603,7 @@
 	RateLimit                 apidef.RateLimitMeta
 
 	IgnoreCase bool
+	// Has unexported fields.
 }
     URLSpec represents a flattened specification for URLs, used to check if
     a proxy URL path is on any of the white, black or ignored lists. This is
@@ -12456,6 +12507,9 @@
 type EndpointMethods []EndpointMethod
     EndpointMethods is a collection of EndpointMethod.
 
+func (em EndpointMethods) Contains(method string) bool
+    Contains is used to assert if a method exists in EndpointMethods.
+
 func (em EndpointMethods) Len() int
     Len is used to implement sort interface.
 
@@ -12489,9 +12543,6 @@
     If duplicate entries are found, it would get overwritten with latest entries
     Endpoints.
 
-func (es Endpoints) RateLimitInfo(method string, reqEndpoint string) (*EndpointRateLimitInfo, bool)
-    RateLimitInfo returns EndpointRateLimitInfo for endpoint rate limiting.
-
 func (es Endpoints) Swap(i, j int)
     Swap is used to implement sort interface.
 

[jeffy-mathew]

@titpetric we have the same behaviour to be followed when matching URL for endpoint level rate limits.

[titpetric]

Suggested change

	var apiLangIDsRegex = regexp.MustCompile(`{([^}]+)}`)
	var apiLangIDsRegex = regexp.MustCompile(`{([^}]+)}`)

I've seen listenpath hacks that provide /{}/users as the URL. Wondering if this should remain supported.

[sonarcloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint