Skip to content

Commit

Permalink
Fix missing type checks while using index expressions (#391)
Browse files Browse the repository at this point in the history
* Fix missing type checks while using index expressions

* Fixup nested traversal walks and missing tests

* Fixup template expression part replacement

* Fixup still support simple template expression with empty string fallback

* Add changelog

* Added template wrap expr to test case

* print test with test case name when error

Co-authored-by: Johannes Koch <johannes.koch@avenga.com>
  • Loading branch information
Marcel Ludwig and Johannes Koch authored Nov 24, 2021
1 parent eaa1899 commit 8ede2ca
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 19 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<label>.attributes.<name>` ([#369](https://github.com/avenga/couper/pull/369))
* Missing support for RelativeTraversalExpr, IndexExpr, UnaryOpExpr ([#389](https://github.com/avenga/couper/pull/389))
* HCL: Missing support for RelativeTraversalExpr, IndexExpr, UnaryOpExpr ([#389](https://github.com/avenga/couper/pull/389))
* HCL: Missing support for different variable index key types ([#391](https://github.com/avenga/couper/pull/391))

---

Expand Down
38 changes: 28 additions & 10 deletions eval/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,15 @@ 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

// "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
Expand Down Expand Up @@ -319,13 +320,30 @@ 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 !variables.CanIterateElements() {
return fallback
}

switch t.Key.Type() {
case cty.Number:
if variables.HasIndex(t.Key).True() {
if hasNext {
fidx := t.Key.AsBigFloat()
idx, _ := fidx.Int64()
return walk(variables.AsValueSlice()[idx], fallback, traversal[1:])
}
return variables
}
case cty.String:
current, exist := currentFn(t.Key.AsString())
if !exist {
return fallback
}
if hasNext {
return walk(variables, fallback, traversal[1:])
return walk(current, fallback, traversal[1:])
}
return variables
}

return fallback
default:
panic("eval: unsupported traversal: " + reflect.TypeOf(t).String())
Expand Down
21 changes: 14 additions & 7 deletions eval/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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},
Expand All @@ -48,12 +55,12 @@ func TestValue(t *testing.T) {
for _, attr := range attrs {
got, err := Value(evalCtx, attr.Expr)
if (err != nil) != tt.wantErr {
t.Errorf("Value() error = %v, wantErr %v", err, tt.wantErr)
t.Error(err.(errors.GoError).LogError())
st.Errorf("Value() error = %v, wantErr %v", err, tt.wantErr)
st.Error(err.(errors.GoError).LogError())
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Value() got = %v, want %v", got.GoString(), tt.want.GoString())
st.Errorf("Value() got = %v, want %v", got.GoString(), tt.want.GoString())
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion server/http_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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", "X-Default-6": "default"}, http.StatusOK},
} {
t.Run(tc.path[1:], func(subT *testing.T) {
helper := test.New(subT)
Expand Down
2 changes: 2 additions & 0 deletions server/testdata/integration/functions/01_couper.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ 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
x-default-6 = "${default(request.cookies.undef, "default")}" # template wrap expr
}
}
}
Expand Down

0 comments on commit 8ede2ca

Please sign in to comment.