[TT-15160][TT-16062] Header request validation fix

[lghiur]

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

Ticket Details

TT-15160

Status	Blocked
Summary	Tyk Skips Validation of Array-Type Parameter Schema when Request contains multi-valued header

Generated at: 2025-11-08 17:35:45

[github-actions]

🚨 Jira Linter Failed

Commit: 3133a46
Failed at: 2025-11-08 17:35:46 UTC

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

🔍 Click to view error details

failed to validate Jira issue: jira ticket TT-15160 has status 'Blocked' but must be one of: Ready For Dev, Dod Check, In Dev, In Code Review

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.

[github-actions]

API Changes

no api changes detected

[probelabs]

🔍 Code Analysis Results

This PR fixes a bug in the OpenAPI (OAS) request validation middleware where requests with multiple values for the same header would incorrectly fail validation.

The root cause was that the validation library expected a single, comma-separated value for headers like Accept, but the Go http.Request object preserves multiple values as a slice of strings.

The solution introduces a header normalization step that runs just before validation. This step creates a temporary, normalized version of the request headers:

• Multiple values for standard headers are combined into a single comma-separated string (e.g., Accept: text/html, Accept: application/json becomes Accept: text/html,application/json).
• Special headers that cannot be combined (e.g., Set-Cookie) are handled correctly by using only the first value.
• To prevent side effects in other middleware, this normalization is performed on a clone of the request object, leaving the original request unmodified.

The changes are accompanied by extensive unit tests for the new normalization logic and an integration test to confirm the end-to-end fix.

Files Changed Analysis

• gateway/mw_oas_validate_request.go (+74, -2): Implements the core logic for header normalization. It introduces normalizeHeaders to combine header values and cloneRequestWithNormalizedHeaders to apply this logic safely on a request copy before validation.
• gateway/mw_oas_validate_request_test.go (+274, -0): Adds comprehensive tests. This includes unit tests for the new helper functions (TestNormalizeHeaders, TestCloneRequestWithNormalizedHeaders, TestContainsComma) and a new integration test (TestValidateRequest_DuplicateHeaders) that verifies the fix within a running gateway instance.

Architecture & Impact Assessment

• What this PR accomplishes: It makes the OAS request validation middleware compliant with standard HTTP practices for handling duplicate headers, fixing a bug that rejected valid requests.
• Key technical changes introduced:
  1. Header Normalization Logic: A new normalizeHeaders function intelligently combines multi-value headers into a single string, with special handling for headers that cannot be merged (e.g., Set-Cookie).
  2. Non-destructive Middleware Behavior: The request is cloned before its headers are normalized for validation. This ensures the original request object remains untouched as it passes through the rest of the middleware chain, preventing unintended side effects.
  3. Middleware Flow Update: The ValidateRequest.ProcessRequest function is updated to use the cloned, normalized request for validation instead of the original one.
• Affected system components: The change is highly localized to the OAS Request Validation Middleware within the Tyk Gateway. It only affects APIs that have this feature enabled. Downstream services are unaffected as they receive the original, unmodified request.

Visualization

This diagram illustrates how the middleware processes a request with duplicate headers before and after the change.

sequenceDiagram
    participant Client
    participant Gateway
    participant ValidateRequestMiddleware as OAS Validator
    participant Upstream

    Client->>Gateway: GET /path
Header: Accept: text/html
Header: Accept: application/json

    Gateway->>OAS Validator: ProcessRequest(originalReq)
    
    Note over OAS Validator: Clones request and normalizes headers
    OAS Validator->>OAS Validator: normalizedReq = cloneRequestWithNormalizedHeaders(originalReq)
    Note right of OAS Validator: normalizedReq.Header["Accept"] is now "text/html,application/json"

    OAS Validator->>OAS Validator: Validate(normalizedReq)
    Note right of OAS Validator: Validation succeeds

    OAS Validator-->>Gateway: Next()
    Gateway->>Upstream: Forward originalReq (unmodified)

Loading

Scope Discovery & Context Expansion

• Broader Scope: The change is confined to the gateway module, specifically the mw_oas_validate_request.go file. Its impact is limited to the pre-processing step before invoking the kin-openapi/openapi3filter validation library.
• Context: This is a pure bug fix that improves the robustness of an existing feature. It doesn't introduce new configuration or alter existing APIs. The primary goal is to align the gateway's behavior with common HTTP practices and prevent valid requests from being rejected. The extensive testing added in mw_oas_validate_request_test.go provides high confidence that the fix is correct and does not introduce regressions.

Metadata

• Review Effort: 3 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2025-11-08T17:40:24.554Z | Triggered by: opened | Commit: 3133a46

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

[probelabs]

🔍 Code Analysis Results

Security Issues (2)

