Skip to content

[TT-12238] Javascript regex issue on OAS API#7923

Merged
radkrawczyk merged 25 commits intomasterfrom
TT-12238-javascript-regex-issue-on-oas-api-gw-fix
Apr 16, 2026
Merged

[TT-12238] Javascript regex issue on OAS API#7923
radkrawczyk merged 25 commits intomasterfrom
TT-12238-javascript-regex-issue-on-oas-api-gw-fix

Conversation

@MaciekMis
Copy link
Copy Markdown
Contributor

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@github-actions
Copy link
Copy Markdown
Contributor

API Changes

no api changes detected

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Mar 25, 2026

This PR addresses an issue with how Javascript-style regular expressions containing unicode escape sequences (e.g., \u0000) are handled in OAS API definitions. The gateway's internal RE2 regex engine uses a different format, which previously caused these patterns to be misinterpreted. This fix introduces a transformation layer to correctly process these patterns internally while preserving the original format for users.

Files Changed Analysis

  • gateway/api.go: Implements the core transformation logic at the API boundaries. On ingestion (extractOASObjFromReq), it converts Javascript unicode escapes to the RE2 format. On egress (handleGetAPIOAS, writeOASAndAPIDefToFile), it clones the cached OAS object and restores the original Javascript format before responding or writing to a file. Cloning is critical to prevent mutating the shared in-memory API definition.
  • gateway/api_definition.go: Applies the same ingestion logic for OAS APIs loaded from files (loadDefFromFilePath), ensuring consistent behavior regardless of the source.
  • gateway/api_definition_test.go: Adds a new test suite (TestLoadDefFromFilePath) to validate the file-loading process and specifically verify that the regex unicode transformation is applied correctly.

Architecture & Impact Assessment

  • What this PR accomplishes: It fixes a bug causing incorrect validation for OAS APIs that use regex patterns with unicode escapes. It ensures these patterns are correctly interpreted by the gateway's RE2 engine while preserving the original Javascript format for external representations.

  • Key technical changes introduced:

    1. Bidirectional Regex Transformation: A schema.Visitor is used to convert Javascript-style unicode escapes (\u...) to a Go-compatible RE2 format (\x{...}) on ingestion and reverts this on egress.
    2. State Isolation via Cloning: The handleGetAPIOAS and writeOASAndAPIDefToFile functions now clone the OAS object from the cache before applying the reverse transformation. This is a crucial defensive measure to prevent race conditions and state corruption of the live API definition.
    3. Consistent Handling: The transformation is applied across all entry and exit points for an OAS definition: API creation/update (HTTP), API retrieval (HTTP), file writing, and file loading.
  • Affected system components: This change impacts the lifecycle management of OAS API definitions within the Tyk Gateway, specifically any validation that relies on regex patterns with unicode characters.

OAS Regex Handling Flow

graph TD
    subgraph Ingestion [API Definition Ingestion]
        A["HTTP Request (Create/Update API) <br>w/ JS Regex \\u..."] --> B{extractOASObjFromReq};
        C["API Definition File <br>w/ JS Regex \\u..."] --> D{loadDefFromFilePath};
        E((In-Memory Cache <br> with RE2 Regex));
        B --|Transform JS \\u to RE2 \\x(...)|--> E;
        D --|Transform JS \\u to RE2 \\x(...)|--> E;
    end

    subgraph Egress [API Definition Egress]
        F["HTTP Request (Get API)"] --> G{handleGetAPIOAS};
        J["Write to File Trigger"] --> K{writeOASAndAPIDefToFile};

        G --|1. Get from Cache|--> E;
        E --|2. Clone Object|--> H[Cloned OAS Object];
        H --|3. Restore RE2 \\x(...) to JS \\u|--> I["HTTP Response <br> w/ JS Regex \\u..."];

        K --|1. Get from Cache|--> E;
        E --|2. Clone Object|--> L[Cloned OAS Object];
        L --|3. Restore RE2 \\x(...) to JS \\u|--> M["API Definition File <br> w/ JS Regex \\u..."];
    end
Loading

Scope Discovery & Context Expansion

  • The changes are well-contained within the gateway package but are fundamental to the data integrity of all OAS API definitions that use regex validation with unicode characters.
  • The core transformation logic (TransformUnicodeEscapesToRE2Manipulation and RestoreUnicodeEscapesFromRE2Manipulation) is located in the pkg/schema package (not modified in this PR), indicating a clean separation of concerns.
  • The new test TestLoadDefFromFilePath provides essential coverage for the file-loading scenario, which is a key ingestion path alongside the main HTTP API.

References

  • gateway/api.go: Implements the core logic for cloning, transformation, and restoration of OAS definitions at the API and file system boundaries.
  • gateway/api_definition.go: Applies transformation logic to definitions loaded from files.
  • gateway/api_definition_test.go: Contains new tests for the file loading and transformation logic.
Metadata
  • Review Effort: 3 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-16T05:20:59.079Z | Triggered by: pr_updated | Commit: be3b306

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

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Mar 25, 2026

Security Issues (2)

