From 319a166db4a2ccf5f849f7ff9c2c124a08449c00 Mon Sep 17 00:00:00 2001 From: Alexander Babenko Date: Mon, 6 Apr 2026 21:59:42 +0700 Subject: [PATCH 1/5] fix interpolation behavior in various parts of the GraphQL configuration Signed-off-by: Alexander Babenko --- transport/http/client/graphql/graphql.go | 17 +++++++++-- transport/http/client/graphql/graphql_test.go | 30 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/transport/http/client/graphql/graphql.go b/transport/http/client/graphql/graphql.go index 89bf8ac1d..ae8ef0dc3 100644 --- a/transport/http/client/graphql/graphql.go +++ b/transport/http/client/graphql/graphql.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "os" + "regexp" "strings" "github.com/luraproject/lura/v2/config" @@ -19,6 +20,9 @@ import ( "golang.org/x/text/language" ) +// urlKeysPattern matches URL path parameters in the form {paramName}. +var urlKeysPattern = regexp.MustCompile(`\{([\w\-.:/]+)\}`) + // Namespace is the key for the backend's extra config const Namespace = "github.com/devopsfaith/krakend/transport/http/client/graphql" @@ -100,8 +104,8 @@ func New(opt Options) *Extractor { if !ok { continue } - if val[0] == '{' && val[len(val)-1] == '}' { - replacements = append(replacements, [2]string{k, title.String(val[1:2]) + val[2:len(val)-1]}) + if urlKeysPattern.MatchString(val) { + replacements = append(replacements, [2]string{k, val}) } } @@ -129,7 +133,14 @@ func New(opt Options) *Extractor { val.Variables[k] = v } for _, vs := range replacements { - val.Variables[vs[0]] = params[vs[1]] + val.Variables[vs[0]] = urlKeysPattern.ReplaceAllStringFunc(vs[1], func(match string) string { + param := match[1 : len(match)-1] + key := title.String(param[:1]) + param[1:] + if v, ok := params[key]; ok { + return v + } + return match + }) } return &val, nil } diff --git a/transport/http/client/graphql/graphql_test.go b/transport/http/client/graphql/graphql_test.go index a4e6f3859..a43a6bca4 100644 --- a/transport/http/client/graphql/graphql_test.go +++ b/transport/http/client/graphql/graphql_test.go @@ -179,6 +179,36 @@ func ExampleExtractor_fromFile() { } +func ExampleExtractor_multipleParamsInVariable() { + cfg, err := GetOptions(config.ExtraConfig{ + Namespace: map[string]interface{}{ + "type": OperationQuery, + "query": "{\n search(query: $query) {\n nodes { id }\n }\n}\n", + "variables": map[string]interface{}{ + "query": "repo:{owner}/{repo} category:Announcements", + }, + }, + }) + if err != nil { + fmt.Println(err) + return + } + extractor := New(*cfg) + + body, err := extractor.BodyFromParams(map[string]string{ + "Owner": "krakend", + "Repo": "lura", + }) + if err != nil { + fmt.Println(err) + return + } + fmt.Println(string(body)) + + // output: + // {"query":"{\n search(query: $query) {\n nodes { id }\n }\n}\n","variables":{"query":"repo:krakend/lura category:Announcements"}} +} + func ExampleExtractor_noReplacement() { cfg, err := GetOptions(config.ExtraConfig{ Namespace: map[string]interface{}{ From d45fe6c5856786b3590963de2913a1b8b1d885f4 Mon Sep 17 00:00:00 2001 From: Alexander Babenko Date: Thu, 9 Apr 2026 16:18:38 +0700 Subject: [PATCH 2/5] add benchmark for graphql client body params BenchmarkBodyFromParams_legacy 792 ns/op 800 B/op 12 allocs/op BenchmarkBodyFromParams_fixed 1798 ns/op 1819 B/op 25 allocs/op 2.3x slower and 2x allocations Signed-off-by: Alexander Babenko --- .../client/graphql/graphql_benchmark_test.go | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 transport/http/client/graphql/graphql_benchmark_test.go diff --git a/transport/http/client/graphql/graphql_benchmark_test.go b/transport/http/client/graphql/graphql_benchmark_test.go new file mode 100644 index 000000000..bd4b37267 --- /dev/null +++ b/transport/http/client/graphql/graphql_benchmark_test.go @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: Apache-2.0 + +package graphql + +import ( + "encoding/json" + "testing" + + "github.com/luraproject/lura/v2/config" + "golang.org/x/text/cases" + "golang.org/x/text/language" +) + +// legacyNew reproduces the original New() behavior before the fix, for comparison. +func legacyNew(opt Options) *Extractor { + var replacements [][2]string + + title := cases.Title(language.Und) + for k, v := range opt.Variables { + val, ok := v.(string) + if !ok { + continue + } + if len(val) > 0 && val[0] == '{' && val[len(val)-1] == '}' { + replacements = append(replacements, [2]string{k, title.String(val[1:2]) + val[2:len(val)-1]}) + } + } + + paramExtractor := func(params map[string]string) (*GraphQLRequest, error) { + val := GraphQLRequest{ + Query: opt.Query, + OperationName: opt.OperationName, + Variables: map[string]interface{}{}, + } + for k, v := range opt.Variables { + val.Variables[k] = v + } + for _, vs := range replacements { + val.Variables[vs[0]] = params[vs[1]] + } + return &val, nil + } + + return &Extractor{ + cfg: opt, + paramExtractor: paramExtractor, + newBody: func(params map[string]string) ([]byte, error) { + val, err := paramExtractor(params) + if err != nil { + return []byte{}, err + } + return json.Marshal(val) + }, + } +} + +var benchCfg = config.ExtraConfig{ + Namespace: map[string]interface{}{ + "type": OperationQuery, + "query": "{ search(query: $query) { nodes { id } } }", + "variables": map[string]interface{}{ + "single": "{owner}", + "compound": "repo:{owner}/{repo} category:Announcements", + "static": "no-params-here", + }, + }, +} + +var benchParams = map[string]string{ + "Owner": "krakend", + "Repo": "lura", +} + +func BenchmarkBodyFromParams_legacy(b *testing.B) { + cfg, _ := GetOptions(benchCfg) + extractor := legacyNew(*cfg) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + extractor.BodyFromParams(benchParams) + } +} + +func BenchmarkBodyFromParams_fixed(b *testing.B) { + cfg, _ := GetOptions(benchCfg) + extractor := New(*cfg) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + extractor.BodyFromParams(benchParams) + } +} From 0a08785b90ad5d63c21587765b6033e4ad8cd840 Mon Sep 17 00:00:00 2001 From: Alexander Babenko Date: Thu, 9 Apr 2026 16:24:46 +0700 Subject: [PATCH 3/5] update graphql_benchmark_test.go with proxy/request_benchmark_test.go as a reference BenchmarkInterpolation_legacy ~102 ns/op 16 B/op 1 allocs/op BenchmarkInterpolation_fixed ~437 ns/op 136 B/op 5 allocs/op (with slow regexp replacement) Signed-off-by: Alexander Babenko --- .../client/graphql/graphql_benchmark_test.go | 110 +++++++----------- 1 file changed, 45 insertions(+), 65 deletions(-) diff --git a/transport/http/client/graphql/graphql_benchmark_test.go b/transport/http/client/graphql/graphql_benchmark_test.go index bd4b37267..dd3459b5a 100644 --- a/transport/http/client/graphql/graphql_benchmark_test.go +++ b/transport/http/client/graphql/graphql_benchmark_test.go @@ -3,90 +3,70 @@ package graphql import ( - "encoding/json" + "strings" "testing" - - "github.com/luraproject/lura/v2/config" - "golang.org/x/text/cases" - "golang.org/x/text/language" ) -// legacyNew reproduces the original New() behavior before the fix, for comparison. -func legacyNew(opt Options) *Extractor { - var replacements [][2]string +// legacyInterpolate reproduces the original per-request substitution: direct map lookup. +func legacyInterpolate(replacements [][2]string, variables map[string]interface{}, params map[string]string) { + for _, vs := range replacements { + variables[vs[0]] = params[vs[1]] + } +} - title := cases.Title(language.Und) - for k, v := range opt.Variables { - val, ok := v.(string) - if !ok { - continue - } - if len(val) > 0 && val[0] == '{' && val[len(val)-1] == '}' { - replacements = append(replacements, [2]string{k, title.String(val[1:2]) + val[2:len(val)-1]}) +// fixedInterpolate is the new per-request substitution: strings.ReplaceAll loop (mirrors proxy.GeneratePath). +func fixedInterpolate(templates [][2]string, variables map[string]interface{}, params map[string]string) { + for _, tmpl := range templates { + buff := tmpl[1] + for k, v := range params { + buff = strings.ReplaceAll(buff, "{{."+k+"}}", v) } + variables[tmpl[0]] = buff } +} - paramExtractor := func(params map[string]string) (*GraphQLRequest, error) { - val := GraphQLRequest{ - Query: opt.Query, - OperationName: opt.OperationName, - Variables: map[string]interface{}{}, - } - for k, v := range opt.Variables { - val.Variables[k] = v - } - for _, vs := range replacements { - val.Variables[vs[0]] = params[vs[1]] - } - return &val, nil +var ( + // legacy: replacements built as [varKey, capitalizedParamKey] + legacyReplacements = [][2]string{ + {"single", "Owner"}, } - return &Extractor{ - cfg: opt, - paramExtractor: paramExtractor, - newBody: func(params map[string]string) ([]byte, error) { - val, err := paramExtractor(params) - if err != nil { - return []byte{}, err - } - return json.Marshal(val) - }, + // fixed: templates built as [varKey, "{{.Owner}}/{{.Repo}} ..."] + fixedTemplates = [][2]string{ + {"single", "{{.Owner}}"}, + {"compound", "repo:{{.Owner}}/{{.Repo}} category:Announcements"}, } -} -var benchCfg = config.ExtraConfig{ - Namespace: map[string]interface{}{ - "type": OperationQuery, - "query": "{ search(query: $query) { nodes { id } } }", - "variables": map[string]interface{}{ - "single": "{owner}", - "compound": "repo:{owner}/{repo} category:Announcements", - "static": "no-params-here", - }, - }, -} + benchVariables = map[string]interface{}{ + "single": "{owner}", + "compound": "repo:{owner}/{repo} category:Announcements", + "static": "no-params-here", + } -var benchParams = map[string]string{ - "Owner": "krakend", - "Repo": "lura", -} + benchParams = map[string]string{ + "Owner": "krakend", + "Repo": "lura", + } +) -func BenchmarkBodyFromParams_legacy(b *testing.B) { - cfg, _ := GetOptions(benchCfg) - extractor := legacyNew(*cfg) +func BenchmarkInterpolation_legacy(b *testing.B) { b.ReportAllocs() - b.ResetTimer() for i := 0; i < b.N; i++ { - extractor.BodyFromParams(benchParams) + vars := map[string]interface{}{} + for k, v := range benchVariables { + vars[k] = v + } + legacyInterpolate(legacyReplacements, vars, benchParams) } } -func BenchmarkBodyFromParams_fixed(b *testing.B) { - cfg, _ := GetOptions(benchCfg) - extractor := New(*cfg) +func BenchmarkInterpolation_fixed(b *testing.B) { b.ReportAllocs() - b.ResetTimer() for i := 0; i < b.N; i++ { - extractor.BodyFromParams(benchParams) + vars := map[string]interface{}{} + for k, v := range benchVariables { + vars[k] = v + } + fixedInterpolate(fixedTemplates, vars, benchParams) } } From 9a01a93ddd921ec21da9a587cea1fccf8703e3ac Mon Sep 17 00:00:00 2001 From: Alexander Babenko Date: Thu, 9 Apr 2026 16:36:14 +0700 Subject: [PATCH 4/5] update graphql/graphql.go to fix inconsistent interpolation behavior without slowing down the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Here are the benchmarks isolating just the interpolation step (no JSON marshaling): ▎ BenchmarkInterpolation_legacy ~103 ns/op 16 B/op 1 allocs/op ▎ BenchmarkInterpolation_fixed ~425 ns/op 136 B/op 5 allocs/op The difference is expected and honest: the legacy code silently skipped compound variable strings like "repo:{owner}/{repo} ..." entirely — so it had nothing to substitute. The fixed version correctly processes both the single-param and compound cases, plus copies the variables map on each request. The overhead is the cost of functionality that previously didn't work at all. The approach mirrors config.go:665 (setup-time transformation) and proxy/request.go:GeneratePath (per-request bytes.ReplaceAll loop), so it should be familiar. Signed-off-by: Alexander Babenko --- transport/http/client/graphql/graphql.go | 32 ++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/transport/http/client/graphql/graphql.go b/transport/http/client/graphql/graphql.go index ae8ef0dc3..a941ea1ea 100644 --- a/transport/http/client/graphql/graphql.go +++ b/transport/http/client/graphql/graphql.go @@ -96,7 +96,10 @@ func GetOptions(cfg config.ExtraConfig) (*Options, error) { // New resturns a new Extractor, ready to be use on a middleware func New(opt Options) *Extractor { - var replacements [][2]string + // templates holds [varKey, templateString] pairs where {param} has been + // pre-converted to {{.Param}} at setup time, mirroring the approach used + // in config.go for url_pattern interpolation. + var templates [][2]string title := cases.Title(language.Und) for k, v := range opt.Variables { @@ -104,12 +107,17 @@ func New(opt Options) *Extractor { if !ok { continue } - if urlKeysPattern.MatchString(val) { - replacements = append(replacements, [2]string{k, val}) + // Convert all {param} occurrences to {{.Param}} once at setup time. + tmpl := urlKeysPattern.ReplaceAllStringFunc(val, func(match string) string { + param := match[1 : len(match)-1] + return "{{." + title.String(param[:1]) + param[1:] + "}}" + }) + if tmpl != val { + templates = append(templates, [2]string{k, tmpl}) } } - if len(replacements) == 0 { + if len(templates) == 0 { b, _ := json.Marshal(opt.GraphQLRequest) return &Extractor{ @@ -132,15 +140,13 @@ func New(opt Options) *Extractor { for k, v := range opt.Variables { val.Variables[k] = v } - for _, vs := range replacements { - val.Variables[vs[0]] = urlKeysPattern.ReplaceAllStringFunc(vs[1], func(match string) string { - param := match[1 : len(match)-1] - key := title.String(param[:1]) + param[1:] - if v, ok := params[key]; ok { - return v - } - return match - }) + // Per-request: plain strings.ReplaceAll, no regex — same as proxy.GeneratePath. + for _, tmpl := range templates { + buff := tmpl[1] + for k, v := range params { + buff = strings.ReplaceAll(buff, "{{."+k+"}}", v) + } + val.Variables[tmpl[0]] = buff } return &val, nil } From 77406613540111dc4483217634ffdddf761faa99 Mon Sep 17 00:00:00 2001 From: Alexander Babenko Date: Thu, 9 Apr 2026 16:52:55 +0700 Subject: [PATCH 5/5] update graphql benchmark test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no_params 2 ns/op 0 B/op 0 allocs ← static single_param 605 ns/op 584 B/op 9 allocs compound_params 700 ns/op 704 B/op 10 allocs ← new case mixed 1118 ns/op 920 B/op 16 allocs Signed-off-by: Alexander Babenko --- .../client/graphql/graphql_benchmark_test.go | 104 ++++++++---------- 1 file changed, 46 insertions(+), 58 deletions(-) diff --git a/transport/http/client/graphql/graphql_benchmark_test.go b/transport/http/client/graphql/graphql_benchmark_test.go index dd3459b5a..11e744c56 100644 --- a/transport/http/client/graphql/graphql_benchmark_test.go +++ b/transport/http/client/graphql/graphql_benchmark_test.go @@ -3,70 +3,58 @@ package graphql import ( - "strings" "testing" -) - -// legacyInterpolate reproduces the original per-request substitution: direct map lookup. -func legacyInterpolate(replacements [][2]string, variables map[string]interface{}, params map[string]string) { - for _, vs := range replacements { - variables[vs[0]] = params[vs[1]] - } -} - -// fixedInterpolate is the new per-request substitution: strings.ReplaceAll loop (mirrors proxy.GeneratePath). -func fixedInterpolate(templates [][2]string, variables map[string]interface{}, params map[string]string) { - for _, tmpl := range templates { - buff := tmpl[1] - for k, v := range params { - buff = strings.ReplaceAll(buff, "{{."+k+"}}", v) - } - variables[tmpl[0]] = buff - } -} - -var ( - // legacy: replacements built as [varKey, capitalizedParamKey] - legacyReplacements = [][2]string{ - {"single", "Owner"}, - } - // fixed: templates built as [varKey, "{{.Owner}}/{{.Repo}} ..."] - fixedTemplates = [][2]string{ - {"single", "{{.Owner}}"}, - {"compound", "repo:{{.Owner}}/{{.Repo}} category:Announcements"}, - } - - benchVariables = map[string]interface{}{ - "single": "{owner}", - "compound": "repo:{owner}/{repo} category:Announcements", - "static": "no-params-here", - } + "github.com/luraproject/lura/v2/config" +) - benchParams = map[string]string{ +func BenchmarkExtractor_BodyFromParams(b *testing.B) { + params := map[string]string{ "Owner": "krakend", "Repo": "lura", } -) - -func BenchmarkInterpolation_legacy(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - vars := map[string]interface{}{} - for k, v := range benchVariables { - vars[k] = v - } - legacyInterpolate(legacyReplacements, vars, benchParams) - } -} -func BenchmarkInterpolation_fixed(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - vars := map[string]interface{}{} - for k, v := range benchVariables { - vars[k] = v - } - fixedInterpolate(fixedTemplates, vars, benchParams) + for _, tc := range []struct { + name string + variables map[string]interface{} + }{ + { + name: "no_params", + variables: map[string]interface{}{"static": "no-params-here"}, + }, + { + name: "single_param", + variables: map[string]interface{}{"owner": "{owner}"}, + }, + { + name: "compound_params", + variables: map[string]interface{}{ + "query": "repo:{owner}/{repo} category:Announcements", + }, + }, + { + name: "mixed", + variables: map[string]interface{}{ + "single": "{owner}", + "compound": "repo:{owner}/{repo} category:Announcements", + "static": "no-params-here", + }, + }, + } { + cfg, _ := GetOptions(config.ExtraConfig{ + Namespace: map[string]interface{}{ + "type": OperationQuery, + "query": "{ search(query: $query) { nodes { id } } }", + "variables": tc.variables, + }, + }) + extractor := New(*cfg) + + b.Run(tc.name, func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + extractor.BodyFromParams(params) + } + }) } }