Skip to content

Commit 510d3fc

Browse files
probelabs[bot]Tyk Botpvormste
authored
Merging to release-5.13.2: [TT-17921] Improve header handling for GraphQL-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>
1 parent 7eaca1c commit 510d3fc

22 files changed

Lines changed: 3068 additions & 75 deletions

gateway/reverse_proxy.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,32 +1085,28 @@ func isCORSPreflight(r *http.Request) bool {
10851085
return r.Method == http.MethodOptions
10861086
}
10871087

1088-
type variableReplaceRoundTripper struct {
1089-
next http.RoundTripper
1090-
outReq *http.Request
1091-
gw *Gateway
1092-
}
1093-
1094-
func (d *variableReplaceRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
1095-
for key := range req.Header {
1096-
val := d.gw.ReplaceTykVariables(d.outReq, req.Header.Get(key), false)
1097-
req.Header.Set(key, val)
1098-
}
1099-
1100-
return d.next.RoundTrip(req)
1101-
}
1102-
11031088
func (p *ReverseProxy) handleGraphQL(roundTripper *TykRoundTripper, outreq *http.Request, w http.ResponseWriter) (res *http.Response, hijacked bool, err error) {
11041089
isWebSocketUpgrade := ctxGetGraphQLIsWebSocketUpgrade(outreq)
11051090
needsEngine := needsGraphQLExecutionEngine(p.TykAPISpec)
11061091

11071092
requestHeadersRewrite := make(map[string]apidef.RequestHeadersRewriteConfig)
11081093
for key, value := range p.TykAPISpec.GraphQL.Proxy.RequestHeadersRewrite {
1094+
// Resolved here, once, because these are the only header values the graph engine
1095+
// adds that no header modifier sees: the transport applies them itself, after the
1096+
// engine has already finalised the fetch headers.
1097+
//
1098+
// Nothing else on this path resolves variables any more. Upstream headers of the
1099+
// engine, global and per data source alike, are resolved by the header modifier
1100+
// while the fetch input is built, which is what the subscription connection key is
1101+
// computed from (TT-17921). A round tripper that walked the outgoing headers
1102+
// instead used to sit here, and it ran too late for that key, collapsed multi
1103+
// value headers and expanded variables inside header values the caller had sent.
1104+
value.Value = p.Gw.ReplaceTykVariables(outreq, value.Value, false)
11091105
// Use the canonical format of the MIME header key.
11101106
requestHeadersRewrite[textproto.CanonicalMIMEHeaderKey(key)] = value
11111107
}
11121108
res, hijacked, err = p.TykAPISpec.GraphEngine.HandleReverseProxy(graphengine.ReverseProxyParams{
1113-
RoundTripper: &variableReplaceRoundTripper{next: roundTripper, outReq: outreq, gw: p.Gw},
1109+
RoundTripper: roundTripper,
11141110
ResponseWriter: w,
11151111
OutRequest: outreq,
11161112
WebSocketUpgrader: &p.wsUpgrader,

gateway/reverse_proxy_test.go

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,13 @@ func TestGraphQL_UDGHeaders(t *testing.T) {
11151115
strings.Contains(string(b), `{"name":"Context","value":"request-context"}`) &&
11161116
strings.Contains(string(b), `{"name":"Global-Static","value":"foobar"}`) &&
11171117
strings.Contains(string(b), `{"name":"Global-Context","value":"follow-up-request-global-context"}`) &&
1118-
strings.Contains(string(b), `{"name":"Does-Exist-Already","value":"ds-does-exist-already"}`)
1118+
strings.Contains(string(b), `{"name":"Does-Exist-Already","value":"ds-does-exist-already"}`) &&
1119+
// A header with more than one value has to keep all of them. The round
1120+
// tripper that used to resolve variables on the way to the upstream read
1121+
// with Get and wrote with Set, so it collapsed this to gzip alone.
1122+
strings.Contains(string(b), `{"name":"Accept-Encoding","value":"gzip"}`) &&
1123+
strings.Contains(string(b), `{"name":"Accept-Encoding","value":"deflate"}`) &&
1124+
strings.Contains(string(b), `{"name":"Accept-Encoding","value":"br"}`)
11191125
},
11201126
},
11211127
}...)
@@ -1194,6 +1200,107 @@ func TestGraphQL_ProxyOnlyHeaders(t *testing.T) {
11941200
})
11951201
assert.NoError(t, err)
11961202
})
1203+
1204+
t.Run("test context variable request headers rewrite", func(t *testing.T) {
1205+
// request_headers_rewrite is applied by the engine transport, after the header
1206+
// modifier has already finalised the fetch headers, so it is resolved where the
1207+
// rules are built instead. See handleGraphQL.
1208+
spec := defaultSpec
1209+
spec.GraphQL.Proxy.RequestHeadersRewrite = map[string]apidef.RequestHeadersRewriteConfig{
1210+
"X-Rewritten": {Value: "$tyk_context.headers_Test_Header"},
1211+
}
1212+
spec.EnableContextVars = true
1213+
g.Gw.LoadAPI(spec)
1214+
g.AddDynamicHandler("/dynamic", func(writer http.ResponseWriter, r *http.Request) {
1215+
if !headerCheck("X-Rewritten", "test-value", r.Header) {
1216+
t.Errorf("rewritten header not resolved, got %q", r.Header.Get("X-Rewritten"))
1217+
}
1218+
})
1219+
_, err := g.Run(t, test.TestCase{
1220+
Path: "/",
1221+
Headers: map[string]string{
1222+
"Test-Header": "test-value",
1223+
},
1224+
Method: http.MethodPost,
1225+
Data: graphql.Request{
1226+
Query: gqlContinentQuery,
1227+
},
1228+
})
1229+
assert.NoError(t, err)
1230+
})
1231+
1232+
t.Run("the consumer's credential reaches the upstream once", func(t *testing.T) {
1233+
// Two writers put it there and neither knows about the other: with strip_auth_data
1234+
// off the engine adds the consumer's auth header to the fetch input through
1235+
// propagateAuthHeaders, and setProxyOnlyHeaders then forwards the consumer's
1236+
// headers again. The upstream used to receive the credential twice.
1237+
spec := defaultSpec
1238+
spec.GraphQL.Proxy.RequestHeadersRewrite = nil
1239+
spec.UseKeylessAccess = false
1240+
spec.UseStandardAuth = true
1241+
spec.StripAuthData = false
1242+
spec.AuthConfigs = map[string]apidef.AuthConfig{
1243+
apidef.AuthTokenType: {AuthHeaderName: "X-API-KEY"},
1244+
}
1245+
g.Gw.LoadAPI(spec)
1246+
1247+
_, authKey := g.CreateSession(func(s *user.SessionState) {
1248+
s.AccessRights = map[string]user.AccessDefinition{
1249+
spec.APIID: {APIName: spec.Name, APIID: spec.APIID, Versions: []string{"Default"}},
1250+
}
1251+
s.OrgID = spec.OrgID
1252+
})
1253+
1254+
g.AddDynamicHandler("/dynamic", func(writer http.ResponseWriter, r *http.Request) {
1255+
values := r.Header.Values("X-Api-Key")
1256+
if len(values) != 1 {
1257+
t.Errorf("upstream received X-Api-Key %d times: %v", len(values), values)
1258+
return
1259+
}
1260+
if values[0] != authKey {
1261+
t.Errorf("upstream received the wrong credential: %q", values[0])
1262+
}
1263+
})
1264+
_, err := g.Run(t, test.TestCase{
1265+
Path: "/",
1266+
Headers: map[string]string{
1267+
"X-API-KEY": authKey,
1268+
},
1269+
Method: http.MethodPost,
1270+
Data: graphql.Request{
1271+
Query: gqlContinentQuery,
1272+
},
1273+
})
1274+
assert.NoError(t, err)
1275+
})
1276+
1277+
t.Run("a variable inside a header the caller sent is not expanded", func(t *testing.T) {
1278+
// Only values that come from the API definition are resolved. A round tripper that
1279+
// walked every outgoing header used to sit on this path and expanded whatever the
1280+
// caller had put in one, which let a caller read the context of their own session
1281+
// back out of the upstream request. See handleGraphQL.
1282+
spec := defaultSpec
1283+
spec.GraphQL.Proxy.RequestHeadersRewrite = nil
1284+
spec.EnableContextVars = true
1285+
g.Gw.LoadAPI(spec)
1286+
g.AddDynamicHandler("/dynamic", func(writer http.ResponseWriter, r *http.Request) {
1287+
if !headerCheck("X-Injection-Probe", "$tyk_context.headers_Test_Header", r.Header) {
1288+
t.Errorf("caller supplied variable was expanded, got %q", r.Header.Get("X-Injection-Probe"))
1289+
}
1290+
})
1291+
_, err := g.Run(t, test.TestCase{
1292+
Path: "/",
1293+
Headers: map[string]string{
1294+
"Test-Header": "test-value",
1295+
"X-Injection-Probe": "$tyk_context.headers_Test_Header",
1296+
},
1297+
Method: http.MethodPost,
1298+
Data: graphql.Request{
1299+
Query: gqlContinentQuery,
1300+
},
1301+
})
1302+
assert.NoError(t, err)
1303+
})
11971304
}
11981305

11991306
func TestGraphQL_ProxyOnlyPassHeadersWithOTel(t *testing.T) {

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ require (
2525
github.com/TykTechnologies/goautosocket v0.0.0-20190430121222-97bfa5e7e481
2626
github.com/TykTechnologies/gorpc v0.0.0-20250214161245-e9f3f088e8c6
2727
github.com/TykTechnologies/goverify v0.0.0-20260203113354-7a104729566e
28-
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260624141309-dae0d8d8d038
28+
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260818151404-c416c52a2dd3
2929
github.com/TykTechnologies/graphql-translator v0.0.0-20250602105400-41c2e7514a36
3030
github.com/TykTechnologies/murmur3 v0.0.0-20230310161213-aad17efd5632
3131
github.com/TykTechnologies/openid2go v0.1.2
@@ -96,10 +96,11 @@ require (
9696
github.com/Azure/go-amqp v1.4.0
9797
github.com/IBM/sarama v1.46.3
9898
github.com/Jeffail/gabs/v2 v2.7.0
99-
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20250926102005-c54e73aae17d
99+
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20260818151404-c416c52a2dd3
100100
github.com/TykTechnologies/opentelemetry v0.0.26
101101
github.com/TykTechnologies/structviewer v1.2.0
102102
github.com/alecthomas/kingpin/v2 v2.4.0
103+
github.com/coder/websocket v1.8.13
103104
github.com/eclipse/paho.mqtt.golang v1.5.1
104105
github.com/getkin/kin-openapi v0.133.0
105106
github.com/go-playground/validator/v10 v10.30.1
@@ -263,7 +264,6 @@ require (
263264
github.com/clbanning/mxj/v2 v2.7.0 // indirect
264265
github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 // indirect
265266
github.com/cockroachdb/apd/v3 v3.2.1 // indirect
266-
github.com/coder/websocket v1.8.13 // indirect
267267
github.com/colinmarc/hdfs v1.1.3 // indirect
268268
github.com/containerd/continuity v0.4.2 // indirect
269269
github.com/containerd/errdefs v1.0.0 // indirect

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ github.com/TykTechnologies/gorpc v0.0.0-20250214161245-e9f3f088e8c6 h1:wwt23wdyi
171171
github.com/TykTechnologies/gorpc v0.0.0-20250214161245-e9f3f088e8c6/go.mod h1:v6v7Mlj08+EmEcXOfpuTxGt2qYU9yhqqtv4QF9Wf50E=
172172
github.com/TykTechnologies/goverify v0.0.0-20260203113354-7a104729566e h1:wbd35YYCywZbgc4Fk/G2pQHnvwYzjedrw8pNzUsVowg=
173173
github.com/TykTechnologies/goverify v0.0.0-20260203113354-7a104729566e/go.mod h1:WtiSLIlItUVsHyHQB1NG+de/L4/PTtmZWc9CnzTRBLs=
174-
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260624141309-dae0d8d8d038 h1:akyeFvwB5ZnVq9jbGPx1pP33ETxp0ziDlt5r40HRCDY=
175-
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260624141309-dae0d8d8d038/go.mod h1:bvVafmGebtdjIFG2bXkLd+O1jOjjU/3To+mQHcLr4KI=
176-
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20250926102005-c54e73aae17d h1:bK9T78hExbTuDK4UaBuGi9aL28hK68Uw3OyNZswdPcA=
177-
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20250926102005-c54e73aae17d/go.mod h1:XM1owY0ZCJ1Rai64Q1UKXZYNDkWikZDojgefZw8raPk=
174+
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260818151404-c416c52a2dd3 h1:muQy5S8Y0bZk3wNLQnHjDJzUBB5I3f5FzftIkpCIP5s=
175+
github.com/TykTechnologies/graphql-go-tools v1.6.2-0.20260818151404-c416c52a2dd3/go.mod h1:bvVafmGebtdjIFG2bXkLd+O1jOjjU/3To+mQHcLr4KI=
176+
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20260818151404-c416c52a2dd3 h1:R7ZoRYvA/rEJj648PWCRK2q6wHtEO/oDp22kRCDtdF0=
177+
github.com/TykTechnologies/graphql-go-tools/v2 v2.0.0-20260818151404-c416c52a2dd3/go.mod h1:XM1owY0ZCJ1Rai64Q1UKXZYNDkWikZDojgefZw8raPk=
178178
github.com/TykTechnologies/graphql-translator v0.0.0-20250602105400-41c2e7514a36 h1:7nNsyocI/RKBqo73RR9G/SiFMZ8w2sN+HMsQXYp9wPI=
179179
github.com/TykTechnologies/graphql-translator v0.0.0-20250602105400-41c2e7514a36/go.mod h1:qiglVaUPPOWET4bQsBmsAaLiCml20a01REQSi7TSUa0=
180180
github.com/TykTechnologies/kin-openapi v0.512.1-0.20260817104659-7198d52254ff h1:lpF67zYIQzUMacE5UraBYrzaajVEkHO3h/TzCNWSCwY=

internal/graphengine/context.go

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package graphengine
33
import (
44
"context"
55
"net/http"
6+
"sync"
67

78
"github.com/TykTechnologies/tyk/apidef"
89
)
@@ -16,12 +17,80 @@ const (
1617

1718
type contextKey struct{}
1819

20+
type transportContextKey struct{}
21+
1922
var graphqlProxyContextInfo = contextKey{}
23+
var graphqlTransportContextInfo = transportContextKey{}
24+
25+
type graphQLEngineTransportContextValues struct {
26+
roundTripper http.RoundTripper
27+
headersConfig ReverseProxyHeadersConfig
28+
}
29+
30+
func SetGraphQLEngineTransportContextValue(ctx context.Context, roundTripper http.RoundTripper, headersConfig ReverseProxyHeadersConfig) context.Context {
31+
value := &graphQLEngineTransportContextValues{
32+
roundTripper: roundTripper,
33+
headersConfig: headersConfig,
34+
}
35+
return context.WithValue(ctx, graphqlTransportContextInfo, value)
36+
}
37+
38+
func copyGraphQLEngineTransportContextValue(target, source context.Context) context.Context {
39+
value, ok := source.Value(graphqlTransportContextInfo).(*graphQLEngineTransportContextValues)
40+
if !ok || value == nil {
41+
return target
42+
}
43+
return context.WithValue(target, graphqlTransportContextInfo, value)
44+
}
45+
46+
func getGraphQLEngineTransportContextValue(ctx context.Context) *graphQLEngineTransportContextValues {
47+
value, ok := ctx.Value(graphqlTransportContextInfo).(*graphQLEngineTransportContextValues)
48+
if !ok {
49+
return nil
50+
}
51+
return value
52+
}
53+
54+
// subscriptionRequestContext returns the context that a websocket subscription handler
55+
// runs on. It carries the transport values of the request that opened the connection, so
56+
// upstream fetches keep using that caller's round tripper, but it is rooted at the long
57+
// lived engine context so that releasing the API ends the subscription.
58+
//
59+
// The request context cannot be used as the parent: net/http cancels it as soon as
60+
// ServeHTTP returns, and the gateway returns as soon as the connection is hijacked, so a
61+
// subscription rooted there is cancelled before it delivers anything.
62+
func subscriptionRequestContext(engineCtx context.Context, outreq *http.Request) context.Context {
63+
if engineCtx == nil {
64+
engineCtx = context.Background()
65+
}
66+
if outreq == nil {
67+
return engineCtx
68+
}
69+
return copyGraphQLEngineTransportContextValue(engineCtx, outreq.Context())
70+
}
2071

2172
type GraphQLProxyOnlyContextValues struct {
2273
forwardedRequest *http.Request
23-
upstreamResponse *http.Response
2474
ignoreForwardedHeaders map[string]bool
75+
76+
// upstreamResponse is written by the transport and read by the engine. Those can be
77+
// different goroutines: a subscription fetch runs on the resolver's trigger goroutine
78+
// and can outlive the handover that reads the response, so both sides go through the
79+
// accessors below.
80+
mu sync.Mutex
81+
upstreamResponse *http.Response
82+
}
83+
84+
func (g *GraphQLProxyOnlyContextValues) setUpstreamResponse(response *http.Response) {
85+
g.mu.Lock()
86+
defer g.mu.Unlock()
87+
g.upstreamResponse = response
88+
}
89+
90+
func (g *GraphQLProxyOnlyContextValues) getUpstreamResponse() *http.Response {
91+
g.mu.Lock()
92+
defer g.mu.Unlock()
93+
return g.upstreamResponse
2594
}
2695

2796
func SetProxyOnlyContextValue(ctx context.Context, req *http.Request) context.Context {

0 commit comments

Comments
 (0)