Severity Location Issue
🟡 Warning gateway/api.go:1280
The error returned from `apiOAS.Clone()` is unchecked. While the current implementation may not return an error, this could change in the future, leading to silent failures. This also affects line 1461.
💡 SuggestionCheck the error returned from `apiOAS.Clone()` and handle it appropriately, for example by logging it or returning an error response. This applies to all calls to `Clone()` where the error is currently ignored.
🟡 Warning gateway/api.go:1461
The error returned from `oasObj.Clone()` is unchecked. While the current implementation may not return an error, this could change in the future, leading to silent failures. This also affects line 1280.
💡 SuggestionCheck the error returned from `oasObj.Clone()` and handle it appropriately, for example by logging it or returning an error response. This applies to all calls to `Clone()` where the error is currently ignored.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Security Issues (2)

Severity Location Issue
🟡 Warning gateway/api.go:1280
The error returned from `apiOAS.Clone()` is unchecked. While the current implementation may not return an error, this could change in the future, leading to silent failures. This also affects line 1461.
💡 SuggestionCheck the error returned from `apiOAS.Clone()` and handle it appropriately, for example by logging it or returning an error response. This applies to all calls to `Clone()` where the error is currently ignored.
🟡 Warning gateway/api.go:1461
The error returned from `oasObj.Clone()` is unchecked. While the current implementation may not return an error, this could change in the future, leading to silent failures. This also affects line 1280.
💡 SuggestionCheck the error returned from `oasObj.Clone()` and handle it appropriately, for example by logging it or returning an error response. This applies to all calls to `Clone()` where the error is currently ignored.
\n\n ### ✅ Architecture Check Passed

No architecture issues found – changes LGTM.

\n\n

Performance Issues (2)

Severity Location Issue
🟡 Warning gateway/api.go:1282
Cloning and processing a large OAS object on every GET request can be inefficient. The `apiOAS.Clone()` operation followed by `visitor.ProcessOAS()` introduces CPU and memory overhead for each call to this endpoint. For large and complex OAS definitions, this can lead to increased latency and garbage collection pressure under load.
💡 SuggestionConsider caching the processed, public-facing OAS definition. When an API is loaded or updated, you could generate and cache both the internal representation and the public representation (with Tyk extensions removed and regexes restored). This would trade a small amount of memory for a significant performance gain on this read-heavy endpoint, turning the expensive computation into a simple cache lookup.
🟡 Warning gateway/api.go:3698
The function re-marshals the modified OAS object into `reqBodyInBytes`, but this return value appears to be unused by its callers (`handleAddApi` and `handleUpdateAPI`). This results in unnecessary CPU and memory allocation for a serialization operation whose result is immediately discarded.
💡 SuggestionVerify the usage of the returned `reqBodyInBytes` in the calling functions. If it is indeed unused, remove the `json.Marshal` call and the related error handling. The function signature can be updated to no longer return the `[]byte` slice.

Quality Issues (2)

Severity Location Issue
🟡 Warning gateway/api.go:1280
The error returned by `apiOAS.Clone()` is ignored. While the author has stated that this function currently never returns an error, this behavior could change in future library updates. Ignoring the error violates defensive programming principles and could hide future bugs.
💡 SuggestionHandle the error explicitly, even if it's just to log it. This ensures that if the function's behavior changes to return errors in the future, it will be immediately visible in logs rather than failing silently.
🟡 Warning gateway/api.go:1466
The error returned by `oasObj.Clone()` is ignored. While the author has stated that this function currently never returns an error, this behavior could change in future library updates. Ignoring the error violates defensive programming principles and could hide future bugs.
💡 SuggestionHandle the error explicitly, even if it's just to log it. This ensures that if the function's behavior changes to return errors in the future, it will be immediately visible in logs rather than failing silently.

Powered by Visor from Probelabs

Last updated: 2026-04-16T05:20:55.393Z | Triggered by: pr_updated | Commit: be3b306

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

@MaciekMis
Copy link
Copy Markdown
Contributor Author

/Visor ask Function apiOAS.Clone() always return nil as an error - that is why it is ignored here. OAS clone is returned from this function to the handlers which can modify it's state with schema visitor (and we do not want those changes to be propagated into cache).

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Mar 27, 2026


Powered by Visor from Probelabs

Last updated: 2026-03-27T09:08:02.067Z | Triggered by: MaciekMis | Commit: 5cb4b4b

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

@radkrawczyk radkrawczyk requested a review from pvormste April 8, 2026 05:25
@radkrawczyk radkrawczyk reopened this Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🚨 Jira Linter Failed

Commit: be3b306
Failed at: 2026-04-16 09:56:51 UTC

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

🔍 Click to view error details
failed to get Jira issue: failed to fetch Jira issue TT-12238: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

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.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@radkrawczyk radkrawczyk merged commit 2a8cc1e into master Apr 16, 2026
125 of 155 checks passed
@radkrawczyk radkrawczyk deleted the TT-12238-javascript-regex-issue-on-oas-api-gw-fix branch April 16, 2026 13:22
@radkrawczyk
Copy link
Copy Markdown
Contributor

/release to release-5.8

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 17, 2026

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

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