Skip to content

Commit 90ff5a9

Browse files
bugerclaude
andauthored
fix: backport #7972 — [TT-16890] validate request middleware regression (#8072)
## Summary Backport of #7972 to release-5.8.13. Critical regression fix for validate request middleware. - Adds static path shield mechanism to prevent parameterized paths from incorrectly matching static paths - Adds `sortURLSpecsByPathPriority` for consistent path ordering - Exports `PathLess` and `PathParamRegex` from `oasutil` ## Test plan - [ ] Unit Tests & Linting passes - [ ] `go build ./gateway/...` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4ff98bf commit 90ff5a9

File tree

4 files changed

+656
-30
lines changed

4 files changed

+656
-30
lines changed

gateway/api_definition.go

Lines changed: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"net/url"
1313
"os"
1414
"path/filepath"
15+
"sort"
1516
"strings"
1617
"sync/atomic"
1718
texttemplate "text/template"
@@ -21,6 +22,7 @@ import (
2122
"github.com/TykTechnologies/tyk/storage/kv"
2223

2324
"github.com/TykTechnologies/tyk/internal/httputil"
25+
"github.com/TykTechnologies/tyk/internal/oasutil"
2426

2527
"github.com/getkin/kin-openapi/routers/gorillamux"
2628

@@ -1343,7 +1345,6 @@ func (a APIDefinitionLoader) compileOASValidateRequestPathSpec(apiSpec *APISpec,
13431345
continue
13441346
}
13451347

1346-
// Find the path and method for this operation
13471348
path, method := a.findPathAndMethodForOperation(apiSpec, operationID)
13481349
if path == "" || method == "" {
13491350
continue
@@ -1355,14 +1356,20 @@ func (a APIDefinitionLoader) compileOASValidateRequestPathSpec(apiSpec *APISpec,
13551356
OASPath: path,
13561357
}
13571358

1358-
// The path in OAS is relative to the server URL (listenPath)
1359-
// For regex matching, we don't prepend listenPath because URLSpec.matchesPath
1360-
// will strip the listenPath before matching
1361-
// Use standard regex generation with gateway config
13621359
a.generateRegex(path, &newSpec, OASValidateRequest, conf)
13631360
urlSpec = append(urlSpec, newSpec)
13641361
}
13651362

1363+
urlSpec = a.addStaticPathShields(apiSpec, conf, urlSpec, OASValidateRequest, func(path, method string) URLSpec {
1364+
return URLSpec{
1365+
OASValidateRequestMeta: &oas.ValidateRequest{Enabled: false},
1366+
OASMethod: method,
1367+
OASPath: path,
1368+
}
1369+
})
1370+
1371+
sortURLSpecsByPathPriority(urlSpec)
1372+
13661373
return urlSpec
13671374
}
13681375

@@ -1388,7 +1395,6 @@ func (a APIDefinitionLoader) compileOASMockResponsePathSpec(apiSpec *APISpec, co
13881395
continue
13891396
}
13901397

1391-
// Find the path and method for this operation
13921398
path, method := a.findPathAndMethodForOperation(apiSpec, operationID)
13931399
if path == "" || method == "" {
13941400
continue
@@ -1400,11 +1406,20 @@ func (a APIDefinitionLoader) compileOASMockResponsePathSpec(apiSpec *APISpec, co
14001406
OASPath: path,
14011407
}
14021408

1403-
// Use standard regex generation with gateway config
14041409
a.generateRegex(path, &newSpec, OASMockResponse, conf)
14051410
urlSpec = append(urlSpec, newSpec)
14061411
}
14071412

1413+
urlSpec = a.addStaticPathShields(apiSpec, conf, urlSpec, OASMockResponse, func(path, method string) URLSpec {
1414+
return URLSpec{
1415+
OASMockResponseMeta: &oas.MockResponse{Enabled: false},
1416+
OASMethod: method,
1417+
OASPath: path,
1418+
}
1419+
})
1420+
1421+
sortURLSpecsByPathPriority(urlSpec)
1422+
14081423
return urlSpec
14091424
}
14101425

@@ -1426,6 +1441,69 @@ func (a APIDefinitionLoader) findPathAndMethodForOperation(apiSpec *APISpec, ope
14261441
return "", ""
14271442
}
14281443

1444+
// addStaticPathShields adds synthetic disabled URLSpec entries for static OAS paths
1445+
// that don't already have an entry in urlSpec. These entries act as shields: when the
1446+
// middleware scans the path list, a static shield entry matches before any parameterised
1447+
// regex, and the middleware sees Enabled=false and skips it. This prevents parameterised
1448+
// paths (e.g. /employees/{id}) from incorrectly matching static paths (e.g. /employees/static).
1449+
//
1450+
// Shield entries are only added when urlSpec contains at least one parameterised path,
1451+
// since without parameterised paths there is no cross-matching risk.
1452+
func (a APIDefinitionLoader) addStaticPathShields(
1453+
apiSpec *APISpec,
1454+
conf config.Config,
1455+
urlSpec []URLSpec,
1456+
status URLStatus,
1457+
newDisabledSpec func(path, method string) URLSpec,
1458+
) []URLSpec {
1459+
if apiSpec.OAS.Paths == nil {
1460+
return urlSpec
1461+
}
1462+
1463+
existing, hasParameterised := indexURLSpecs(urlSpec)
1464+
if !hasParameterised {
1465+
return urlSpec
1466+
}
1467+
1468+
for path, pathItem := range apiSpec.OAS.Paths.Map() {
1469+
if httputil.IsMuxTemplate(path) {
1470+
continue
1471+
}
1472+
for method := range pathItem.Operations() {
1473+
method = strings.ToUpper(method)
1474+
if _, exists := existing[path+":"+method]; exists {
1475+
continue
1476+
}
1477+
newSpec := newDisabledSpec(path, method)
1478+
a.generateRegex(path, &newSpec, status, conf)
1479+
urlSpec = append(urlSpec, newSpec)
1480+
}
1481+
}
1482+
1483+
return urlSpec
1484+
}
1485+
1486+
// indexURLSpecs builds a set of "path:METHOD" keys from the given specs and reports
1487+
// whether any spec uses a parameterised (mux-template) path.
1488+
func indexURLSpecs(specs []URLSpec) (existing map[string]struct{}, hasParameterised bool) {
1489+
existing = make(map[string]struct{}, len(specs))
1490+
for _, spec := range specs {
1491+
existing[spec.OASPath+":"+spec.OASMethod] = struct{}{}
1492+
if httputil.IsMuxTemplate(spec.OASPath) {
1493+
hasParameterised = true
1494+
}
1495+
}
1496+
return
1497+
}
1498+
1499+
// sortURLSpecsByPathPriority sorts URLSpec entries using the same path priority
1500+
// rules as oasutil.SortByPathLength, ensuring consistent ordering across the gateway.
1501+
func sortURLSpecsByPathPriority(specs []URLSpec) {
1502+
sort.Slice(specs, func(i, j int) bool {
1503+
return oasutil.PathLess(specs[i].OASPath, specs[j].OASPath)
1504+
})
1505+
}
1506+
14291507
func (a APIDefinitionLoader) getExtendedPathSpecs(apiVersionDef apidef.VersionInfo, apiSpec *APISpec, conf config.Config) ([]URLSpec, bool) {
14301508
// TODO: New compiler here, needs to put data into a different structure
14311509

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package gateway
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/getkin/kin-openapi/openapi3"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/TykTechnologies/tyk/apidef/oas"
13+
)
14+
15+
// BenchmarkSortURLSpecsByPathPriority measures the overhead of sorting URLSpec entries.
16+
func BenchmarkSortURLSpecsByPathPriority(b *testing.B) {
17+
for _, n := range []int{10, 50, 200} {
18+
b.Run(fmt.Sprintf("paths=%d", n), func(b *testing.B) {
19+
// Build a mix of static and parameterised paths
20+
template := make([]URLSpec, n)
21+
for i := 0; i < n; i++ {
22+
if i%3 == 0 {
23+
template[i] = URLSpec{OASPath: fmt.Sprintf("/api/v1/resource%d/{id}", i)}
24+
} else {
25+
template[i] = URLSpec{OASPath: fmt.Sprintf("/api/v1/resource%d/static", i)}
26+
}
27+
}
28+
29+
b.ReportAllocs()
30+
b.ResetTimer()
31+
for i := 0; i < b.N; i++ {
32+
specs := make([]URLSpec, n)
33+
copy(specs, template)
34+
sortURLSpecsByPathPriority(specs)
35+
}
36+
})
37+
}
38+
}
39+
40+
// BenchmarkOASValidateRequestStaticVsParameterized measures request-time performance
41+
// for static and parameterised paths with the shield mechanism.
42+
func BenchmarkOASValidateRequestStaticVsParameterized(b *testing.B) {
43+
ts := StartTest(nil)
44+
defer ts.Close()
45+
46+
paths := openapi3.NewPaths()
47+
48+
// Parameterised path with validation
49+
paths.Set("/users/{id}", &openapi3.PathItem{
50+
Get: &openapi3.Operation{
51+
OperationID: "getUserById",
52+
Parameters: openapi3.Parameters{
53+
&openapi3.ParameterRef{
54+
Value: &openapi3.Parameter{
55+
Name: "id",
56+
In: "path",
57+
Required: true,
58+
Schema: &openapi3.SchemaRef{
59+
Value: &openapi3.Schema{
60+
Type: &openapi3.Types{"integer"},
61+
},
62+
},
63+
},
64+
},
65+
},
66+
Responses: openapi3.NewResponses(
67+
openapi3.WithStatus(200, &openapi3.ResponseRef{
68+
Value: &openapi3.Response{
69+
Description: stringPtrHelper("Success"),
70+
},
71+
}),
72+
),
73+
},
74+
})
75+
76+
// Static path without validation
77+
paths.Set("/users/admin", &openapi3.PathItem{
78+
Get: &openapi3.Operation{
79+
OperationID: "getAdminUser",
80+
Responses: openapi3.NewResponses(
81+
openapi3.WithStatus(200, &openapi3.ResponseRef{
82+
Value: &openapi3.Response{
83+
Description: stringPtrHelper("Success"),
84+
},
85+
}),
86+
),
87+
},
88+
})
89+
90+
// Add extra static paths to test scaling
91+
for i := 0; i < 50; i++ {
92+
opID := fmt.Sprintf("getResource%d", i)
93+
paths.Set(fmt.Sprintf("/resources/item%d", i), &openapi3.PathItem{
94+
Get: &openapi3.Operation{
95+
OperationID: opID,
96+
Responses: openapi3.NewResponses(
97+
openapi3.WithStatus(200, &openapi3.ResponseRef{
98+
Value: &openapi3.Response{
99+
Description: stringPtrHelper("Success"),
100+
},
101+
}),
102+
),
103+
},
104+
})
105+
}
106+
107+
doc := openapi3.T{
108+
OpenAPI: "3.0.0",
109+
Info: &openapi3.Info{Title: "Benchmark API", Version: "1.0.0"},
110+
Paths: paths,
111+
}
112+
113+
oasAPI := oas.OAS{T: doc}
114+
oasAPI.SetTykExtension(&oas.XTykAPIGateway{
115+
Middleware: &oas.Middleware{
116+
Operations: oas.Operations{
117+
"getUserById": {
118+
ValidateRequest: &oas.ValidateRequest{Enabled: true},
119+
},
120+
},
121+
},
122+
})
123+
124+
api := ts.Gw.BuildAndLoadAPI(func(spec *APISpec) {
125+
spec.Name = "Benchmark API"
126+
spec.APIID = "benchmark-api"
127+
spec.Proxy.ListenPath = "/api/"
128+
spec.UseKeylessAccess = true
129+
spec.IsOAS = true
130+
spec.OAS = oasAPI
131+
})[0]
132+
133+
require.NotNil(b, api)
134+
135+
b.Run("StaticPath_ShieldHit", func(b *testing.B) {
136+
req := httptest.NewRequest(http.MethodGet, "/api/users/admin", nil)
137+
b.ReportAllocs()
138+
b.ResetTimer()
139+
for i := 0; i < b.N; i++ {
140+
rec := httptest.NewRecorder()
141+
ts.TestServerRouter.ServeHTTP(rec, req)
142+
}
143+
})
144+
145+
b.Run("ParameterizedPath_ValidationHit", func(b *testing.B) {
146+
req := httptest.NewRequest(http.MethodGet, "/api/users/123", nil)
147+
b.ReportAllocs()
148+
b.ResetTimer()
149+
for i := 0; i < b.N; i++ {
150+
rec := httptest.NewRecorder()
151+
ts.TestServerRouter.ServeHTTP(rec, req)
152+
}
153+
})
154+
}

0 commit comments

Comments
 (0)