Skip to content

[TT-16890] Validate middleware speicifc fix#7974

Merged
lghiur merged 23 commits intomasterfrom
TT-16890-v2
Apr 15, 2026
Merged

[TT-16890] Validate middleware speicifc fix#7974
lghiur merged 23 commits intomasterfrom
TT-16890-v2

Conversation

@andrei-tyk
Copy link
Copy Markdown
Contributor

Description

When two OAS endpoints share the same base path but differ only in path parameter schemas (e.g., /employees/{prct} with pattern: ^[a-z]$ vs /employees/{zd} with pattern: [1-9]), both compile to the identical regex ^/employees/([^/]+)$. The gateway's FindSpecMatchesStatus returns the first match, so one endpoint's validation rules are always ignored.

This fix introduces a two-step disambiguation mechanism scoped to the validate request middleware:

  1. Compilation: A new groupCollapsedValidateRequestSpecs function detects URLSpec entries that compile to the same regex+method pair and groups them as candidates on a single representative URLSpec.

  2. Request processing: When a URLSpec has multiple candidates, processRequestWithCandidates constructs the OAS route directly for each candidate (bypassing the OAS router which cannot distinguish structurally identical parameterized paths) and runs full openapi3filter.ValidateRequest per candidate. The first candidate that passes full validation (including path parameter schema, headers, query params, body) wins. If no candidate matches, the request is rejected with a clear error.

Key design decisions

  • Scoped to validate request only — no changes to FindSpecMatchesStatus or core URLSpec matching, avoiding blast radius to other middleware.
  • Full schema validation for disambiguation — uses openapi3filter.ValidateRequest per candidate rather than just regex pattern matching, correctly handling type, enum, format, and other JSON Schema constraints on path parameters.
  • Direct route construction — constructs routers.Route directly from the OAS spec path items instead of using the OAS router, which cannot distinguish between structurally identical parameterized paths.
  • Deterministic candidate ordering — candidates are sorted alphabetically by OAS path for consistent behavior.

Related Issue

TT-16890

Motivation and Context

OAS API definitions can have multiple endpoints with the same path structure but different path parameter schemas and different validation requirements (e.g., different required headers). Before this fix, only one endpoint's validation rules would ever be applied, causing incorrect validation behavior for the other endpoint(s).

How This Has Been Tested

Unit test (TestGroupCollapsedValidateRequestSpecs): Verifies the grouping logic handles no-collision, same regex+method collision, different-method non-collision, and three-way collision cases.

Integration test (TestSameBasePathDifferentParamSchemas): Sets up two endpoints (/employees/{prct} with pattern ^[a-z]$ requiring header def, and /employees/{zd} with pattern [1-9] requiring header abc) and verifies:

  • /employees/a with header def -> 200 (matches {prct})
  • /employees/5 with header abc -> 200 (matches {zd})
  • /employees/a without header -> 422 (matches {prct} but fails header validation)
  • /employees/5 without header -> 422 (matches {zd} but fails header validation)
  • /employees/!!! -> 422 (matches neither param schema)

All existing tests pass with no regressions: TestValidateRequest*, TestStaticPathTakesPrecedenceOverParameterised, TestSortURLSpecsByPathPriority, TestStaticPathPriorityWithPrefixMatching, TestMockResponseStaticPathPriority.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

API Changes

--- prev.txt	2026-04-15 14:06:16.099725738 +0000
+++ current.txt	2026-04-15 14:06:07.343690786 +0000
@@ -11468,6 +11468,14 @@
 
 func (m *MockReadCloser) Read(p []byte) (n int, err error)
 
+type MockResponseCandidate struct {
+	OASMockResponseMeta *oas.MockResponse
+	OASMethod           string
+	OASPath             string
+}
+    MockResponseCandidate represents one OAS endpoint that maps to the same
+    compiled regex pattern for mock response disambiguation.
+
 type Monitor struct {
 	Gw *Gateway `json:"-"`
 }
@@ -12918,6 +12926,16 @@
 	OASValidateRequestMeta    *oas.ValidateRequest
 	OASMockResponseMeta       *oas.MockResponse
 
