Skip to content

fix: optimize OAS path ordering by only adding synthetic entries for conflicting paths#7966

Open
probelabs[bot] wants to merge 3 commits intofix-oas-path-ordering-approach-1-prfrom
fix-oas-path-ordering-optimized
Open

fix: optimize OAS path ordering by only adding synthetic entries for conflicting paths#7966
probelabs[bot] wants to merge 3 commits intofix-oas-path-ordering-approach-1-prfrom
fix-oas-path-ordering-optimized

Conversation

@probelabs
Copy link
Copy Markdown
Contributor

@probelabs probelabs bot commented Apr 3, 2026

Problem / Task

The original fix for OAS path ordering (PR 7964) added synthetic disabled entries for EVERY static path that didn't have middleware enabled. This caused the regex list to grow linearly with the number of static paths, which could impact performance for APIs with many static paths.

Additionally, using oasutil.SortByPathLength caused a prefix matching flaw where parameterized paths with more fragments were evaluated before static paths with fewer fragments. If enable_suffix: false (prefix matching enabled), a request to /employees/static/abc/def would incorrectly hit the parameterized path instead of the static prefix.

Changes

  • Optimized the path compilation logic to only add synthetic disabled entries for static paths that actually conflict with a parameterized path (e.g., /users/admin conflicts with /users/{id}, but /health does not).
  • Implemented a custom sorting function (sortPathsForOASMiddleware) specifically for OAS middleware compilation to enforce strict precedence:
    1. ALL static paths must come before ANY parameterized paths.
    2. For parameterized paths, prioritize longer static prefixes (e.g., /employees/static/{id} before /employees/{id}/static).
    3. Sort by fragments descending, then length descending.
  • Updated compileOASValidateRequestPathSpec and compileOASMockResponsePathSpec to use this new sorting function.

Testing

  • Extended the benchmark test to include 100 static paths.
  • Added TestOASValidateRequestPrefixMatching to verify prefix matching behavior (/employees/static vs /employees/{id}/abc/def).
  • Ran go test ./gateway -run=TestOAS and go test ./gateway -run=TestMockResponse to ensure no regressions.

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs bot commented Apr 3, 2026

This PR introduces a significant optimization to the OpenAPI Specification (OAS) path ordering and compilation logic, addressing both a performance issue and a prefix matching bug.

The key change is moving from a brute-force approach—where synthetic "shield" entries were created for all static paths—to a more intelligent, conflict-based system. Now, the gateway first compiles a list of regular expressions for only the parameterized paths that have middleware enabled. It then iterates through the static paths and creates a disabled shield entry only if a static path matches one of the parameterized regexes. This prevents the internal regex list from growing unnecessarily large for APIs with many non-conflicting static paths, improving performance.

Additionally, a new custom sorting function, sortPathsForOASMiddleware, is introduced to replace the generic oasutil.SortByPathLength. This new function enforces a stricter and more correct path precedence to fix a prefix matching flaw:

  1. All static paths are prioritized over all parameterized paths.
  2. For parameterized paths, those with longer static prefixes are given higher priority.
  3. Paths are then sorted by the number of fragments and finally by length.

These changes are validated with an expanded benchmark test that now includes 100 static paths and a new integration test (TestOASValidateRequestPrefixMatching) to confirm the prefix matching logic is correct.

Files Changed Analysis

  • gateway/api_definition.go: This file contains the core logic changes. It introduces sortPathsForOASMiddleware and updates compileOASValidateRequestPathSpec and compileOASMockResponsePathSpec to use the new sorting and the optimized two-pass algorithm for creating shield entries.
  • gateway/mw_oas_validate_request_benchmark_test.go: The benchmark test is extended by adding 100 non-conflicting static paths to demonstrate that the optimization prevents performance degradation at scale.
  • gateway/mw_oas_validate_request_path_ordering_test.go: A new test, TestOASValidateRequestPrefixMatching, is added to verify that the new sorting logic correctly handles prefix matching scenarios, ensuring a request like /employees/static/abc correctly matches the /employees/static path instead of a parameterized one like /employees/{id}/....