Severity	Location	Issue
🟠 Error	gateway/mw_oas_validate_request.go:51-66	The `normalizeHeaders` function modifies HTTP headers for validation, but the original, unmodified request is forwarded to the upstream service. When multiple values are provided for a header, the function may use only the first value for validation while discarding the rest (e.g., for 'special' headers or headers containing a comma). This creates a desynchronization where the upstream application processes unvalidated header values, potentially leading to a bypass of security controls like authorization, SQL injection filters, or XSS defenses. This is a Time-of-Check to Time-of-Use (TOCTOU) vulnerability. 💡 Suggestion To mitigate this, the gateway must ensure the validated request is identical to the one forwarded upstream. The recommended approach is to reject requests that would result in dropped header values (i.e., multiple values for special headers or headers containing commas) with a `400 Bad Request` status. Alternatively, modify the original request object (`r.Header`) to match the normalized headers, though this could be a breaking change for upstream services that legitimately expect multiple header values.
🟡 Warning	gateway/mw_oas_validate_request.go:29-38	The `specialHeaders` map includes several headers that are defined as HTTP response headers (e.g., `Set-Cookie`, `Www-Authenticate`, `Location`), not request headers. Applying response header semantics during request validation is incorrect. Additionally, for headers that must not appear more than once in a request (e.g., `Content-Type`, `Content-Length` per RFC 7231), the implementation permissively picks the first value instead of rejecting the request as malformed. This can lead to security risks if the upstream server interprets the malformed request differently than the gateway. 💡 Suggestion Review the `specialHeaders` list and remove headers that are not applicable to HTTP requests. For headers that are syntactically incorrect when duplicated (e.g., `Content-Type`), modify the logic to reject the request with a `400 Bad Request` status instead of attempting to sanitize it by picking the first value.

Architecture Issues (2)

Severity	Location	Issue
🟠 Error	gateway/mw_oas_validate_request.go:58	The `normalizeHeaders` function incorrectly handles multi-value headers when one of the values already contains a comma. It discards all subsequent values, which violates RFC 7230 for standard comma-joinable headers like `Accept`. For example, a request with `Accept: text/html, application/xhtml+xml` and `Accept: application/xml;q=0.9` would be incorrectly truncated to just the first value. This leads to a loss of information and can cause the OAS validator to fail valid requests. 💡 Suggestion Remove the `if containsComma(values)` block. The default behavior should be to combine values with a comma. If certain headers are known to cause issues when combined, they should be added to the `specialHeaders` map as exceptions.
🟡 Warning	gateway/mw_oas_validate_request_test.go:80	The unit test 'case insensitive header names' is non-deterministic because it relies on Go's map iteration order, which is not guaranteed. The test initializes an `http.Header` with two keys that are case-insensitively identical (`content-type` and `Content-Type`), making the test's result unpredictable. The weak assertion (`assert.NotEmpty`) masks this flaw. 💡 Suggestion Rewrite the test to be deterministic. A reliable way to test case-insensitivity is to use a single header with a non-canonical name (e.g., `x-my-header`) and verify that the normalized header map contains the canonical key (`X-My-Header`) with the correct value.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_oas_validate_request.go:155-158	Unconditional request cloning and header processing on every request introduces performance overhead. The function `cloneRequestWithNormalizedHeaders` is called for every request passing through the OAS validation middleware, causing unnecessary memory allocations and GC pressure on the hot path, even when headers do not need normalization. 💡 Suggestion Avoid cloning the request and normalizing headers unless necessary. Add a preliminary check to see if any header has multiple values. If not, use the original request for validation. This will optimize the common path where normalization is not required. 🔧 Suggested Fix // In ProcessRequest function, replace the unconditional call: reqToValidate := r if needsNormalization(r.Header) { reqToValidate = cloneRequestWithNormalizedHeaders(r) } requestValidationInput := &openapi3filter.RequestValidationInput{ Request: reqToValidate, PathParams: operation.pathParams, Route: operation.route, Options: &openapi3filter.Options{ AuthenticationFunc: openapi3filter.NoopAuthenticationFunc, }, } // Add this helper function to the file: func needsNormalization(h http.Header) bool { for _, values := range h { if len(values) > 1 { return true } } return false }

Quality Issues (2)

Severity	Location	Issue
🟡 Warning	gateway/mw_oas_validate_request_test.go:311	The 'case insensitive header names' test is unreliable due to its dependency on Go's non-deterministic map iteration order. It initializes an `http.Header` with `content-type` and `Content-Type`, which canonicalize to the same key. The final value is unpredictable, and the test's `assert.NotEmpty` is too weak to properly validate the behavior. 💡 Suggestion Rewrite the test to be deterministic. Set a header with a non-canonical key (e.g., `x-custom-header`) and assert that its value can be retrieved using the canonical key (`X-Custom-Header`). 🔧 Suggested Fix t.Run("case insensitive header names", func(t *testing.T) { headers := http.Header{ "x-custom-header": []string{"value1", "value2"}, } normalized := normalizeHeaders(headers) assert.Equal(t, "value1,value2", normalized.Get("X-Custom-Header")) })
🟡 Warning	gateway/mw_oas_validate_request_test.go:354	The assertion to verify that the original request remains unchanged is weak. `originalHeaders.Get("Accept")` returns only the first value (`"text/html"`), so the `assert.NotEqual` against the combined string will always pass. This does not guarantee the original header slice was not mutated. 💡 Suggestion Strengthen the assertion by directly comparing the original header's value slice to its expected state. 🔧 Suggested Fix assert.Equal(t, []string{"text/html", "application/json"}, req.Header["Accept"])

✅ Dependency Check Passed

No dependency issues found – changes LGTM.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.

---

Powered by Visor from Probelabs

Last updated: 2025-11-08T17:40:25.531Z | Triggered by: opened | Commit: 3133a46

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud