Repository: avenga/couper
Pull request #596 (2022-09-16 13:52:09)

Fix errors in `default()` function
use unsafe unified return type (like in `stdlib.CoalesceFunc`) from arguments ignoring `cty.NilType` arguments; and returning `cty.NilType` if all arguments are `cty.NilType` arguments (as expected by testcase `default(request.cookies.undef, request.cookies.undef)`)

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 40f6083c..0decd542 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
 
 * **Fixed**
   * [endpoint `/**` path wildcards](https://docs.couper.io/configuration/block/endpoint) sometimes not matching ([#603](https://github.com/avenga/couper/pull/603))
+  * Some errors in the [`default()` function](https://docs.couper.io/configuration/functions) ([#596](https://github.com/avenga/couper/pull/596))
 
 
 ---
diff --git a/eval/lib/default.go b/eval/lib/default.go
index ee8c7ebd..c94db819 100644
--- a/eval/lib/default.go
+++ b/eval/lib/default.go
@@ -17,11 +17,23 @@ var DefaultFunc = function.New(&function.Spec{
 		AllowNull:        true,
 	},
 	Type: func(args []cty.Value) (cty.Type, error) {
-		if len(args) < 2 {
-			return cty.NilType, fmt.Errorf("not enough arguments")
+		var argTypes []cty.Type
+		for _, val := range args {
+			// ignore NilType values when determining the unsafe-unified return type
+			if val.Type() == cty.NilType {
+				continue
+			}
+			argTypes = append(argTypes, val.Type())
+		}
+		if len(argTypes) == 0 {
+			// no non-NilVal arguments
+			return cty.NilType, nil
+		}
+		retType, _ := convert.UnifyUnsafe(argTypes)
+		if retType == cty.NilType {
+			return cty.NilType, fmt.Errorf("all defined arguments must have the same type")
 		}
-		// last argument defines the impl return type
-		return args[len(args)-1].Type(), nil
+		return retType, nil
 	},
 	Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
 		for _, argVal := range args {


Test patch:
diff --git a/eval/lib/default_test.go b/eval/lib/default_test.go
new file mode 100644
index 00000000..6be787bc
--- /dev/null
+++ b/eval/lib/default_test.go
@@ -0,0 +1,52 @@
+package lib_test
+
+import (
+	"testing"
+
+	"github.com/zclconf/go-cty/cty"
+
+	"github.com/avenga/couper/config/configload"
+	"github.com/avenga/couper/config/request"
+	"github.com/avenga/couper/eval"
+	"github.com/avenga/couper/internal/test"
+)
+
+func TestDefaultErrors(t *testing.T) {
+	helper := test.New(t)
+
+	cf, err := configload.LoadBytes([]byte(`server {}`), "couper.hcl")
+	helper.Must(err)
+
+	tests := []struct {
+		name    string
+		args    []cty.Value
+		wantErr string
+	}{
+		{
+			"mixed types",
+			[]cty.Value{
+				cty.TupleVal([]cty.Value{
+					cty.StringVal("1"),
+				}),
+				cty.ObjectVal(map[string]cty.Value{
+					"a": cty.NumberIntVal(1),
+				}),
+			},
+			"all defined arguments must have the same type",
+		},
+	}
+
+	hclContext := cf.Context.Value(request.ContextType).(*eval.Context).HCLContext()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(subT *testing.T) {
+			_, err := hclContext.Functions["default"].Call(tt.args)
+			if err == nil {
+				subT.Error("Error expected")
+			}
+			if err != nil && err.Error() != tt.wantErr {
+				subT.Errorf("Wrong error message; expected %#v, got: %#v", tt.wantErr, err.Error())
+			}
+		})
+	}
+}
diff --git a/server/http_integration_test.go b/server/http_integration_test.go
index 83b6690a..8fc746d8 100644
--- a/server/http_integration_test.go
+++ b/server/http_integration_test.go
@@ -4110,6 +4110,9 @@ func TestFunctions(t *testing.T) {
 			"X-Default-10": "",
 			"X-Default-11": "0",
 			"X-Default-12": "",
+			"X-Default-13": `{"a":1}`,
+			"X-Default-14": `{"a":1}`,
+			"X-Default-15": `[1,2]`,
 		}, http.StatusOK},
 		{"contains", "/v1/contains", map[string]string{
 			"X-Contains-1":  "yes",
@@ -4233,10 +4236,10 @@ func TestFunction_to_number_errors(t *testing.T) {
 	}
 
 	for _, tc := range []testCase{
-		{"string", "/v1/to_number/string", wd + `/01_couper.hcl:62,23-28: Invalid function argument; Invalid value for "v" parameter: cannot convert "two" to number; given string must be a decimal representation of a number.`},
-		{"bool", "/v1/to_number/bool", wd + `/01_couper.hcl:70,23-27: Invalid function argument; Invalid value for "v" parameter: cannot convert bool to number.`},
-		{"tuple", "/v1/to_number/tuple", wd + `/01_couper.hcl:78,23-24: Invalid function argument; Invalid value for "v" parameter: cannot convert tuple to number.`},
-		{"object", "/v1/to_number/object", wd + `/01_couper.hcl:86,23-24: Invalid function argument; Invalid value for "v" parameter: cannot convert object to number.`},
+		{"string", "/v1/to_number/string", wd + `/01_couper.hcl:65,23-28: Invalid function argument; Invalid value for "v" parameter: cannot convert "two" to number; given string must be a decimal representation of a number.`},
+		{"bool", "/v1/to_number/bool", wd + `/01_couper.hcl:73,23-27: Invalid function argument; Invalid value for "v" parameter: cannot convert bool to number.`},
+		{"tuple", "/v1/to_number/tuple", wd + `/01_couper.hcl:81,23-24: Invalid function argument; Invalid value for "v" parameter: cannot convert tuple to number.`},
+		{"object", "/v1/to_number/object", wd + `/01_couper.hcl:89,23-24: Invalid function argument; Invalid value for "v" parameter: cannot convert object to number.`},
 	} {
 		t.Run(tc.path[1:], func(subT *testing.T) {
 			helper := test.New(subT)
@@ -4277,9 +4280,9 @@ func TestFunction_length_errors(t *testing.T) {
 	}
 
 	for _, tc := range []testCase{
-		{"object", "/v1/length/object", wd + `/01_couper.hcl:123,19-26: Error in function call; Call to function "length" failed: collection must be a list, a map or a tuple.`},
-		{"string", "/v1/length/string", wd + `/01_couper.hcl:131,19-26: Error in function call; Call to function "length" failed: collection must be a list, a map or a tuple.`},
-		{"null", "/v1/length/null", wd + `/01_couper.hcl:139,26-30: Invalid function argument; Invalid value for "collection" parameter: argument must not be null.`},
+		{"object", "/v1/length/object", wd + `/01_couper.hcl:126,19-26: Error in function call; Call to function "length" failed: collection must be a list, a map or a tuple.`},
+		{"string", "/v1/length/string", wd + `/01_couper.hcl:134,19-26: Error in function call; Call to function "length" failed: collection must be a list, a map or a tuple.`},
+		{"null", "/v1/length/null", wd + `/01_couper.hcl:142,26-30: Invalid function argument; Invalid value for "collection" parameter: argument must not be null.`},
 	} {
 		t.Run(tc.path[1:], func(subT *testing.T) {
 			helper := test.New(subT)
@@ -4320,7 +4323,7 @@ func TestFunction_lookup_errors(t *testing.T) {
 	}
 
 	for _, tc := range []testCase{
-		{"null inputMap", "/v1/lookup/inputMap-null", wd + `/01_couper.hcl:200,26-30: Invalid function argument; Invalid value for "inputMap" parameter: argument must not be null.`},
+		{"null inputMap", "/v1/lookup/inputMap-null", wd + `/01_couper.hcl:203,26-30: Invalid function argument; Invalid value for "inputMap" parameter: argument must not be null.`},
 	} {
 		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 0d5b42f2..e8f26564 100644
--- a/server/testdata/integration/functions/01_couper.hcl
+++ b/server/testdata/integration/functions/01_couper.hcl
@@ -38,6 +38,9 @@ server "api" {
           x-default-10 = default(request.cookies.undef, request.cookies.undef)
           x-default-11 = default(request.cookies.undef, 0)
           x-default-12 = default(request.cookies.undef, false)
+          x-default-13 = json_encode(default({a = 1}, {}))
+          x-default-14 = json_encode(default({a = 1}, {b = 2}))
+          x-default-15 = json_encode(default([1, 2], []))
         }
       }
     }