Conversation
Contributor
🎯 Recommended Merge TargetsBased on JIRA ticket TT-17921: Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight Fix Version: Tyk 5.15.0Required:
Recommended:
Fix Version: Tyk 5.13.2Required:
📋 Workflow
|
Contributor
|
This PR provides a comprehensive fix for a set of critical race conditions and state leakage issues within the GraphQL engine. The core problem was that request-specific data, such as HTTP round trippers and resolved headers, was being written to shared, long-lived state. Under concurrent load, this led to non-deterministic behavior where one request could inadvertently use data from another. The solution refactors the engine to pass all request-specific transport configuration via the request's Key changes include:
Files Changed AnalysisThe changes are spread across 22 files, with a significant net addition of nearly 3000 lines, the vast majority of which are new tests. The most notable additions are Core logic changes are concentrated in
Architecture & Impact Assessment
Architectural Flow: Before vs. AfterThe following diagrams illustrate the shift from a mutable, shared transport (prone to race conditions) to an isolated, context-scoped approach. Old Architecture (Race Condition) sequenceDiagram
participant ClientA
participant ClientB
participant GQL Engine
participant SharedClient
ClientA->>GQL Engine: Request A (with TransportA)
GQL Engine->>SharedClient: Transport = TransportA
ClientB->>GQL Engine: Request B (with TransportB)
GQL Engine->>SharedClient: Transport = TransportB (overwrites A)
GQL Engine->>SharedClient: RoundTrip(reqA)
Note right of SharedClient: Uses TransportB for Request A! (RACE)
New Architecture (Isolated) sequenceDiagram
participant ClientA
participant ClientB
participant GQL Engine
participant Transport
participant reqA Context
participant reqB Context
ClientA->>GQL Engine: Request A (with RoundTripperA)
GQL Engine->>reqA Context: SetValue(RoundTripperA)
ClientB->>GQL Engine: Request B (with RoundTripperB)
GQL Engine->>reqB Context: SetValue(RoundTripperB)
GQL Engine->>Transport: RoundTrip(reqA)
Transport->>reqA Context: GetValue() returns RoundTripperA
Note right of Transport: Uses correct RoundTripper for each request.
Scope Discovery & Context Expansion
Metadata
Powered by Visor from Probelabs Last updated: 2026-08-21T09:08:24.271Z | Triggered by: pr_updated | Commit: 1060276 💡 TIP: You can chat with Visor using |
Contributor
✅ Security Check PassedNo security issues found – changes LGTM. ✅ Architecture Check PassedNo architecture issues found – changes LGTM. Performance Issues (2)
✅ Quality Check PassedNo quality issues found – changes LGTM. Powered by Visor from Probelabs Last updated: 2026-08-21T09:08:20.031Z | Triggered by: pr_updated | Commit: 1060276 💡 TIP: You can chat with Visor using |
kofoworola
approved these changes
Aug 19, 2026
pvormste
changed the title
Three defects found while completing the credential isolation matrix. 1. Websocket subscriptions never delivered anything. The handovers passed the request context to the subscription handler, but net/http cancels that context as soon as ServeHTTP returns, before the hijacked check in conn.serve, and the gateway returns as soon as the connection is hijacked. The subscription was therefore cancelled right after connection_ack, without ever reaching the upstream. Subscriptions now run on a context built from the engine spec context with the request's transport values copied across. They keep the per-request round tripper, survive the handler returning, and end when the API is released. That last part also closes a leak: before, a reloaded API's subscriptions ran on because the library used context.Background. EngineV1 passes no context at all: its legacy data sources build the upstream request with http.NewRequest and drop the execution context, so there is nothing request scoped to carry over. 2. GraphQLProxyOnlyContextValues.upstreamResponse was written by the transport and read by the engine without a lock. For a subscription the fetch runs on the resolver's trigger goroutine, so the two raced; reproduced roughly one run in twenty under -race. Both sides now go through accessors. 3. handleProxyOnly took the method of the caller's request for every upstream call, which turned an upstream websocket handshake into a POST and got it rejected. Handshakes now keep their method. Defects 2 and 3 predate the transport scoping work.
Patches 0001 and 0002 covered execution engine mode only, over HTTP, SSE and
graphql-ws, with sequential callers. The measured defect also covers proxy-only
and supergraph, and it is a shared state defect, so the sequential cases are the
weaker ones.
Adds, in a new credential_isolation_test.go that holds the shared fixtures:
- proxy-only HTTP, SSE and graphql-ws, for both engines. The HTTP cases assert
a plain caller header reached the upstream too, which is what proves the
handleProxyOnly branch ran.
- supergraph, which is the only mode that turns on the data loader and single
flight. Both the root fetch and the _entities fetch are asserted, and
NewEngineV3 is pinned to reject supergraph.
- graphql-transport-ws, the second upstream protocol handler.
- the client websocket upgrade path, which nothing covered before and which is
where the subscription context defect hid.
- negative controls: an unauthenticated caller after an authenticated one,
StripAuthData, and a custom auth header name.
- concurrent callers, in two flavours. Credential isolation alone does not
prove transport isolation, because callers sharing one round tripper cannot
tell a shared client transport from a request scoped one, so a second test
gives every caller its own.
The tests patches 0001 and 0002 added keep their names and assertions and move
onto the shared builders.
Engine v3 has no websocket upgrade test: 3-preview starts the upstream
subscription with the right credential but never writes a payload back to the
client, independent of the subscription context.
… path Both modules go to 1ff2d4d7, which carries the schema hashing and cached plan fixes. The v2 module needed its own bump: v2/ is a separate Go module, so the earlier bump of the root module left it on the old commit and still mutating the cached plan at resolve time. With those in, the concurrent isolation tests no longer need a warm-up to stay clean under -race, so they gain a cold twin. The cold burst is what exercises the lazily initialised state that used to race: against the previous pin it reports races in Schema.Hash and in ResolveGraphQLResponse. The warm variants stay, since a cached plan is the condition the credential defect itself needs.
Applies scratchpad patch 0003 and completes its coverage. Config version 2 never resolved a dynamic upstream header where the library needs it resolved. The v1 header modifier merged Tyk's additional headers and stopped, leaving tykVariableReplacer injected but unread. That matters because the library applies the modifier before it derives the key that decides whether two callers share an upstream connection: connectionKey in the v1 subscription client, UniqueRequestID in the v2 resolver. A value still holding the literal $tyk_context token is identical for every caller, so callers hash alike and multiplex onto one upstream WebSocket opened with the first caller's credential. Twenty concurrent callers produced exactly one upstream connection. Resolving there is only safe now that graphql-go-tools f75d6891 moved header modification to the execution phase. TT-14357 had removed it precisely because the modifier ran at plan post-process time and the result was cached into the plan, and added variableReplaceRoundTripper as the workaround. That workaround is retired here. It resolved too late for the connection key, collapsed multi-value headers to their first value on the way to the wire, and expanded variables inside header values the caller had sent, which let a caller read their own session context back out of the upstream request. Its one remaining job, request_headers_rewrite, is now resolved once where the rules are built, from configuration only. Both modifiers also keep every value of a multi-value header and canonicalise the key they write, which a direct map assignment does not do on its own. Behaviour change worth noting: no value is resolved twice any more, on config version 2 or 3-preview. A value that resolved to something containing a second variable token used to get another pass.
Repairs a regression from cf506a9, found while verifying patch 0003. TestGraphQL_InternalDataSource/graphql_engine_v1 fails on the branch tip and passes at cf83ddb: a config version 1 API with a tyk:// data source answers 500 instead of proxying. Before the branch, reverseProxyPreHandlerV1 assigned params.RoundTripper onto the shared httpClient.Transport of the API. That was the cross-caller defect patch 0001 set out to remove, and it was also the only thing giving EngineV1 the gateway round tripper. Only that round tripper can route tyk://, so removing it left those fetches on the plain transport of the API client, which cannot. The per-request replacement cannot reach EngineV1: its legacy data sources build the upstream request with http.NewRequest and drop the execution context, so the context values never arrive. The round tripper is therefore handed over as a transport level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 are unaffected and stay fully request scoped. The fallback is the one piece of the engine transport shared between callers, so it is mutex guarded and refreshed per request rather than stored once, since the gateway rebuilds an API's transport when max_conn_time expires. Sharing it is only sound because the round tripper the gateway hands over no longer carries caller state - the wrapper that made it caller specific went away in the previous commit. Both halves are commented so neither gets undone alone. No concurrent EngineV1 test: the version 1 planner in graphql-go-tools is not safe for concurrent execution, with no Tyk code on the racing stacks, and pkg/execution and pkg/astvisitor are identical across the pinned versions. Pre-existing property of the deprecated config version, not this package's to guard. The thread safety that is ours is covered directly instead.
A proxy-only API with strip_auth_data off sent the credential to the upstream
twice:
UPSTREAM_SAW x-api-key=eyJ...M2E=,eyJ...M2E=
Two writers put it there and neither knows about the other. The engine adds the
caller's auth header to the fetch input through propagateAuthHeaders, then
setProxyOnlyHeaders forwards every caller header again with Header.Add, and
X-Api-Key is not one of the three keys in ignoreForwardedHeaders. With
use_immutable_headers off nothing deletes the first copy.
Neither writer is new. propagateAuthHeaders arrived in TT-16017 (bc2c9d1),
14 months after TT-14357 (adca34d) added variableReplaceRoundTripper, which
ran after setProxyOnlyHeaders and did Set(key, Get(key)) on every key. That
collapsed the duplicate, so the double-write was masked from the day it was
written. Retiring the wrapper in 4d67f9a exposed it.
setProxyOnlyHeaders now skips a value the engine already placed on the request.
Compared against a snapshot taken before forwarding rather than the header as it
grows, so a caller that really did send the same value twice keeps both, and a
configured request_headers value that differs from the caller's still reaches
the upstream alongside it - TT-11990 and TT-12190 semantics are unchanged.
Behaviour does change for any exact overlap between a configured value and a
caller value, not only for the auth header: it now goes upstream once.
Also hoists the use_immutable_headers delete out of the value loop. Inside it,
it removed the value added on the previous pass, so a caller header with more
than one value reached the upstream holding only its last one.
The tests could not see any of this. authorizationRecorder reported header
values through Header.Get and TestGraphQL_ProxyOnlyHeaders through val[0], so
all six proxy-only isolation tests passed while duplicating. They now assert the
full value list: with the fix reverted, eleven tests fail and name the duplicate.
pvormste
added
the
deps-reviewed
pvormste
enabled auto-merge (squash)
August 20, 2026 13:08
|
SentinelOne CNS Hardcoded Secret Detector SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes. |
|
Collaborator
Author
|
/release to release-5.13 |
Contributor
|
|
pvormste
added a commit
that referenced
this pull request
Aug 21, 2026
Values that belong to a single request — the round tripper, the resolved upstream headers, the upstream response — were being written into, or keyed on, state that outlives the request: the shared per-API `http.Client`, the plan cache, and the upstream connection-reuse key. Under concurrency the result was not deterministic: which request's values a fetch used depended on timing. This makes that state request-scoped, and fixes the header handling that depended on it. Seven commits, each independently revertable. `internal/graphengine` goes from 22 to 76 test functions (273 including subtests), green under `-race -count=10`. Behavioural detail and reproduction steps are on the ticket. `reverseProxyPreHandlerV1/V2.PreHandle` assigned `params.RoundTripper` onto the per-API `httpClient.Transport` on every request, so concurrent requests overwrote each other's. The round tripper and headers config now travel in the request context (`SetGraphQLEngineTransportContextValue`), and `GraphQLEngineTransport.RoundTrip` builds a per-request view from them, falling back to the client's own transport. Three defects, two of them introduced by the commit above: - **WebSocket subscriptions never delivered.** The handovers passed `WithContext(params.OutRequest.Context())`, but `net/http` calls `w.cancelCtx()` as soon as `ServeHTTP` returns — before the hijacked check in `conn.serve` — and the gateway returns as soon as the connection is hijacked. The subscription context was cancelled before anything was sent. `subscriptionRequestContext` roots the subscription at the long-lived engine spec context while carrying the opening request's transport values across. EngineV1 passes no context: its legacy data sources drop it, so inheriting the request's cancellation could only break the subscription. - **`handleProxyOnly` rewrote the outgoing method**, turning a proxy-only upstream WebSocket handshake into a POST, which WebSocket servers reject. Handshakes now stay `GET`. - **`GraphQLProxyOnlyContextValues.upstreamResponse` was unsynchronised.** The transport writes it on the resolver's trigger goroutine while the engine reads it (~1 in 20 under `-race`). Now behind a mutex. `credential_isolation_test.go`: proxy-only, UDG and supergraph × HTTP, SSE, `graphql-ws` and `graphql-transport-ws` × EngineV2 and EngineV3. Each test drives more than one request through one engine and asserts what each upstream connection actually received, with negative controls (`strip_auth_data`, a custom auth header name, a request that sets no header after one that did), 20-request concurrent bursts, a per-request round tripper test that shared mutable transport state cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through. Both module pins move to the library branch; `coder/websocket` becomes a direct dependency. Two races are fixed upstream — v1 `Schema.Hash` writing `s.hash` on first use, and v2 `ResolveGraphQLResponse` writing `Info` on the cached plan — so the cold-start concurrent burst becomes permanent coverage instead of something a warm-up had to work around. The v1 header modifier merged Tyk's additional headers and stopped, leaving `tykVariableReplacer` injected but never read, so `$tyk_context.*` in an upstream header reached the fetch unresolved on config version 2. That matters for more than the value that arrives upstream: the library applies the modifier before it derives the key that groups equivalent subscriptions onto one upstream connection (`connectionKey` in the v1 subscription client, `UniqueRequestID` in the v2 resolver). An unresolved template is the same string for every request, so requests that should have been distinct were treated as equivalent and pooled together. Resolving there is only correct now that the library moved header modification to the execution phase. TT-14357 (`adca34d46`) had removed it because the modifier ran at plan post-process time and the result was cached into the plan, and added `variableReplaceRoundTripper` as the workaround. That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value headers via `Get`/`Set` on the way to the wire, and it expanded variables in header values that did not come from the API definition. Its one remaining job, `request_headers_rewrite`, is resolved once where the rules are built, from configuration only. Regression from `cf506a971`, caught while verifying the above: `TestGraphQL_InternalDataSource/graphql_engine_v1` fails at the branch tip and passes at the base — a config version 1 API with a `tyk://` data source answers 500. The transport assignment that commit removed was also the only thing giving EngineV1 the gateway round tripper, and only that round tripper can route `tyk://`. Since EngineV1's data sources drop the execution context, the round tripper is handed over as a mutex-guarded transport-level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 stay fully request-scoped. That fallback is the one piece of transport state shared between requests, and it is only sound because the round tripper the gateway hands over no longer carries request-specific state — the wrapper retired in `4d67f9af4` did. **The two commits must not be reverted independently**; both sides carry comments saying so. Two writers put the consumer's headers on the upstream request and neither knew about the other: `propagateAuthHeaders` adds them to the fetch input when `strip_auth_data` is off, and `setProxyOnlyHeaders` then forwards every consumer header again with `Header.Add`. With `use_immutable_headers` off nothing removed the first copy, so a header present in both arrived duplicated. It went unnoticed because the wrapper retired in `4d67f9af4` had been collapsing repeated values with `Get`/`Set` since before the second writer was added. `setProxyOnlyHeaders` now skips a value the engine already placed there, compared against a snapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both. It also hoists the `use_immutable_headers` delete out of the value loop, where it removed the value added on the previous pass and left a multi-value consumer header holding only its last value. 1. **An API reload or release now ends that API's subscriptions.** Previously they ran on `context.Background()` and outlived the reload. 2. **No header value is resolved twice** on config version 2 or 3-preview. Values that do not come from the API definition are no longer expanded, and a value that resolved into another template token no longer gets a second pass. 3. **An exact overlap between a configured `request_headers` value and a consumer value now reaches the upstream once**, not twice. Differing values still both go upstream — TT-11990 / TT-12190 semantics are unchanged. 4. **Multi-value headers survive to the upstream.** `Accept-Encoding: gzip, deflate, br` used to arrive as `gzip` alone. <!---TykTechnologies/jira-linter starts here--> <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17921" title="TT-17921" target="_blank">TT-17921</a> </summary> | | | |---------|----| | Status | Merge | | Summary | Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight | Generated at: 2026-08-20 13:03:45 </details> <!---TykTechnologies/jira-linter ends here-->
Collaborator
Author
|
/release to release-5.13.2 |
Contributor
|
|
pvormste
added a commit
that referenced
this pull request
Aug 21, 2026
Values that belong to a single request — the round tripper, the resolved upstream headers, the upstream response — were being written into, or keyed on, state that outlives the request: the shared per-API `http.Client`, the plan cache, and the upstream connection-reuse key. Under concurrency the result was not deterministic: which request's values a fetch used depended on timing. This makes that state request-scoped, and fixes the header handling that depended on it. Seven commits, each independently revertable. `internal/graphengine` goes from 22 to 76 test functions (273 including subtests), green under `-race -count=10`. Behavioural detail and reproduction steps are on the ticket. `reverseProxyPreHandlerV1/V2.PreHandle` assigned `params.RoundTripper` onto the per-API `httpClient.Transport` on every request, so concurrent requests overwrote each other's. The round tripper and headers config now travel in the request context (`SetGraphQLEngineTransportContextValue`), and `GraphQLEngineTransport.RoundTrip` builds a per-request view from them, falling back to the client's own transport. Three defects, two of them introduced by the commit above: - **WebSocket subscriptions never delivered.** The handovers passed `WithContext(params.OutRequest.Context())`, but `net/http` calls `w.cancelCtx()` as soon as `ServeHTTP` returns — before the hijacked check in `conn.serve` — and the gateway returns as soon as the connection is hijacked. The subscription context was cancelled before anything was sent. `subscriptionRequestContext` roots the subscription at the long-lived engine spec context while carrying the opening request's transport values across. EngineV1 passes no context: its legacy data sources drop it, so inheriting the request's cancellation could only break the subscription. - **`handleProxyOnly` rewrote the outgoing method**, turning a proxy-only upstream WebSocket handshake into a POST, which WebSocket servers reject. Handshakes now stay `GET`. - **`GraphQLProxyOnlyContextValues.upstreamResponse` was unsynchronised.** The transport writes it on the resolver's trigger goroutine while the engine reads it (~1 in 20 under `-race`). Now behind a mutex. `credential_isolation_test.go`: proxy-only, UDG and supergraph × HTTP, SSE, `graphql-ws` and `graphql-transport-ws` × EngineV2 and EngineV3. Each test drives more than one request through one engine and asserts what each upstream connection actually received, with negative controls (`strip_auth_data`, a custom auth header name, a request that sets no header after one that did), 20-request concurrent bursts, a per-request round tripper test that shared mutable transport state cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through. Both module pins move to the library branch; `coder/websocket` becomes a direct dependency. Two races are fixed upstream — v1 `Schema.Hash` writing `s.hash` on first use, and v2 `ResolveGraphQLResponse` writing `Info` on the cached plan — so the cold-start concurrent burst becomes permanent coverage instead of something a warm-up had to work around. The v1 header modifier merged Tyk's additional headers and stopped, leaving `tykVariableReplacer` injected but never read, so `$tyk_context.*` in an upstream header reached the fetch unresolved on config version 2. That matters for more than the value that arrives upstream: the library applies the modifier before it derives the key that groups equivalent subscriptions onto one upstream connection (`connectionKey` in the v1 subscription client, `UniqueRequestID` in the v2 resolver). An unresolved template is the same string for every request, so requests that should have been distinct were treated as equivalent and pooled together. Resolving there is only correct now that the library moved header modification to the execution phase. TT-14357 (`adca34d46`) had removed it because the modifier ran at plan post-process time and the result was cached into the plan, and added `variableReplaceRoundTripper` as the workaround. That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value headers via `Get`/`Set` on the way to the wire, and it expanded variables in header values that did not come from the API definition. Its one remaining job, `request_headers_rewrite`, is resolved once where the rules are built, from configuration only. Regression from `cf506a971`, caught while verifying the above: `TestGraphQL_InternalDataSource/graphql_engine_v1` fails at the branch tip and passes at the base — a config version 1 API with a `tyk://` data source answers 500. The transport assignment that commit removed was also the only thing giving EngineV1 the gateway round tripper, and only that round tripper can route `tyk://`. Since EngineV1's data sources drop the execution context, the round tripper is handed over as a mutex-guarded transport-level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 stay fully request-scoped. That fallback is the one piece of transport state shared between requests, and it is only sound because the round tripper the gateway hands over no longer carries request-specific state — the wrapper retired in `4d67f9af4` did. **The two commits must not be reverted independently**; both sides carry comments saying so. Two writers put the consumer's headers on the upstream request and neither knew about the other: `propagateAuthHeaders` adds them to the fetch input when `strip_auth_data` is off, and `setProxyOnlyHeaders` then forwards every consumer header again with `Header.Add`. With `use_immutable_headers` off nothing removed the first copy, so a header present in both arrived duplicated. It went unnoticed because the wrapper retired in `4d67f9af4` had been collapsing repeated values with `Get`/`Set` since before the second writer was added. `setProxyOnlyHeaders` now skips a value the engine already placed there, compared against a snapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both. It also hoists the `use_immutable_headers` delete out of the value loop, where it removed the value added on the previous pass and left a multi-value consumer header holding only its last value. 1. **An API reload or release now ends that API's subscriptions.** Previously they ran on `context.Background()` and outlived the reload. 2. **No header value is resolved twice** on config version 2 or 3-preview. Values that do not come from the API definition are no longer expanded, and a value that resolved into another template token no longer gets a second pass. 3. **An exact overlap between a configured `request_headers` value and a consumer value now reaches the upstream once**, not twice. Differing values still both go upstream — TT-11990 / TT-12190 semantics are unchanged. 4. **Multi-value headers survive to the upstream.** `Accept-Encoding: gzip, deflate, br` used to arrive as `gzip` alone. <!---TykTechnologies/jira-linter starts here--> <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17921" title="TT-17921" target="_blank">TT-17921</a> </summary> | | | |---------|----| | Status | Merge | | Summary | Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight | Generated at: 2026-08-20 13:03:45 </details> <!---TykTechnologies/jira-linter ends here-->
pvormste
added a commit
that referenced
this pull request
Aug 21, 2026
…phQL-based APIs (#8602) (#8641) Cherry-pick of `d55ab6ba241a233576c7d8fcf469f39f328809fa` from `master` to `release-5.13.2` requires manual resolution. **Conflicts detected:** 3 - go.mod Tips: - Check out this branch locally and run: `git cherry-pick -x d55ab6b` - Resolve conflicts (including submodules if any), then push back to this branch. Original commit: d55ab6b --------- Co-authored-by: Tyk Bot <bot@tyk.io> Co-authored-by: Patric Vormstein <pvormstein@googlemail.com>
pvormste
added a commit
that referenced
this pull request
Aug 21, 2026
…QL-based APIs (#8602) (#8639) Cherry-pick of `d55ab6ba241a233576c7d8fcf469f39f328809fa` from `master` to `release-5.13` requires manual resolution. **Conflicts detected:** 3 - go.mod Tips: - Check out this branch locally and run: `git cherry-pick -x d55ab6b` - Resolve conflicts (including submodules if any), then push back to this branch. Original commit: d55ab6b --------- Co-authored-by: Tyk Bot <bot@tyk.io> Co-authored-by: Patric Vormstein <pvormstein@googlemail.com> Co-authored-by: Vlad Zabolotnyi <109525963+vladzabolotnyi@users.noreply.github.com>
Collaborator
Author
|
/release to release-5.15.0 |
Contributor
|
✅ Cherry-pick successful. A PR was created: #8643 |
Collaborator
Author
|
/release to release-5.15 |
Contributor
|
✅ Cherry-pick successful. A PR was created: #8644 |
pvormste
added a commit
that referenced
this pull request
Aug 24, 2026
…phQL-based APIs (#8602) (#8643) [TT-17921] Improve header handling for GraphQL-based APIs (#8602) ## Summary Values that belong to a single request — the round tripper, the resolved upstream headers, the upstream response — were being written into, or keyed on, state that outlives the request: the shared per-API `http.Client`, the plan cache, and the upstream connection-reuse key. Under concurrency the result was not deterministic: which request's values a fetch used depended on timing. This makes that state request-scoped, and fixes the header handling that depended on it. Seven commits, each independently revertable. `internal/graphengine` goes from 22 to 76 test functions (273 including subtests), green under `-race -count=10`. Behavioural detail and reproduction steps are on the ticket. ## What changed ### scope transports to requests `reverseProxyPreHandlerV1/V2.PreHandle` assigned `params.RoundTripper` onto the per-API `httpClient.Transport` on every request, so concurrent requests overwrote each other's. The round tripper and headers config now travel in the request context (`SetGraphQLEngineTransportContextValue`), and `GraphQLEngineTransport.RoundTrip` builds a per-request view from them, falling back to the client's own transport. ### keep subscriptions alive, isolate proxy-only responses Three defects, two of them introduced by the commit above: - **WebSocket subscriptions never delivered.** The handovers passed `WithContext(params.OutRequest.Context())`, but `net/http` calls `w.cancelCtx()` as soon as `ServeHTTP` returns — before the hijacked check in `conn.serve` — and the gateway returns as soon as the connection is hijacked. The subscription context was cancelled before anything was sent. `subscriptionRequestContext` roots the subscription at the long-lived engine spec context while carrying the opening request's transport values across. EngineV1 passes no context: its legacy data sources drop it, so inheriting the request's cancellation could only break the subscription. - **`handleProxyOnly` rewrote the outgoing method**, turning a proxy-only upstream WebSocket handshake into a POST, which WebSocket servers reject. Handshakes now stay `GET`. - **`GraphQLProxyOnlyContextValues.upstreamResponse` was unsynchronised.** The transport writes it on the resolver's trigger goroutine while the engine reads it (~1 in 20 under `-race`). Now behind a mutex. ### cover the request isolation matrix `credential_isolation_test.go`: proxy-only, UDG and supergraph × HTTP, SSE, `graphql-ws` and `graphql-transport-ws` × EngineV2 and EngineV3. Each test drives more than one request through one engine and asserts what each upstream connection actually received, with negative controls (`strip_auth_data`, a custom auth header name, a request that sets no header after one that did), 20-request concurrent bursts, a per-request round tripper test that shared mutable transport state cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through. ### move to the fixed graphql-go-tools, cover the cold path Both module pins move to the library branch; `coder/websocket` becomes a direct dependency. Two races are fixed upstream — v1 `Schema.Hash` writing `s.hash` on first use, and v2 `ResolveGraphQLResponse` writing `Info` on the cached plan — so the cold-start concurrent burst becomes permanent coverage instead of something a warm-up had to work around. ### resolve dynamic upstream headers before the fetch is keyed The v1 header modifier merged Tyk's additional headers and stopped, leaving `tykVariableReplacer` injected but never read, so `$tyk_context.*` in an upstream header reached the fetch unresolved on config version 2. That matters for more than the value that arrives upstream: the library applies the modifier before it derives the key that groups equivalent subscriptions onto one upstream connection (`connectionKey` in the v1 subscription client, `UniqueRequestID` in the v2 resolver). An unresolved template is the same string for every request, so requests that should have been distinct were treated as equivalent and pooled together. Resolving there is only correct now that the library moved header modification to the execution phase. TT-14357 (`adca34d46`) had removed it because the modifier ran at plan post-process time and the result was cached into the plan, and added `variableReplaceRoundTripper` as the workaround. That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value headers via `Get`/`Set` on the way to the wire, and it expanded variables in header values that did not come from the API definition. Its one remaining job, `request_headers_rewrite`, is resolved once where the rules are built, from configuration only. ### restore the gateway round tripper for EngineV1 fetches Regression from `cf506a971`, caught while verifying the above: `TestGraphQL_InternalDataSource/graphql_engine_v1` fails at the branch tip and passes at the base — a config version 1 API with a `tyk://` data source answers 500. The transport assignment that commit removed was also the only thing giving EngineV1 the gateway round tripper, and only that round tripper can route `tyk://`. Since EngineV1's data sources drop the execution context, the round tripper is handed over as a mutex-guarded transport-level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 stay fully request-scoped. That fallback is the one piece of transport state shared between requests, and it is only sound because the round tripper the gateway hands over no longer carries request-specific state — the wrapper retired in `4d67f9af4` did. **The two commits must not be reverted independently**; both sides carry comments saying so. ### forward a consumer header to the upstream once Two writers put the consumer's headers on the upstream request and neither knew about the other: `propagateAuthHeaders` adds them to the fetch input when `strip_auth_data` is off, and `setProxyOnlyHeaders` then forwards every consumer header again with `Header.Add`. With `use_immutable_headers` off nothing removed the first copy, so a header present in both arrived duplicated. It went unnoticed because the wrapper retired in `4d67f9af4` had been collapsing repeated values with `Get`/`Set` since before the second writer was added. `setProxyOnlyHeaders` now skips a value the engine already placed there, compared against a snapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both. It also hoists the `use_immutable_headers` delete out of the value loop, where it removed the value added on the previous pass and left a multi-value consumer header holding only its last value. ## Behaviour changes worth a reviewer's sign-off 1. **An API reload or release now ends that API's subscriptions.** Previously they ran on `context.Background()` and outlived the reload. 2. **No header value is resolved twice** on config version 2 or 3-preview. Values that do not come from the API definition are no longer expanded, and a value that resolved into another template token no longer gets a second pass. 3. **An exact overlap between a configured `request_headers` value and a consumer value now reaches the upstream once**, not twice. Differing values still both go upstream — TT-11990 / TT-12190 semantics are unchanged. 4. **Multi-value headers survive to the upstream.** `Accept-Encoding: gzip, deflate, br` used to arrive as `gzip` alone. <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17921" title="TT-17921" target="_blank">TT-17921</a> </summary> | | | |---------|----| | Status | Merge | | Summary | Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight | Generated at: 2026-08-20 13:03:45 </details> <!---TykTechnologies/jira-linter ends here--> [TT-17921]: https://tyktech.atlassian.net/browse/TT-17921?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Patric Vormstein <pvormstein@googlemail.com>
pvormste
added a commit
that referenced
this pull request
Aug 24, 2026
…QL-based APIs (#8602) (#8644) [TT-17921] Improve header handling for GraphQL-based APIs (#8602) ## Summary Values that belong to a single request — the round tripper, the resolved upstream headers, the upstream response — were being written into, or keyed on, state that outlives the request: the shared per-API `http.Client`, the plan cache, and the upstream connection-reuse key. Under concurrency the result was not deterministic: which request's values a fetch used depended on timing. This makes that state request-scoped, and fixes the header handling that depended on it. Seven commits, each independently revertable. `internal/graphengine` goes from 22 to 76 test functions (273 including subtests), green under `-race -count=10`. Behavioural detail and reproduction steps are on the ticket. ## What changed ### scope transports to requests `reverseProxyPreHandlerV1/V2.PreHandle` assigned `params.RoundTripper` onto the per-API `httpClient.Transport` on every request, so concurrent requests overwrote each other's. The round tripper and headers config now travel in the request context (`SetGraphQLEngineTransportContextValue`), and `GraphQLEngineTransport.RoundTrip` builds a per-request view from them, falling back to the client's own transport. ### keep subscriptions alive, isolate proxy-only responses Three defects, two of them introduced by the commit above: - **WebSocket subscriptions never delivered.** The handovers passed `WithContext(params.OutRequest.Context())`, but `net/http` calls `w.cancelCtx()` as soon as `ServeHTTP` returns — before the hijacked check in `conn.serve` — and the gateway returns as soon as the connection is hijacked. The subscription context was cancelled before anything was sent. `subscriptionRequestContext` roots the subscription at the long-lived engine spec context while carrying the opening request's transport values across. EngineV1 passes no context: its legacy data sources drop it, so inheriting the request's cancellation could only break the subscription. - **`handleProxyOnly` rewrote the outgoing method**, turning a proxy-only upstream WebSocket handshake into a POST, which WebSocket servers reject. Handshakes now stay `GET`. - **`GraphQLProxyOnlyContextValues.upstreamResponse` was unsynchronised.** The transport writes it on the resolver's trigger goroutine while the engine reads it (~1 in 20 under `-race`). Now behind a mutex. ### cover the request isolation matrix `credential_isolation_test.go`: proxy-only, UDG and supergraph × HTTP, SSE, `graphql-ws` and `graphql-transport-ws` × EngineV2 and EngineV3. Each test drives more than one request through one engine and asserts what each upstream connection actually received, with negative controls (`strip_auth_data`, a custom auth header name, a request that sets no header after one that did), 20-request concurrent bursts, a per-request round tripper test that shared mutable transport state cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through. ### move to the fixed graphql-go-tools, cover the cold path Both module pins move to the library branch; `coder/websocket` becomes a direct dependency. Two races are fixed upstream — v1 `Schema.Hash` writing `s.hash` on first use, and v2 `ResolveGraphQLResponse` writing `Info` on the cached plan — so the cold-start concurrent burst becomes permanent coverage instead of something a warm-up had to work around. ### resolve dynamic upstream headers before the fetch is keyed The v1 header modifier merged Tyk's additional headers and stopped, leaving `tykVariableReplacer` injected but never read, so `$tyk_context.*` in an upstream header reached the fetch unresolved on config version 2. That matters for more than the value that arrives upstream: the library applies the modifier before it derives the key that groups equivalent subscriptions onto one upstream connection (`connectionKey` in the v1 subscription client, `UniqueRequestID` in the v2 resolver). An unresolved template is the same string for every request, so requests that should have been distinct were treated as equivalent and pooled together. Resolving there is only correct now that the library moved header modification to the execution phase. TT-14357 (`adca34d46`) had removed it because the modifier ran at plan post-process time and the result was cached into the plan, and added `variableReplaceRoundTripper` as the workaround. That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value headers via `Get`/`Set` on the way to the wire, and it expanded variables in header values that did not come from the API definition. Its one remaining job, `request_headers_rewrite`, is resolved once where the rules are built, from configuration only. ### restore the gateway round tripper for EngineV1 fetches Regression from `cf506a971`, caught while verifying the above: `TestGraphQL_InternalDataSource/graphql_engine_v1` fails at the branch tip and passes at the base — a config version 1 API with a `tyk://` data source answers 500. The transport assignment that commit removed was also the only thing giving EngineV1 the gateway round tripper, and only that round tripper can route `tyk://`. Since EngineV1's data sources drop the execution context, the round tripper is handed over as a mutex-guarded transport-level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 stay fully request-scoped. That fallback is the one piece of transport state shared between requests, and it is only sound because the round tripper the gateway hands over no longer carries request-specific state — the wrapper retired in `4d67f9af4` did. **The two commits must not be reverted independently**; both sides carry comments saying so. ### forward a consumer header to the upstream once Two writers put the consumer's headers on the upstream request and neither knew about the other: `propagateAuthHeaders` adds them to the fetch input when `strip_auth_data` is off, and `setProxyOnlyHeaders` then forwards every consumer header again with `Header.Add`. With `use_immutable_headers` off nothing removed the first copy, so a header present in both arrived duplicated. It went unnoticed because the wrapper retired in `4d67f9af4` had been collapsing repeated values with `Get`/`Set` since before the second writer was added. `setProxyOnlyHeaders` now skips a value the engine already placed there, compared against a snapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both. It also hoists the `use_immutable_headers` delete out of the value loop, where it removed the value added on the previous pass and left a multi-value consumer header holding only its last value. ## Behaviour changes worth a reviewer's sign-off 1. **An API reload or release now ends that API's subscriptions.** Previously they ran on `context.Background()` and outlived the reload. 2. **No header value is resolved twice** on config version 2 or 3-preview. Values that do not come from the API definition are no longer expanded, and a value that resolved into another template token no longer gets a second pass. 3. **An exact overlap between a configured `request_headers` value and a consumer value now reaches the upstream once**, not twice. Differing values still both go upstream — TT-11990 / TT-12190 semantics are unchanged. 4. **Multi-value headers survive to the upstream.** `Accept-Encoding: gzip, deflate, br` used to arrive as `gzip` alone. <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17921" title="TT-17921" target="_blank">TT-17921</a> </summary> | | | |---------|----| | Status | Merge | | Summary | Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight | Generated at: 2026-08-20 13:03:45 </details> <!---TykTechnologies/jira-linter ends here--> [TT-17921]: https://tyktech.atlassian.net/browse/TT-17921?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Patric Vormstein <pvormstein@googlemail.com>
Member
|
/release to release-5.8 |
Contributor
|
|
pvormste
added a commit
that referenced
this pull request
Sep 11, 2026
Values that belong to a single request — the round tripper, the resolved upstream headers, the upstream response — were being written into, or keyed on, state that outlives the request: the shared per-API `http.Client`, the plan cache, and the upstream connection-reuse key. Under concurrency the result was not deterministic: which request's values a fetch used depended on timing. This makes that state request-scoped, and fixes the header handling that depended on it. Seven commits, each independently revertable. `internal/graphengine` goes from 22 to 76 test functions (273 including subtests), green under `-race -count=10`. Behavioural detail and reproduction steps are on the ticket. `reverseProxyPreHandlerV1/V2.PreHandle` assigned `params.RoundTripper` onto the per-API `httpClient.Transport` on every request, so concurrent requests overwrote each other's. The round tripper and headers config now travel in the request context (`SetGraphQLEngineTransportContextValue`), and `GraphQLEngineTransport.RoundTrip` builds a per-request view from them, falling back to the client's own transport. Three defects, two of them introduced by the commit above: - **WebSocket subscriptions never delivered.** The handovers passed `WithContext(params.OutRequest.Context())`, but `net/http` calls `w.cancelCtx()` as soon as `ServeHTTP` returns — before the hijacked check in `conn.serve` — and the gateway returns as soon as the connection is hijacked. The subscription context was cancelled before anything was sent. `subscriptionRequestContext` roots the subscription at the long-lived engine spec context while carrying the opening request's transport values across. EngineV1 passes no context: its legacy data sources drop it, so inheriting the request's cancellation could only break the subscription. - **`handleProxyOnly` rewrote the outgoing method**, turning a proxy-only upstream WebSocket handshake into a POST, which WebSocket servers reject. Handshakes now stay `GET`. - **`GraphQLProxyOnlyContextValues.upstreamResponse` was unsynchronised.** The transport writes it on the resolver's trigger goroutine while the engine reads it (~1 in 20 under `-race`). Now behind a mutex. `credential_isolation_test.go`: proxy-only, UDG and supergraph × HTTP, SSE, `graphql-ws` and `graphql-transport-ws` × EngineV2 and EngineV3. Each test drives more than one request through one engine and asserts what each upstream connection actually received, with negative controls (`strip_auth_data`, a custom auth header name, a request that sets no header after one that did), 20-request concurrent bursts, a per-request round tripper test that shared mutable transport state cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through. Both module pins move to the library branch; `coder/websocket` becomes a direct dependency. Two races are fixed upstream — v1 `Schema.Hash` writing `s.hash` on first use, and v2 `ResolveGraphQLResponse` writing `Info` on the cached plan — so the cold-start concurrent burst becomes permanent coverage instead of something a warm-up had to work around. The v1 header modifier merged Tyk's additional headers and stopped, leaving `tykVariableReplacer` injected but never read, so `$tyk_context.*` in an upstream header reached the fetch unresolved on config version 2. That matters for more than the value that arrives upstream: the library applies the modifier before it derives the key that groups equivalent subscriptions onto one upstream connection (`connectionKey` in the v1 subscription client, `UniqueRequestID` in the v2 resolver). An unresolved template is the same string for every request, so requests that should have been distinct were treated as equivalent and pooled together. Resolving there is only correct now that the library moved header modification to the execution phase. TT-14357 (`adca34d46`) had removed it because the modifier ran at plan post-process time and the result was cached into the plan, and added `variableReplaceRoundTripper` as the workaround. That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value headers via `Get`/`Set` on the way to the wire, and it expanded variables in header values that did not come from the API definition. Its one remaining job, `request_headers_rewrite`, is resolved once where the rules are built, from configuration only. Regression from `cf506a971`, caught while verifying the above: `TestGraphQL_InternalDataSource/graphql_engine_v1` fails at the branch tip and passes at the base — a config version 1 API with a `tyk://` data source answers 500. The transport assignment that commit removed was also the only thing giving EngineV1 the gateway round tripper, and only that round tripper can route `tyk://`. Since EngineV1's data sources drop the execution context, the round tripper is handed over as a mutex-guarded transport-level fallback, used only when a request carries none of its own. EngineV2 and EngineV3 stay fully request-scoped. That fallback is the one piece of transport state shared between requests, and it is only sound because the round tripper the gateway hands over no longer carries request-specific state — the wrapper retired in `4d67f9af4` did. **The two commits must not be reverted independently**; both sides carry comments saying so. Two writers put the consumer's headers on the upstream request and neither knew about the other: `propagateAuthHeaders` adds them to the fetch input when `strip_auth_data` is off, and `setProxyOnlyHeaders` then forwards every consumer header again with `Header.Add`. With `use_immutable_headers` off nothing removed the first copy, so a header present in both arrived duplicated. It went unnoticed because the wrapper retired in `4d67f9af4` had been collapsing repeated values with `Get`/`Set` since before the second writer was added. `setProxyOnlyHeaders` now skips a value the engine already placed there, compared against a snapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both. It also hoists the `use_immutable_headers` delete out of the value loop, where it removed the value added on the previous pass and left a multi-value consumer header holding only its last value. 1. **An API reload or release now ends that API's subscriptions.** Previously they ran on `context.Background()` and outlived the reload. 2. **No header value is resolved twice** on config version 2 or 3-preview. Values that do not come from the API definition are no longer expanded, and a value that resolved into another template token no longer gets a second pass. 3. **An exact overlap between a configured `request_headers` value and a consumer value now reaches the upstream once**, not twice. Differing values still both go upstream — TT-11990 / TT-12190 semantics are unchanged. 4. **Multi-value headers survive to the upstream.** `Accept-Encoding: gzip, deflate, br` used to arrive as `gzip` alone. <!---TykTechnologies/jira-linter starts here--> <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17921" title="TT-17921" target="_blank">TT-17921</a> </summary> | | | |---------|----| | Status | Merge | | Summary | Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight | Generated at: 2026-08-20 13:03:45 </details> <!---TykTechnologies/jira-linter ends here-->
ilijabojanovic
pushed a commit
that referenced
this pull request
Sep 11, 2026
…L-based APIs (#8602) (#8689) Cherry-pick of `d55ab6ba241a233576c7d8fcf469f39f328809fa` from `master` to `release-5.8` requires manual resolution. **Conflicts detected:** 6 - go.mod - go.sum Tips: - Check out this branch locally and run: `git cherry-pick -x d55ab6b` - Resolve conflicts (including submodules if any), then push back to this branch. Original commit: d55ab6b --------- Co-authored-by: Tyk Bot <bot@tyk.io> Co-authored-by: Patric Vormstein <pvormstein@googlemail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Values that belong to a single request — the round tripper, the resolved upstream headers, the
upstream response — were being written into, or keyed on, state that outlives the request: the
shared per-API
http.Client, the plan cache, and the upstream connection-reuse key. Underconcurrency the result was not deterministic: which request's values a fetch used depended on
timing.
This makes that state request-scoped, and fixes the header handling that depended on it. Seven
commits, each independently revertable.
internal/graphenginegoes from 22 to 76 test functions (273 including subtests), green under-race -count=10.Behavioural detail and reproduction steps are on the ticket.
What changed
scope transports to requests
reverseProxyPreHandlerV1/V2.PreHandleassignedparams.RoundTripperonto the per-APIhttpClient.Transporton every request, so concurrent requests overwrote each other's. The roundtripper and headers config now travel in the request context
(
SetGraphQLEngineTransportContextValue), andGraphQLEngineTransport.RoundTripbuilds aper-request view from them, falling back to the client's own transport.
keep subscriptions alive, isolate proxy-only responses
Three defects, two of them introduced by the commit above:
WithContext(params.OutRequest.Context()), butnet/httpcallsw.cancelCtx()as soon asServeHTTPreturns — before the hijacked check inconn.serve— and the gateway returns as soonas the connection is hijacked. The subscription context was cancelled before anything was sent.
subscriptionRequestContextroots the subscription at the long-lived engine spec context whilecarrying the opening request's transport values across. EngineV1 passes no context: its legacy
data sources drop it, so inheriting the request's cancellation could only break the subscription.
handleProxyOnlyrewrote the outgoing method, turning a proxy-only upstream WebSockethandshake into a POST, which WebSocket servers reject. Handshakes now stay
GET.GraphQLProxyOnlyContextValues.upstreamResponsewas unsynchronised. The transport writes iton the resolver's trigger goroutine while the engine reads it (~1 in 20 under
-race). Nowbehind a mutex.
cover the request isolation matrix
credential_isolation_test.go: proxy-only, UDG and supergraph × HTTP, SSE,graphql-wsandgraphql-transport-ws× EngineV2 and EngineV3. Each test drives more than one request through oneengine and asserts what each upstream connection actually received, with negative controls
(
strip_auth_data, a custom auth header name, a request that sets no header after one that did),20-request concurrent bursts, a per-request round tripper test that shared mutable transport state
cannot pass, and the client WebSocket-upgrade path that the regression above had slipped through.
move to the fixed graphql-go-tools, cover the cold path
Both module pins move to the library branch;
coder/websocketbecomes a direct dependency. Tworaces are fixed upstream — v1
Schema.Hashwritings.hashon first use, and v2ResolveGraphQLResponsewritingInfoon the cached plan — so the cold-start concurrent burstbecomes permanent coverage instead of something a warm-up had to work around.
resolve dynamic upstream headers before the fetch is keyed
The v1 header modifier merged Tyk's additional headers and stopped, leaving
tykVariableReplacerinjected but never read, so
$tyk_context.*in an upstream header reached the fetch unresolved onconfig version 2. That matters for more than the value that arrives upstream: the library applies
the modifier before it derives the key that groups equivalent subscriptions onto one upstream
connection (
connectionKeyin the v1 subscription client,UniqueRequestIDin the v2 resolver).An unresolved template is the same string for every request, so requests that should have been
distinct were treated as equivalent and pooled together.
Resolving there is only correct now that the library moved header modification to the execution
phase. TT-14357 (
adca34d46) had removed it because the modifier ran at plan post-process time andthe result was cached into the plan, and added
variableReplaceRoundTripperas the workaround.That wrapper is retired here: it ran after the grouping key was derived, it collapsed multi-value
headers via
Get/Seton the way to the wire, and it expanded variables in header values that didnot come from the API definition. Its one remaining job,
request_headers_rewrite, is resolvedonce where the rules are built, from configuration only.
restore the gateway round tripper for EngineV1 fetches
Regression from
cf506a971, caught while verifying the above:TestGraphQL_InternalDataSource/graphql_engine_v1fails at the branch tip and passes at the base —a config version 1 API with a
tyk://data source answers 500. The transport assignment thatcommit removed was also the only thing giving EngineV1 the gateway round tripper, and only that
round tripper can route
tyk://. Since EngineV1's data sources drop the execution context, theround tripper is handed over as a mutex-guarded transport-level fallback, used only when a request
carries none of its own. EngineV2 and EngineV3 stay fully request-scoped.
That fallback is the one piece of transport state shared between requests, and it is only sound
because the round tripper the gateway hands over no longer carries request-specific state — the
wrapper retired in
4d67f9af4did. The two commits must not be reverted independently; bothsides carry comments saying so.
forward a consumer header to the upstream once
Two writers put the consumer's headers on the upstream request and neither knew about the other:
propagateAuthHeadersadds them to the fetch input whenstrip_auth_datais off, andsetProxyOnlyHeadersthen forwards every consumer header again withHeader.Add. Withuse_immutable_headersoff nothing removed the first copy, so a header present in both arrivedduplicated. It went unnoticed because the wrapper retired in
4d67f9af4had been collapsingrepeated values with
Get/Setsince before the second writer was added.setProxyOnlyHeadersnow skips a value the engine already placed there, compared against asnapshot taken before forwarding so a consumer that genuinely sent the same value twice keeps both.
It also hoists the
use_immutable_headersdelete out of the value loop, where it removed the valueadded on the previous pass and left a multi-value consumer header holding only its last value.
Behaviour changes worth a reviewer's sign-off
context.Background()and outlived the reload.from the API definition are no longer expanded, and a value that resolved into another template
token no longer gets a second pass.
request_headersvalue and a consumer value now reachesthe upstream once, not twice. Differing values still both go upstream — TT-11990 / TT-12190
semantics are unchanged.
Accept-Encoding: gzip, deflate, brused toarrive as
gzipalone.Ticket Details
TT-17921
Generated at: 2026-08-21 09:06:43