Architecture & Impact Assessment

  • What this PR accomplishes: It fixes a performance regression for OAS APIs with many static endpoints and corrects a critical path precedence bug that could lead to incorrect middleware execution.
  • Key technical changes introduced:
    • A new path sorting algorithm that prioritizes static paths and longer static prefixes in parameterized paths.
    • An optimized two-pass compilation strategy that only creates synthetic path entries for static paths that genuinely conflict with parameterized paths.
  • Affected system components: The primary impact is on the APIDefinitionLoader within the Tyk Gateway. It specifically alters how OAS-based API definitions are compiled into the internal URLSpec representation used for routing middleware like OASValidateRequest and OASMockResponse.

Optimized Path Compilation Flow

graph TD
    A[Start OAS Path Compilation] --> B[Sort Paths using new custom logic];
    B --> C{First Pass: Collect Regexes<br>for Parameterized Paths w/ Middleware};    
    C --> D{Second Pass: Iterate All Paths};
    D --|For Each Path|--> E{Is Path Static?};
    E --|"No (Parameterized)"|--> F[Process middleware as configured];
    E --|"Yes (Static)"|--> G{Does it conflict with a<br>parameterized regex?};
    G --|No|--> H[Ignore for this middleware];
    G --|Yes|--> I[Create synthetic disabled "shield" entry];
    F --> J[Add to URL Spec];
    I --> J;
    H --> K[End Loop];
    J --> K;
Loading

Scope Discovery & Context Expansion

  • The changes are localized to the gateway's OAS API definition loading process but have a broad impact on the runtime behavior of all OAS-based APIs managed by Tyk. The fix ensures that routing is both performant and predictable, which is critical for gateway stability and correctness.
  • This change directly affects any user leveraging OAS features like request validation or mock responses. The improved logic in compileOASValidateRequestPathSpec and compileOASMockResponsePathSpec is called during the API loading phase, meaning the benefits are realized as soon as an API definition is loaded or reloaded.
  • The new sorting logic in sortPathsForOASMiddleware is a foundational improvement that could potentially be applied to other areas where OAS path ordering is critical.
Metadata
  • Review Effort: 3 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-06T07:29:03.691Z | Triggered by: pr_updated | Commit: f3ca8f9

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

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs bot commented Apr 3, 2026

Security Issues (2)

Severity Location Issue
🟡 Warning gateway/api_definition.go:1492-1497
The algorithm to find conflicting static paths iterates through all parameterized path regexes for each static path, resulting in a time complexity of O(S*P) where S is the number of static paths and P is the number of parameterized paths. For an OpenAPI specification with thousands of static and thousands of parameterized paths, this nested loop could cause significant CPU consumption and slow API reloads, potentially leading to a denial-of-service on the gateway's control plane if a user with API definition privileges submits a very large specification.
💡 SuggestionFor very large-scale OpenAPI definitions, consider optimizing this conflict detection. One possible approach is to build a single prefix tree (trie) containing all paths. This would allow for more efficient identification of potential conflicts between static and parameterized paths, potentially reducing the algorithmic complexity. For the common case, the current implementation is a significant improvement and may be sufficient.
🟡 Warning gateway/api_definition.go:1600-1605
The algorithm to find conflicting static paths iterates through all parameterized path regexes for each static path, resulting in a time complexity of O(S*P) where S is the number of static paths and P is the number of parameterized paths. This logic is duplicated from `compileOASValidateRequestPathSpec`. For an OpenAPI specification with thousands of static and thousands of parameterized paths, this nested loop could cause significant CPU consumption and slow API reloads, potentially leading to a denial-of-service on the gateway's control plane if a user with API definition privileges submits a very large specification.
💡 SuggestionFor very large-scale OpenAPI definitions, consider optimizing this conflict detection. One possible approach is to build a single prefix tree (trie) containing all paths. This would allow for more efficient identification of potential conflicts between static and parameterized paths, potentially reducing the algorithmic complexity. For the common case, the current implementation is a significant improvement and may be sufficient.

Architecture Issues (1)

Severity Location Issue
🟠 Error gateway/api_definition.go:1523-1629
The logic for compiling OAS middleware path specifications, including the new two-pass algorithm to create "shield" entries for conflicting static paths, is almost entirely duplicated between `compileOASValidateRequestPathSpec` (starting at line 1415) and `compileOASMockResponsePathSpec` (starting at line 1523). This violates the DRY (Don't Repeat Yourself) principle, increases the maintenance burden, and makes the code harder to reason about. Any future bug fixes or enhancements to this complex logic would need to be applied in two separate places, increasing the risk of inconsistency.
💡 SuggestionRefactor the common logic into a single, generic private method. This new method could accept function arguments to handle the specific differences between middleware types, such as the function to check if a specific middleware is enabled (e.g., `ValidateRequest` vs. `MockResponse`) and the function to create the appropriate `URLSpec` struct. The existing `compileOASValidateRequestPathSpec` and `compileOASMockResponsePathSpec` functions would then become thin, clear wrappers calling this new generalized function.

Performance Issues (2)

Severity Location Issue
🟡 Warning gateway/api_definition.go:1491-1496
The logic to find conflicting static paths introduces a nested loop that could lead to poor performance during API load time for very large OpenAPI specifications. For each static path, the code iterates through all parameterized paths that have middleware enabled, performing a regex match. This results in an algorithmic complexity of O(S * M), where S is the number of static paths and M is the number of parameterized paths. For an API with thousands of static paths and hundreds of parameterized paths, this could introduce a noticeable delay in API loading or reloading.
💡 SuggestionFor APIs with a very large number of paths, consider optimizing the conflict detection mechanism. Instead of a linear scan of all parameterized regexes for each static path, a more efficient data structure could be used. For example, the static prefixes of parameterized paths could be compiled into a single optimized regex or a prefix tree (trie) to allow for faster lookups. However, for the majority of use cases, the current implementation's trade-off of slower load time for faster request time is reasonable.
🟡 Warning gateway/api_definition.go:1617-1622
This section duplicates the O(S * M) algorithmic complexity issue found in `compileOASValidateRequestPathSpec`. For each static path, it iterates through all parameterized paths with mock responses enabled to check for conflicts via regex matching. This can significantly slow down API load times for specifications with a large number of paths.
💡 SuggestionThe suggestion is the same as for the `compileOASValidateRequestPathSpec` function. Consider a more optimized data structure for conflict detection, such as a combined regex or a prefix tree, if load times for very large APIs become a concern.

Quality Issues (1)

Severity Location Issue
🟡 Warning gateway/api_definition.go:1376-1402
The logic to collect and compile regexes for parameterized paths is duplicated between `compileOASValidateRequestPathSpec` (lines 1376-1402) and `compileOASMockResponsePathSpec` (lines 1444-1470). This violates the DRY (Don't Repeat Yourself) principle, making the code harder to maintain as any future bug fixes or enhancements to this logic will need to be implemented in two separate places.
💡 SuggestionRefactor the duplicated code into a single private helper function. This function could take the `sortedPaths` and a predicate function as arguments to check whether a specific middleware (e.g., `ValidateRequest` or `MockResponse`) is enabled for an operation. The function would then return the list of compiled regular expressions, eliminating the code duplication.

Powered by Visor from Probelabs

Last updated: 2026-04-06T07:28:30.499Z | Triggered by: pr_updated | Commit: f3ca8f9

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

API Changes

no api changes detected

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

🚨 Jira Linter Failed

Commit: f3ca8f9
Failed at: 2026-04-06 07:27:25 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details
failed to validate branch and PR title rules: branch name 'fix-oas-path-ordering-optimized' must contain a valid Jira ticket ID (e.g., ABC-123)

Next Steps

  • Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
  • Verify your PR title matches the branch's Jira ticket ID
  • Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

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.

1 participant