+	// OASValidateRequestCandidates holds multiple OAS endpoints that compile to the
+	// same regex pattern. When non-empty, the validate request middleware must
+	// disambiguate by checking path parameter schemas against each candidate.
+	OASValidateRequestCandidates []ValidateRequestCandidate
+
+	// OASMockResponseCandidates holds multiple OAS endpoints that compile to the
+	// same regex pattern. When non-empty, the mock response middleware must
+	// disambiguate by checking path parameter schemas against each candidate.
+	OASMockResponseCandidates []MockResponseCandidate
+
 	IgnoreCase bool
 	// OASMethod stores the HTTP method for OAS-specific middleware
 	// This is needed because OAS operations are method-specific
@@ -13020,6 +13038,16 @@
     ProcessRequest will run any checks on the request on the way through the
     system, return an error to have the chain fail
 
+type ValidateRequestCandidate struct {
+	OASValidateRequestMeta *oas.ValidateRequest
+	OASMethod              string
+	OASPath                string
+}
+    ValidateRequestCandidate represents one OAS endpoint that maps to the
+    same compiled regex pattern. Used for disambiguation when multiple
+    parameterized paths collapse to the same regex (e.g., /employees/{prct} and
+    /employees/{zd}).
+
 type ValueExtractor struct {
 	BaseExtractor
 }

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 7, 2026

This PR resolves an issue where OAS endpoints with identical parameterized path structures but differing parameter schemas (e.g., /users/{id:integer} vs. /users/{name:string}) resulted in incorrect validation, as only the first matched endpoint's rules were applied.

A two-phase disambiguation mechanism has been introduced, specifically scoped to the OAS request validation and mock response middleware, to address this without altering the core routing engine.

  1. Compile-Time Grouping: During API definition loading, a new function (groupCollapsedSpecs) identifies endpoints that compile to the same regex and HTTP method. These endpoints are grouped as "candidates" within a single URLSpec. These candidates are then sorted by the restrictiveness of their path parameter schemas (sortByRestrictiveness), ensuring more specific types like integer are evaluated before general ones like string.

  2. Request-Time Disambiguation: When a request matches a URLSpec containing these candidates, the middleware (processRequestWithCandidates) iterates through the sorted list. It performs a lightweight validation of the request's path parameters against each candidate's schema. The first candidate that matches is selected, and its full validation or mock response logic is executed. If no candidate's path parameters match, the request is rejected.

This change ensures that requests are correctly validated against the specific endpoint whose schema—including path parameter types, patterns, and formats—they match.

Files Changed Analysis

The changes are concentrated within the gateway/ package. A substantial portion of the 3,782 new lines is dedicated to comprehensive testing, which underscores the complexity of the scenarios addressed by this fix.

  • gateway/api_definition.go: Introduces the new compile-time logic for detecting, grouping, and sorting colliding OAS path specifications.
  • gateway/model_urlspec.go: The URLSpec struct has been updated to store the grouped OASValidateRequestCandidates and OASMockResponseCandidates.
  • gateway/mw_oas_validate_request.go: Implements the core request-time disambiguation logic (processRequestWithCandidates) that iterates through candidates and pre-validates path parameters to select the correct endpoint rules.
  • gateway/mw_mock_response.go: Mirrors the validation middleware changes, applying the same candidate resolution logic for mock responses.
  • gateway/model_apispec.go: Adds a shared helper function (matchCandidatePath) for the disambiguation logic.
  • ..._test.go files: Two new test files (mw_mock_response_collapsed_test.go, mw_oas_validate_request_scenarios_test.go) and significant additions to an existing one provide a robust suite of unit and integration tests covering numerous hierarchy and priority scenarios.

