Skip to content

Commit

Permalink
[TT-1944] Fix regex matching for parameters (#6480)
Browse files Browse the repository at this point in the history
JIRA: https://tyktech.atlassian.net/browse/TT-1944

Match used `*` allowing it to match 0 characters.
Match now uses `+`, matching a minimum of 1 character.

Behaviour change:

Previously requests for `/users/{id}` would allow `/users/` to pass
without parameter.
When replacing named parameter patterns, `{}` would be matched, which is
invalid.

Follow up issue:

Mux can define a regex as `{id:[0-9]+}` or similar custom regex rules.
The match does not respect this.

---------

Co-authored-by: Tit Petric <tit@tyk.io>
  • Loading branch information
2 people authored and jeffy-mathew committed Sep 13, 2024
1 parent 7cbb40a commit 7ee58e0
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
4 changes: 2 additions & 2 deletions .taskfiles/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ tasks:
- for: { var: packages, as: package }
cmd: |-
gotestsum --no-color=false --hide-summary=skipped --raw-command \
go test -p 1 -json {{.testArgs}} {{.args}} -count=1 -v \
go test -p 1 -parallel 1 -json {{.testArgs}} {{.args}} -count=1 -v \
-coverprofile=coverage/{{.package | replace "." "gateway" | replace "/" "-"}}.cov {{.package}} | head -n -2
integration-combined:
Expand All @@ -66,7 +66,7 @@ tasks:
- defer: { task: services:down }
- defer: { task: report }
- rm -rf coverage && mkdir -p coverage
- go test -p 1 -json {{.testArgs}} {{.args}} -coverprofile=coverage/{{.product}}-all.cov -count=1 -v ./... > coverage/{{.product}}-all.json
- go test -p 1 -parallel 1 -json {{.testArgs}} {{.args}} -coverprofile=coverage/{{.product}}-all.cov -count=1 -v ./... > coverage/{{.product}}-all.json

plugin:race:
dir: '{{.dir}}'
Expand Down
9 changes: 7 additions & 2 deletions gateway/api_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,13 +817,18 @@ func (a APIDefinitionLoader) getPathSpecs(apiVersionDef apidef.VersionInfo, conf
return combinedPath, len(whiteListPaths) > 0
}

// match mux tags, `{id}`.
var apiLangIDsRegex = regexp.MustCompile(`{([^}]+)}`)

func (a APIDefinitionLoader) generateRegex(stringSpec string, newSpec *URLSpec, specType URLStatus, conf config.Config) {
apiLangIDsRegex := regexp.MustCompile(`{([^}]*)}`)
asRegexStr := apiLangIDsRegex.ReplaceAllString(stringSpec, `([^/]*)`)
// replace mux named parameters with regex path match
asRegexStr := apiLangIDsRegex.ReplaceAllString(stringSpec, `([^/]+)`)

// Case insensitive match
if newSpec.IgnoreCase || conf.IgnoreEndpointCase {
asRegexStr = "(?i)" + asRegexStr
}

asRegex, _ := regexp.Compile(asRegexStr)
newSpec.Status = specType
newSpec.Spec = asRegex
Expand Down
8 changes: 4 additions & 4 deletions gateway/api_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func TestWhitelist(t *testing.T) {

ts.Run(t, []test.TestCase{
// Should mock path
{Path: "/reply/", Code: http.StatusOK, BodyMatch: "flump"},
{Path: "/reply/", Code: http.StatusForbidden},
{Path: "/reply/123", Code: http.StatusOK, BodyMatch: "flump"},
// Should get original upstream response
{Path: "/get", Code: http.StatusOK, BodyMatch: `"Url":"/get"`},
Expand Down Expand Up @@ -147,14 +147,14 @@ func TestWhitelist(t *testing.T) {

ts.Run(t, []test.TestCase{
{Path: "/foo", Code: http.StatusForbidden},
{Path: "/foo/", Code: http.StatusOK},
{Path: "/foo/", Code: http.StatusForbidden},
{Path: "/foo/1", Code: http.StatusOK},
{Path: "/foo/1/bar", Code: http.StatusForbidden},
{Path: "/foo/1/bar/", Code: http.StatusOK},
{Path: "/foo/1/bar/", Code: http.StatusForbidden},
{Path: "/foo/1/bar/1", Code: http.StatusOK},
{Path: "/", Code: http.StatusForbidden},
{Path: "/baz", Code: http.StatusForbidden},
{Path: "/baz/", Code: http.StatusOK},
{Path: "/baz/", Code: http.StatusForbidden},
{Path: "/baz/1", Code: http.StatusOK},
{Path: "/baz/1/", Code: http.StatusOK},
{Path: "/baz/1/bazz", Code: http.StatusOK},
Expand Down

0 comments on commit 7ee58e0

Please sign in to comment.