From 8d6044b734e028df6c9968ea30ccd3deb234152f Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Tue, 23 Nov 2021 20:32:42 +0100 Subject: [PATCH 1/7] Fix missing type checks while using index expressions --- eval/value.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/eval/value.go b/eval/value.go index 29a5f1ebd..1cc7a9b07 100644 --- a/eval/value.go +++ b/eval/value.go @@ -319,13 +319,26 @@ func walk(variables, fallback cty.Value, traversal hcl.Traversal) cty.Value { } return walk(current, fallback, traversal[1:]) case hcl.TraverseIndex: - if variables.HasIndex(t.Key).True() { - if hasNext { - return walk(variables, fallback, traversal[1:]) - } - return variables + if !variables.CanIterateElements() { + return fallback } + switch t.Key.Type() { + case cty.Number: + if variables.HasIndex(t.Key).True() { + if hasNext { + return walk(variables, fallback, traversal[1:]) + } + return variables + } + case cty.String: + if variables.GetAttr(t.Key.AsString()).IsWhollyKnown() { + if hasNext { + return walk(variables, fallback, traversal[1:]) + } + return variables + } + } return fallback default: panic("eval: unsupported traversal: " + reflect.TypeOf(t).String()) From f7242d61fff6dbbf179e1f57ae9402d99782efc1 Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Wed, 24 Nov 2021 08:57:09 +0100 Subject: [PATCH 2/7] Fixup nested traversal walks and missing tests --- eval/value.go | 16 ++++++++++------ eval/value_test.go | 15 +++++++++++---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/eval/value.go b/eval/value.go index 1cc7a9b07..c8d195b3b 100644 --- a/eval/value.go +++ b/eval/value.go @@ -327,17 +327,21 @@ func walk(variables, fallback cty.Value, traversal hcl.Traversal) cty.Value { case cty.Number: if variables.HasIndex(t.Key).True() { if hasNext { - return walk(variables, fallback, traversal[1:]) + fidx := t.Key.AsBigFloat() + idx, _ := fidx.Int64() + return walk(variables.AsValueSlice()[idx], fallback, traversal[1:]) } return variables } case cty.String: - if variables.GetAttr(t.Key.AsString()).IsWhollyKnown() { - if hasNext { - return walk(variables, fallback, traversal[1:]) - } - return variables + current, exist := currentFn(t.Key.AsString()) + if !exist { + return fallback + } + if hasNext { + return walk(current, fallback, traversal[1:]) } + return variables } return fallback default: diff --git a/eval/value_test.go b/eval/value_test.go index 1347e8298..4733b2570 100644 --- a/eval/value_test.go +++ b/eval/value_test.go @@ -4,19 +4,20 @@ import ( "reflect" "testing" - "github.com/avenga/couper/errors" - + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" "github.com/avenga/couper/config" - "github.com/hashicorp/hcl/v2" - "github.com/zclconf/go-cty/cty" + "github.com/avenga/couper/errors" + "github.com/avenga/couper/internal/seetie" ) func TestValue(t *testing.T) { evalCtx := NewContext(nil, &config.Defaults{}).HCLContext() rootObj := cty.ObjectVal(map[string]cty.Value{ "exist": cty.StringVal("here"), + "slice": seetie.GoToValue([]string{"1", "2"}), }) evalCtx.Variables["rootvar"] = rootObj @@ -28,6 +29,12 @@ func TestValue(t *testing.T) { }{ {"root non nil", "key = rootvar", rootObj, false}, {"child non nil", "key = rootvar.exist", cty.StringVal("here"), false}, + {"child non nil, string key idx expr", `key = rootvar["exist"]`, cty.StringVal("here"), false}, + {"child nil, string key idx expr", `key = rootvar["not"]`, cty.NilVal, false}, + {"child non nil, string key idx expr iterate", `key = rootvar["exist"][1]`, cty.NilVal, false}, + {"child non nil, number key idx expr", `key = rootvar.slice[1]`, cty.StringVal("2"), false}, + {"child non nil, number key idx expr iterate", `key = rootvar.slice[1]["not"]`, cty.NilVal, false}, + {"child non nil, idx nil", `key = rootvar.slice[5]`, cty.NilVal, false}, {"child nil", "key = rootvar.child", cty.NilVal, false}, {"child idx nil", "key = rootvar.child[2].sub", cty.NilVal, false}, {"template attr value exp empty string", `key = "prefix${rootvar.child}"`, cty.StringVal("prefix"), false}, From a63f39fff03cfca22d3832ea45e4414a11922553 Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Wed, 24 Nov 2021 09:24:36 +0100 Subject: [PATCH 3/7] Fixup template expression part replacement --- eval/value.go | 7 +------ server/http_integration_test.go | 2 +- server/testdata/integration/functions/01_couper.hcl | 1 + 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/eval/value.go b/eval/value.go index c8d195b3b..721408037 100644 --- a/eval/value.go +++ b/eval/value.go @@ -110,12 +110,7 @@ func newLiteralValueExpr(ctx *hcl.EvalContext, exp hcl.Expression) hclsyntax.Exp return expr case *hclsyntax.TemplateExpr: for p, part := range expr.Parts { - for _, v := range part.Variables() { - if traversalValue(ctx.Variables, v) == cty.NilVal { - expr.Parts[p] = &hclsyntax.LiteralValueExpr{Val: emptyStringVal, SrcRange: v.SourceRange()} - break - } - } + expr.Parts[p] = newLiteralValueExpr(ctx, part) } return expr case *hclsyntax.TemplateWrapExpr: diff --git a/server/http_integration_test.go b/server/http_integration_test.go index 7421152c2..aedaac55f 100644 --- a/server/http_integration_test.go +++ b/server/http_integration_test.go @@ -3584,7 +3584,7 @@ func TestFunctions(t *testing.T) { for _, tc := range []testCase{ {"merge", "/v1/merge", map[string]string{"X-Merged-1": "{\"foo\":[1,2]}", "X-Merged-2": "{\"bar\":[3,4]}", "X-Merged-3": "[\"a\",\"b\"]"}, http.StatusOK}, {"coalesce", "/v1/coalesce?q=a", map[string]string{"X-Coalesce-1": "/v1/coalesce", "X-Coalesce-2": "default", "X-Coalesce-3": "default", "X-Coalesce-4": "default"}, http.StatusOK}, - {"default", "/v1/default?q=a", map[string]string{"X-Default-1": "/v1/default", "X-Default-2": "default", "X-Default-3": "default", "X-Default-4": "default"}, http.StatusOK}, + {"default", "/v1/default?q=a", map[string]string{"X-Default-1": "/v1/default", "X-Default-2": "default", "X-Default-3": "default", "X-Default-4": "default", "X-Default-5": "prefix-default"}, http.StatusOK}, } { t.Run(tc.path[1:], func(subT *testing.T) { helper := test.New(subT) diff --git a/server/testdata/integration/functions/01_couper.hcl b/server/testdata/integration/functions/01_couper.hcl index b2c4e29fc..7070b850c 100644 --- a/server/testdata/integration/functions/01_couper.hcl +++ b/server/testdata/integration/functions/01_couper.hcl @@ -30,6 +30,7 @@ server "api" { x-default-2 = default(request.cookies.undef, "default") x-default-3 = default(request.query.q[1], "default") x-default-4 = default(request.cookies.undef, request.query.q[1], "default", request.path) + x-default-5 = "prefix-${default(request.cookies.undef, "default")}" # template expr } } } From 3552934cbd031e2acdf5de3579c524dccd06b39a Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Wed, 24 Nov 2021 09:50:28 +0100 Subject: [PATCH 4/7] Fixup still support simple template expression with empty string fallback --- eval/value.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/eval/value.go b/eval/value.go index 721408037..ea439cb9a 100644 --- a/eval/value.go +++ b/eval/value.go @@ -112,7 +112,13 @@ func newLiteralValueExpr(ctx *hcl.EvalContext, exp hcl.Expression) hclsyntax.Exp for p, part := range expr.Parts { expr.Parts[p] = newLiteralValueExpr(ctx, part) } - return expr + + // "pre"-evaluate to be able to combine string expressions with empty strings on NilVal result. + c, _ := expr.Value(ctx) + if c.IsNull() { + return &hclsyntax.LiteralValueExpr{Val: emptyStringVal, SrcRange: expr.Range()} + } + return &hclsyntax.LiteralValueExpr{Val: c, SrcRange: expr.Range()} case *hclsyntax.TemplateWrapExpr: if val := newLiteralValueExpr(ctx, expr.Wrapped); val != nil { expr.Wrapped = val From 991f5ade385bbb0550f7a2aa42c393a9c97624ae Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Wed, 24 Nov 2021 09:53:11 +0100 Subject: [PATCH 5/7] Add changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b50920fd..2089c6ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,8 @@ Unreleased changes are available as `avenga/couper:edge` container. * Reduced memory usage for backend response bodies which just get piped to the client and are not required to be read by Couper due to a variable references ([#375](https://github.com/avenga/couper/pull/375)) * However, if a huge message body is passed and additionally referenced via e.g. `json_body`, Couper may require a lot of memory for storing the data structure. * For each SAML attribute listed in [`array_attributes`](./docs/REFERENCE.md#saml-block) at least an empty array is created in `request.context.