Architecture & Impact Assessment

  • What this PR accomplishes: It correctly disambiguates and applies middleware for OAS endpoints that have structurally identical parameterized paths but different parameter schemas, fixing a critical validation flaw.
  • Key technical changes introduced: The primary change is the implementation of a "candidate" system. At compile-time, conflicting endpoints are grouped and sorted by schema restrictiveness. At request-time, a pre-validation step on path parameters determines the correct candidate before the full middleware logic is executed.
  • Affected system components: The changes are narrowly scoped to the OAS Request Validation and Mock Response middleware, along with the API definition loading process. The core gateway routing engine (FindSpecMatchesStatus) remains untouched to minimize the blast radius of the change.

Disambiguation Flow

sequenceDiagram
    participant Loader as API Definition Loader
    participant Middleware as OAS Validate/Mock Middleware
    participant Client as API Client

    Note over Loader: On API Load
    Loader->>Loader: groupCollapsedSpecs(): Find endpoints with same regex + method
    Loader->>Loader: sortByRestrictiveness(): Sort candidates (integer > string with pattern > string)
    Loader-->>Middleware: Store sorted candidates in URLSpec

    Note over Middleware: On Incoming Request
    Client->>Middleware: GET /employees/123
    Middleware->>Middleware: Find URLSpec matching request path
    alt URLSpec has candidates
        Middleware->>Middleware: processRequestWithCandidates()
        loop For each candidate (most restrictive first)
            Middleware->>Middleware: Does "123" match candidate's path param schema?
            opt Path param schema matches
                Middleware->>Middleware: Commit to this candidate
                Middleware->>Middleware: Run full validation/mock logic & break
            end
        end
    else No candidates
        Middleware->>Middleware: Use default URLSpec logic
    end
Loading

Scope Discovery & Context Expansion

  • The changes are well-contained within the OAS middleware. The architectural decision to not modify the core FindSpecMatchesStatus function successfully limits the impact.
  • The introduction of custom pre-validation logic (pathParamsMatchOperation, valueMatchesSchema) creates a maintenance consideration: this logic must be kept in sync with the behavior of the underlying kin-openapi library to prevent future inconsistencies.
  • The solution is applied consistently to both ValidateRequest and MockResponse middleware, demonstrating a cohesive approach to resolving this ambiguity across different features.
  • The addition of over 3,000 lines of tests in new, dedicated files underscores the complexity and the number of edge cases considered, such as type hierarchies (integer vs. number), pattern length, format constraints, and interactions with static paths.
Metadata
  • Review Effort: 4 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-15T14:06:38.601Z | Triggered by: pr_updated | Commit: e626227

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 7, 2026

\n\n \n\n

Performance Issues (3)

Severity Location Issue
🟡 Warning gateway/mw_oas_validate_request.go:151
The function `valueMatchesSchema` calls `tykregexp.MatchString(s.Pattern, value)` inside a loop that processes request candidates. This can lead to repeated compilation of the same regular expression for each request, which is inefficient. Regular expressions should be compiled once at API load time and reused for matching on each request.
💡 SuggestionCache the compiled regular expressions from the OAS schema at API load time. Create a mechanism to store and retrieve these compiled regex objects, for example, in a map within the `APISpec` or `URLSpec` structs, keyed by the OAS path and parameter name. The `valueMatchesSchema` function should then use the pre-compiled object for matching instead of recompiling the pattern string on every request.
🟡 Warning gateway/mw_oas_validate_request.go:185
The `valueMatchesFormat` function uses `tykregexp.MatchString` with a hardcoded UUID pattern. This regex will be recompiled on every request that requires UUID format validation for a path parameter. This is inefficient and adds unnecessary overhead to request processing.
💡 SuggestionCompile the UUID regex pattern once and store it as a global or package-level variable. Use the pre-compiled regex object for matching within the `valueMatchesFormat` function to avoid repeated compilation on each request.
🟡 Warning gateway/api_definition.go:161
The `removeIndices` function creates a new slice and appends elements to it in a loop. While it correctly pre-allocates capacity, a more efficient approach for removing elements from a slice is to use a two-pointer technique to modify the slice in-place. This avoids the memory allocation and copying overhead of creating a new slice, especially if the number of specs is large.
💡 SuggestionModify the `removeIndices` function to perform an in-place removal. Iterate through the slice, maintaining a separate index for writing. If an element is not in the `toRemove` map, copy it to the current write position and increment the write index. Finally, return a sub-slice of the original slice up to the final write index. This avoids allocating a new slice.

Quality Issues (3)

Severity Location Issue
🟠 Error gateway/mw_oas_validate_request.go:170-264
The functions `pathParamsMatchOperation`, `valueMatchesSchema`, and `valueMatchesFormat` re-implement a subset of the validation logic from the `kin-openapi` library. This custom logic is used as a pre-filter to select a validation candidate. If this logic diverges from the behavior of the `kin-openapi` library, it can lead to incorrect candidate selection, causing valid requests to be rejected or routed to the wrong validation rules. This tight coupling with the internal behavior of a dependency significantly impacts maintainability and creates a high risk of future bugs.
💡 SuggestionAdd a prominent comment at the top of `pathParamsMatchOperation` explaining that this is a performance-optimized re-implementation of `kin-openapi`'s path parameter validation and must be kept in sync with that library's behavior. Ideally, explore if `kin-openapi` exposes a way to validate only path parameters to avoid this duplication.
🟡 Warning gateway/api_definition.go:198-228
The function `schemaRestrictiveness` uses hardcoded integer literals (7, 6, 5, etc.) to represent the restrictiveness score of different schema types. While the current logic is correct, using named constants would make the code more self-documenting and easier to maintain. It would clarify the intended hierarchy of restrictiveness without requiring a reader to parse the whole function.
💡 SuggestionDefine constants for the scores, for example: ```go const ( scoreUnconstrainedString = 0 scoreConstrainedString = 2 scoreObject = 3 scoreArray = 4 scoreBoolean = 5 scoreNumber = 6 scoreInteger = 7 ) ``` Then use these constants within the function.
🟡 Warning gateway/mw_oas_validate_request.go:254-255
The format validation for "email" only checks for the presence of an "@" symbol. This is a very weak check and may not align with the more comprehensive validation performed by `kin-openapi`'s full validator. While this is a pre-filter, a significant discrepancy could exist.
💡 SuggestionEither improve the regex to be more compliant with common email patterns or add a comment acknowledging that this is a minimal check and the full validation is relied upon for correctness. Given this is a pre-filter, a comment is likely sufficient.

Powered by Visor from Probelabs

Last updated: 2026-04-15T14:06:07.970Z | Triggered by: pr_updated | Commit: e626227

💡 TIP: You can chat with Visor using /visor ask <your question>

@lghiur
Copy link
Copy Markdown
Collaborator

lghiur commented Apr 14, 2026

C Reliability Rating on New Code (required ≥ A)

These are mostly small linting issues, like duplicate of error message, and false report on increasing function complexity.

@andrei-tyk
Copy link
Copy Markdown
Contributor Author

andrei-tyk commented Apr 14, 2026

@probelabs I think your CRITICAL finding about regexp.MatchString is an already existing problem that requires elevated privileges such as API creation for it to work and therefore it's not CRITICAL for this Pull request

@lghiur lghiur closed this Apr 15, 2026
@lghiur lghiur reopened this Apr 15, 2026
@lghiur lghiur merged commit d89cf0b into master Apr 15, 2026
80 of 97 checks passed
@lghiur lghiur deleted the TT-16890-v2 branch April 15, 2026 15:40
@ilijabojanovic
Copy link
Copy Markdown
Member

/release to release-5.8

@ilijabojanovic
Copy link
Copy Markdown
Member

/release to release-5.12.1

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 15, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8025

@ilijabojanovic
Copy link
Copy Markdown
Member

/release to release-5.8.13

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 15, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8026

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 15, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8027

@ilijabojanovic
Copy link
Copy Markdown
Member

/release to release-5.12.1

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 15, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8030

@ilijabojanovic
Copy link
Copy Markdown
Member

/release to release-5.12.1

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 16, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8060

@lghiur
Copy link
Copy Markdown
Collaborator

lghiur commented Apr 17, 2026

/release to release-5.12

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 17, 2026

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8066

buger added a commit that referenced this pull request Apr 17, 2026
Backport of [TT-16890] from master to release-5.12.1.
Adds collapsed parameterized path disambiguation for validate-request
and mock-response OAS middleware.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Backport of [TT-16890] from master to release-5.12.
Adds collapsed parameterized path disambiguation for validate-request
and mock-response OAS middleware.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
## Summary
Clean backport of #7974 to release-5.12.1. Replaces #8060 which had
cherry-pick issues (empty diff, 0 changed files).

- Adds collapsed parameterized path disambiguation for validate-request
and mock-response OAS middleware
- Cherry-picked from merge commit d89cf0b with conflict resolution for
the 2-value `mockResponse` return signature on this branch

## Test plan
- [ ] Unit Tests & Linting passes
- [ ] CI Tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Cherry-pick critical fixes that were on release-5.12.1 but missing
from release-5.12:
- #8029: [TT-16890] validate request middleware regression fix
- #8067: backport #7974 validate middleware collapsed path fix
- #7862: dependency updates in go.mod and go.sum

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
## Summary
Sync critical commits that are on release-5.12.1 but missing from
release-5.12.

The branches diverged on Feb 16 with no merge flow between them. This PR
brings release-5.12 up to parity with release-5.12.1 for critical fixes.

## Missing commits synced
- #8029: [TT-16890] validate request middleware regression fix
(CRITICAL)
- #8067: backport #7974 validate middleware collapsed path fix
- #7862: dependency updates in go.mod and go.sum

## Test plan
- [ ] Unit Tests & Linting passes
- [ ] go build ./gateway/... passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

[TT-16890]:
https://tyktech.atlassian.net/browse/TT-16890?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Backport of collapsed parameterized path disambiguation fix from master to release-5.8.
Applied on top of the #7972 backport (first commit on this branch).

Conflict resolution:
- Replaced `pkg/schema.RestoreUnicodeEscapesInError` with `lib.RestoreUnicodeEscapesInError`
  (pkg/schema doesn't exist on release-5.8)
- Kept `lib "github.com/TykTechnologies/tyk/lib/apidef"` import instead of pkg/schema
- Added `tykregexp` import for path parameter pattern matching

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Backport of collapsed parameterized path disambiguation fix from master to release-5.8.
Rebased on top of #8071 (#7972 backport).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
…ix (#8073)

## Summary
Backport of #7974 to release-5.8. Follow-up fix for collapsed
parameterized path disambiguation in validate-request and mock-response
OAS middleware.

This branch includes two commits:
1. Backport of #7972 (prerequisite) -- static path shields and path
priority sorting
2. Backport of #7974 -- collapsed parameterized path disambiguation

Conflict resolution (vs master):
- Replaced `pkg/schema.RestoreUnicodeEscapesInError` with
`lib.RestoreUnicodeEscapesInError` (`pkg/schema` doesn't exist on
release-5.8)
- Removed `internal/mcp` import and MCP-related functions (not present
on release-5.8)
- Added `internal/oasutil` import with exported `PathLess` and
`PathParamRegex`

## Test plan
- [ ] Unit Tests & Linting passes
- [ ] `go build ./gateway/...` passes (verified locally)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Backport of collapsed parameterized path disambiguation fix from master to release-5.8.13.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
Backport of collapsed parameterized path disambiguation fix from master
to release-5.8.13. Rebased on top of #8072 (#7972 backport).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buger added a commit that referenced this pull request Apr 17, 2026
…ix (#8074)

## Summary
Backport of #7974 to release-5.8.13. Follow-up fix for collapsed
parameterized path disambiguation.

- Adds `groupCollapsedValidateRequestSpecs` and
`groupCollapsedMockResponseSpecs` to detect and group URLSpec entries
with identical compiled regex patterns
- Adds candidate-based disambiguation using path parameter schema
validation
- Adds `matchCandidatePath` for shared disambiguation logic
- Removes `lib.RestoreUnicodeEscapesInError` reference (not available on
this branch)

**Note:** This PR includes the #7972 backport commit as a prerequisite
(stacked PR). Merge #7972 backport first:
#8072

## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants