Repository: goadesign/goa
Pull request #2430 (2019-12-14 19:07:52)

Properly validate attributes used in HTTP headers.
Such attributes may only be primitive types or array of primitive types.
The code previously allowed for array of objects for example which would
cause incorrect code to be generated.


Patch:
diff --git a/expr/http_endpoint.go b/expr/http_endpoint.go
index 5e7264ed..f1c53818 100644
--- a/expr/http_endpoint.go
+++ b/expr/http_endpoint.go
@@ -388,6 +388,9 @@ func (e *HTTPEndpointExpr) Validate() error {
 			if hasParams {
 				verr.Add(e, "Payload type is array but HTTP endpoint defines both route or query string parameters and headers. At most one parameter or header must be defined and it must be of type array.")
 			}
+			if !IsPrimitive(AsArray(e.MethodExpr.Payload.Type).ElemType.Type) {
+				verr.Add(e, "Array payloads used in HTTP headers must be of arrays of primitive types.")
+			}
 		}
 		if e.Body != nil && e.Body.Type != Empty {
 			if e.MultipartRequest {
diff --git a/expr/http_response.go b/expr/http_response.go
index 7ac915ec..595f50fb 100644
--- a/expr/http_response.go
+++ b/expr/http_response.go
@@ -150,23 +150,28 @@ func (r *HTTPResponseExpr) Validate(e *HTTPEndpointExpr) *eval.ValidationErrors
 		}
 	}
 
-	hasAttribute := func(name string) bool {
+	resultAttributeType := func(name string) DataType {
 		if !IsObject(e.MethodExpr.Result.Type) {
-			return false
+			return nil
 		}
 		if !isrt {
-			return e.MethodExpr.Result.Find(name) != nil
+			return e.MethodExpr.Result.Find(name).Type
 		}
 		if v, ok := e.MethodExpr.Result.Meta["view"]; ok {
-			return rt.ViewHasAttribute(v[0], name)
+			v := rt.View(v[0])
+			if v == nil {
+				return nil
+			}
+			return v.AttributeExpr.Find(name).Type
 		}
 		for _, v := range rt.Views {
 			if !rt.ViewHasAttribute(v.Name, name) {
-				return false
+				return nil
 			}
 		}
-		return true
+		return e.MethodExpr.Result.Find(name).Type
 	}
+
 	if !r.Headers.IsEmpty() {
 		verr.Merge(r.Headers.Validate("HTTP response headers", r))
 		if e.MethodExpr.Result.Type == Empty {
@@ -174,23 +179,35 @@ func (r *HTTPResponseExpr) Validate(e *HTTPEndpointExpr) *eval.ValidationErrors
 		} else if IsObject(e.MethodExpr.Result.Type) {
 			mobj := AsObject(r.Headers.Type)
 			for _, h := range *mobj {
-				if !hasAttribute(h.Name) {
+				t := resultAttributeType(h.Name)
+				if t == nil {
 					verr.Add(r, "header %q has no equivalent attribute in%s result type, use notation 'attribute_name:header_name' to identify corresponding result type attribute.", h.Name, inview)
 				}
+				if IsArray(t) {
+					if !IsPrimitive(AsArray(t).ElemType.Type) {
+						verr.Add(e, "attribute %q used in HTTP headers must be a primitive type or an array of primitive types.", h.Name)
+					}
+				} else if !IsPrimitive(t) {
+					verr.Add(e, "attribute %q used in HTTP headers must be a primitive type or an array of primitive types.", h.Name)
+				}
 			}
 		} else if len(*AsObject(r.Headers.Type)) > 1 {
 			verr.Add(r, "response defines more than one header but result type is not an object")
+		} else if IsArray(e.MethodExpr.Result.Type) {
+			if !IsPrimitive(AsArray(e.MethodExpr.Result.Type).ElemType.Type) {
+				verr.Add(e, "Array result is mapped to an HTTP header but is not an array of primitive types.")
+			}
 		}
 	}
 	if r.Body != nil {
 		verr.Merge(r.Body.Validate("HTTP response body", r))
 		if att, ok := r.Body.Meta["origin:attribute"]; ok {
-			if !hasAttribute(att[0]) {
+			if resultAttributeType(att[0]) == nil {
 				verr.Add(r, "body %q has no equivalent attribute in%s result type", att[0], inview)
 			}
 		} else if bobj := AsObject(r.Body.Type); bobj != nil {
 			for _, n := range *bobj {
-				if !hasAttribute(n.Name) {
+				if resultAttributeType(n.Name) == nil {
 					verr.Add(r, "body %q has no equivalent attribute in%s result type", n.Name, inview)
 				}
 			}


Test patch:
diff --git a/expr/http_response_test.go b/expr/http_response_test.go
index 141868e0..aa5222bd 100644
--- a/expr/http_response_test.go
+++ b/expr/http_response_test.go
@@ -20,8 +20,10 @@ func TestHTTPResponseValidation(t *testing.T) {
 		{"string result", stringResultResponseWithTextContentTypeDSL, ""},
 		{"object result", objectResultResponseWithHeadersDSL, ""},
 		{"array result", arrayResultResponseWithHeadersDSL, ""},
-		{"map result", mapResultResponseWithHeadersDSL, ""},
+		{"map result", mapResultResponseWithHeadersDSL, `service "MapResultResponseWithHeaders" HTTP endpoint "Method": attribute "foo" used in HTTP headers must be a primitive type or an array of primitive types.`},
 		{"invalid", emptyResultResponseWithHeadersDSL, `HTTP response of service "EmptyResultResponseWithHeaders" HTTP endpoint "Method": response defines headers but result is empty`},
+		{"implicit object in header", implicitObjectResultResponseWithHeadersDSL, `service "ArrayObjectResultResponseWithHeaders" HTTP endpoint "Method": attribute "foo" used in HTTP headers must be a primitive type or an array of primitive types.`},
+		{"array of object in header", arrayObjectResultResponseWithHeadersDSL, `service "ArrayObjectResultResponseWithHeaders" HTTP endpoint "Method": Array result is mapped to an HTTP header but is not an array of primitive types.`},
 		{"not string or []byte", intResultResponseWithTextContentTypeDSL, `HTTP response of service "StringResultResponseWithHeaders" HTTP endpoint "Method": Result type must be String or Bytes when ContentType is 'text/plain'`},
 	}
 	for _, c := range cases {
@@ -114,6 +116,42 @@ var objectResultResponseWithHeadersDSL = func() {
 	})
 }
 
+var implicitObjectResultResponseWithHeadersDSL = func() {
+	Service("ArrayObjectResultResponseWithHeaders", func() {
+		Method("Method", func() {
+			Result(func() {
+				Attribute("foo", func() {
+					Attribute("bar", String)
+					Attribute("baz", String)
+				})
+			})
+			HTTP(func() {
+				POST("/")
+				Response(func() {
+					Header("foo:Location")
+				})
+			})
+		})
+	})
+}
+
+var arrayObjectResultResponseWithHeadersDSL = func() {
+	var Obj = Type("Obj", func() {
+		Attribute("foo", String)
+	})
+	Service("ArrayObjectResultResponseWithHeaders", func() {
+		Method("Method", func() {
+			Result(ArrayOf(Obj))
+			HTTP(func() {
+				POST("/")
+				Response(func() {
+					Header("foo:Location")
+				})
+			})
+		})
+	})
+}
+
 var arrayResultResponseWithHeadersDSL = func() {
 	Service("ArrayResultResponseWithHeaders", func() {
 		Method("Method", func() {