From 09bb970965e506dc0a4d1408bbf12e352d1bfad9 Mon Sep 17 00:00:00 2001 From: Steve Lessard Date: Thu, 15 Dec 2022 18:34:26 -0800 Subject: [PATCH 1/5] issue #709 - validate response headers against their respective schema Previously the code assumed response headers were only strings. Now the code can validate any type of response header --- openapi3/parameter.go | 2 +- openapi3filter/issue201_test.go | 2 +- openapi3filter/validate_response.go | 70 ++++++++++++++++++++++------- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/openapi3/parameter.go b/openapi3/parameter.go index 9124d92a4..c55af474d 100644 --- a/openapi3/parameter.go +++ b/openapi3/parameter.go @@ -90,7 +90,7 @@ func (parameters Parameters) Validate(ctx context.Context, opts ...ValidationOpt } // Parameter is specified by OpenAPI/Swagger 3.0 standard. -// See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#parameterObject +// See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#parameter-object type Parameter struct { ExtensionProps `json:"-" yaml:"-"` diff --git a/openapi3filter/issue201_test.go b/openapi3filter/issue201_test.go index 8b2b99d0e..0d363a17e 100644 --- a/openapi3filter/issue201_test.go +++ b/openapi3filter/issue201_test.go @@ -94,7 +94,7 @@ paths: }, "invalid required header": { - err: `response header "X-Blup" doesn't match the schema: string "bluuuuuup" doesn't match the regular expression "^blup$"`, + err: `response header "X-Blup" doesn't match schema: string "bluuuuuup" doesn't match the regular expression "^blup$"`, headers: map[string]string{ "X-Blip": "blip", "x-blop": "blop", diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index 27bef82d3..9804949a7 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -81,21 +81,9 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error for _, k := range headers { s := response.Headers[k] h := input.Header.Get(k) - if h == "" { - if s.Value.Required { - return &ResponseError{ - Input: input, - Reason: fmt.Sprintf("response header %q missing", k), - } - } - continue - } - if err := s.Value.Schema.Value.VisitJSON(h, opts...); err != nil { - return &ResponseError{ - Input: input, - Reason: fmt.Sprintf("response header %q doesn't match the schema", k), - Err: err, - } + err := validateResponseHeader(k, h, s, input, opts) + if err != nil { + return err } } @@ -171,6 +159,58 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error return nil } +func validateResponseHeader( + headerName string, + headerVal string, + s *openapi3.HeaderRef, + input *ResponseValidationInput, + opts []openapi3.SchemaValidationOption, +) error { + if headerVal == "" && s.Value.Required { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("response header %q missing", headerName), + } + } + + sm, err := s.Value.SerializationMethod() + if err != nil { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("unable to get header %q serialization method", headerName), + Err: err, + } + } + + var decodedValue interface{} + dec := &headerParamDecoder{header: input.Header} + found := false + if decodedValue, found, err = decodeValue(dec, headerName, sm, s.Value.Schema, s.Value.Required); err != nil { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("unable to decode header %q value", headerName), + Err: err, + } + } + + if found { + // If the value was found but not decoded it means there is no + // schema.Type defining how to interpret the header value + if decodedValue == nil { + // revert to the header's original value + decodedValue = headerVal + } + if err := s.Value.Schema.Value.VisitJSON(decodedValue, opts...); err != nil { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("response header %q doesn't match schema", headerName), + Err: err, + } + } + } + return nil +} + // getSchemaIdentifier gets something by which a schema could be identified. // A schema by itself doesn't have a true identity field. This function makes // a best effort to get a value that can fill that void. From c8b9d463a499dbdadab5226d55aa7b731b005030 Mon Sep 17 00:00:00 2001 From: Steve Lessard Date: Fri, 16 Dec 2022 19:32:52 -0800 Subject: [PATCH 2/5] #709 - Unit tests --- openapi3/schema.go | 4 + openapi3filter/req_resp_decoder.go | 4 +- openapi3filter/validate_response.go | 51 ++---- openapi3filter/validate_response_test.go | 214 +++++++++++++++++++++++ 4 files changed, 235 insertions(+), 38 deletions(-) create mode 100644 openapi3filter/validate_response_test.go diff --git a/openapi3/schema.go b/openapi3/schema.go index c5af8536e..b8822a8f7 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -1042,6 +1042,10 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val return } +// The value is not considered in visitJSONNull because according to the spec +// "null is not supported as a type" unless `nullable` is also set to true +// https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#data-types +// https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#schema-object func (schema *Schema) visitJSONNull(settings *schemaValidationSettings) (err error) { if schema.Nullable { return diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 870651ce6..4791b4ad4 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -339,7 +339,7 @@ func decodeValue(dec valueDecoder, param string, sm *openapi3.SerializationMetho } _, found = vDecoder.values[param] case *headerParamDecoder: - _, found = vDecoder.header[param] + _, found = vDecoder.header[http.CanonicalHeaderKey(param)] case *cookieParamDecoder: _, err := vDecoder.req.Cookie(param) found = err != http.ErrNoCookie @@ -888,7 +888,7 @@ func parseArray(raw []string, schemaRef *openapi3.SchemaRef) ([]interface{}, err // parsePrimitive returns a value that is created by parsing a source string to a primitive type // that is specified by a schema. The function returns nil when the source string is empty. -// The function panics when a schema has a non primitive type. +// The function panics when a schema has a non-primitive type. func parsePrimitive(raw string, schema *openapi3.SchemaRef) (interface{}, error) { if raw == "" { return nil, nil diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index 9804949a7..1108031c9 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -78,10 +78,9 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error } } sort.Strings(headers) - for _, k := range headers { - s := response.Headers[k] - h := input.Header.Get(k) - err := validateResponseHeader(k, h, s, input, opts) + for _, headerName := range headers { + headerRef := response.Headers[headerName] + err := validateResponseHeader(headerName, headerRef, input, opts) if err != nil { return err } @@ -159,33 +158,14 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error return nil } -func validateResponseHeader( - headerName string, - headerVal string, - s *openapi3.HeaderRef, - input *ResponseValidationInput, - opts []openapi3.SchemaValidationOption, -) error { - if headerVal == "" && s.Value.Required { - return &ResponseError{ - Input: input, - Reason: fmt.Sprintf("response header %q missing", headerName), - } - } - - sm, err := s.Value.SerializationMethod() - if err != nil { - return &ResponseError{ - Input: input, - Reason: fmt.Sprintf("unable to get header %q serialization method", headerName), - Err: err, - } - } - +func validateResponseHeader(headerName string, headerRef *openapi3.HeaderRef, input *ResponseValidationInput, opts []openapi3.SchemaValidationOption, ) error { + var err error var decodedValue interface{} + var found bool dec := &headerParamDecoder{header: input.Header} - found := false - if decodedValue, found, err = decodeValue(dec, headerName, sm, s.Value.Schema, s.Value.Required); err != nil { + sm, _ := headerRef.Value.SerializationMethod() // ignore error return because a header's serialization method getter does not return an error + + if decodedValue, found, err = decodeValue(dec, headerName, sm, headerRef.Value.Schema, headerRef.Value.Required); err != nil { return &ResponseError{ Input: input, Reason: fmt.Sprintf("unable to decode header %q value", headerName), @@ -194,19 +174,18 @@ func validateResponseHeader( } if found { - // If the value was found but not decoded it means there is no - // schema.Type defining how to interpret the header value - if decodedValue == nil { - // revert to the header's original value - decodedValue = headerVal - } - if err := s.Value.Schema.Value.VisitJSON(decodedValue, opts...); err != nil { + if err = headerRef.Value.Schema.Value.VisitJSON(decodedValue, opts...); err != nil { return &ResponseError{ Input: input, Reason: fmt.Sprintf("response header %q doesn't match schema", headerName), Err: err, } } + } else if headerRef.Value.Required { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("response header %q missing", headerName), + } } return nil } diff --git a/openapi3filter/validate_response_test.go b/openapi3filter/validate_response_test.go new file mode 100644 index 000000000..71e1062b6 --- /dev/null +++ b/openapi3filter/validate_response_test.go @@ -0,0 +1,214 @@ +package openapi3filter + +import ( + "io" + "net/http" + "strings" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/stretchr/testify/require" +) + +func Test_validateResponseHeader(t *testing.T) { + type args struct { + headerName string + headerRef *openapi3.HeaderRef + } + tests := []struct { + name string + args args + isHeaderPresent bool + headerVals []string + wantErr bool + wantErrMsg string + }{ + { + name: "test required string header with single string value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewStringSchema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"blab"}, + wantErr: false, + }, + { + name: "test required string header with single, empty string value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewStringSchema(), true), + }, + isHeaderPresent: true, + headerVals: []string{""}, + wantErr: true, + wantErrMsg: `response header "X-Blab" doesn't match schema: Value is not nullable`, + }, + { + name: "test optional string header with single string value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewStringSchema(), false), + }, + isHeaderPresent: false, + headerVals: []string{"blab"}, + wantErr: false, + }, + { + name: "test required, but missing string header", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewStringSchema(), true), + }, + isHeaderPresent: false, + headerVals: nil, + wantErr: true, + wantErrMsg: `response header "X-Blab" missing`, + }, + { + name: "test integer header with single integer value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewIntegerSchema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"88"}, + wantErr: false, + }, + { + name: "test integer header with single string value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewIntegerSchema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"blab"}, + wantErr: true, + wantErrMsg: `unable to decode header "X-Blab" value: value blab: an invalid integer: invalid syntax`, + }, + { + name: "test int64 header with single int64 value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewInt64Schema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"88"}, + wantErr: false, + }, + { + name: "test int32 header with single int32 value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewInt32Schema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"88"}, + wantErr: false, + }, + { + name: "test float64 header with single float64 value", + args: args{ + headerName: "X-Blab", + headerRef: newHeaderRef(openapi3.NewFloat64Schema(), true), + }, + isHeaderPresent: true, + headerVals: []string{"88.87"}, + wantErr: false, + }, + { + name: "test integer header with multiple csv integer values", + args: args{ + headerName: "X-blab", + headerRef: newHeaderRef(newArraySchema(openapi3.NewIntegerSchema()), true), + }, + isHeaderPresent: true, + headerVals: []string{"87,88"}, + wantErr: false, + }, + { + name: "test integer header with multiple integer values", + args: args{ + headerName: "X-blab", + headerRef: newHeaderRef(newArraySchema(openapi3.NewIntegerSchema()), true), + }, + isHeaderPresent: true, + headerVals: []string{"87", "88"}, + wantErr: false, + }, + { + name: "test non-typed, nullable header with single string value", + args: args{ + headerName: "X-blab", + headerRef: newHeaderRef(&openapi3.Schema{Nullable: true}, true), + }, + isHeaderPresent: true, + headerVals: []string{"blab"}, + wantErr: false, + }, + { + name: "test required non-typed, nullable header not present", + args: args{ + headerName: "X-blab", + headerRef: newHeaderRef(&openapi3.Schema{Nullable: true}, true), + }, + isHeaderPresent: false, + headerVals: []string{"blab"}, + wantErr: true, + wantErrMsg: `response header "X-blab" missing`, + }, + { + name: "test non-typed, non-nullable header with single string value", + args: args{ + headerName: "X-blab", + headerRef: newHeaderRef(&openapi3.Schema{Nullable: false}, true), + }, + isHeaderPresent: true, + headerVals: []string{"blab"}, + wantErr: true, + wantErrMsg: `response header "X-blab" doesn't match schema: Value is not nullable`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + input := newInputDefault() + opts := []openapi3.SchemaValidationOption(nil) + if tt.isHeaderPresent { + input.Header = map[string][]string{http.CanonicalHeaderKey(tt.args.headerName): tt.headerVals} + } + + err := validateResponseHeader(tt.args.headerName, tt.args.headerRef, input, opts) + if tt.wantErr { + require.NotEmpty(t, tt.wantErrMsg, "wanted error message is not populated") + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErrMsg) + } else { + require.NoError(t, err) + } + }) + } +} + +func newInputDefault() *ResponseValidationInput { + return &ResponseValidationInput{ + RequestValidationInput: &RequestValidationInput{ + Request: nil, + PathParams: nil, + Route: nil, + }, + Status: 200, + Header: nil, + Body: io.NopCloser(strings.NewReader(`{}`)), + } +} + +func newHeaderRef(schema *openapi3.Schema, required bool) *openapi3.HeaderRef { + return &openapi3.HeaderRef{Value: &openapi3.Header{Parameter: openapi3.Parameter{Schema: &openapi3.SchemaRef{Value: schema}, Required: required}}} +} + +func newArraySchema(schema *openapi3.Schema) *openapi3.Schema { + arraySchema := openapi3.NewArraySchema() + arraySchema.Items = openapi3.NewSchemaRef("", schema) + + return arraySchema +} \ No newline at end of file From 2fbce808a4998ff00a915ae4f004a2e9e2961fa0 Mon Sep 17 00:00:00 2001 From: Steve Lessard Date: Sat, 17 Dec 2022 09:00:54 -0800 Subject: [PATCH 3/5] #709 - Fix failing unit tests; format source code --- openapi3filter/issue201_test.go | 6 +++++- openapi3filter/validate_response.go | 5 ++--- openapi3filter/validate_response_test.go | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/openapi3filter/issue201_test.go b/openapi3filter/issue201_test.go index 0d363a17e..7e2eaabe1 100644 --- a/openapi3filter/issue201_test.go +++ b/openapi3filter/issue201_test.go @@ -17,7 +17,7 @@ func TestIssue201(t *testing.T) { loader := openapi3.NewLoader() ctx := loader.Context spec := ` -openapi: '3' +openapi: '3.0.3' info: version: 1.0.0 title: Sample API @@ -37,20 +37,24 @@ paths: description: '' required: true schema: + type: string pattern: '^blip$' x-blop: description: '' schema: + type: string pattern: '^blop$' X-Blap: description: '' required: true schema: + type: string pattern: '^blap$' X-Blup: description: '' required: true schema: + type: string pattern: '^blup$' `[1:] diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index 1108031c9..4d09a2966 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -80,8 +80,7 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error sort.Strings(headers) for _, headerName := range headers { headerRef := response.Headers[headerName] - err := validateResponseHeader(headerName, headerRef, input, opts) - if err != nil { + if err := validateResponseHeader(headerName, headerRef, input, opts); err != nil { return err } } @@ -158,7 +157,7 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error return nil } -func validateResponseHeader(headerName string, headerRef *openapi3.HeaderRef, input *ResponseValidationInput, opts []openapi3.SchemaValidationOption, ) error { +func validateResponseHeader(headerName string, headerRef *openapi3.HeaderRef, input *ResponseValidationInput, opts []openapi3.SchemaValidationOption) error { var err error var decodedValue interface{} var found bool diff --git a/openapi3filter/validate_response_test.go b/openapi3filter/validate_response_test.go index 71e1062b6..2d8039593 100644 --- a/openapi3filter/validate_response_test.go +++ b/openapi3filter/validate_response_test.go @@ -211,4 +211,4 @@ func newArraySchema(schema *openapi3.Schema) *openapi3.Schema { arraySchema.Items = openapi3.NewSchemaRef("", schema) return arraySchema -} \ No newline at end of file +} From 60c552e2a22fa0cae87fe254014b40eb437a568d Mon Sep 17 00:00:00 2001 From: Steve Lessard Date: Sat, 17 Dec 2022 09:05:30 -0800 Subject: [PATCH 4/5] #709 - check for error returned from SerializationMethod error --- openapi3filter/validate_response.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index 4d09a2966..c1be31928 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -161,8 +161,16 @@ func validateResponseHeader(headerName string, headerRef *openapi3.HeaderRef, in var err error var decodedValue interface{} var found bool + var sm *openapi3.SerializationMethod dec := &headerParamDecoder{header: input.Header} - sm, _ := headerRef.Value.SerializationMethod() // ignore error return because a header's serialization method getter does not return an error + + if sm, err = headerRef.Value.SerializationMethod(); err != nil { + return &ResponseError{ + Input: input, + Reason: fmt.Sprintf("unable to get header %q serialization method", headerName), + Err: err, + } + } if decodedValue, found, err = decodeValue(dec, headerName, sm, headerRef.Value.Schema, headerRef.Value.Required); err != nil { return &ResponseError{ From 8ab9e5b56103c588b9b3445c0bb841a3dedcff63 Mon Sep 17 00:00:00 2001 From: Steve Lessard Date: Sat, 17 Dec 2022 09:16:17 -0800 Subject: [PATCH 5/5] #709 - format imports --- openapi3filter/validate_response_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openapi3filter/validate_response_test.go b/openapi3filter/validate_response_test.go index 2d8039593..5ce657b0b 100644 --- a/openapi3filter/validate_response_test.go +++ b/openapi3filter/validate_response_test.go @@ -6,8 +6,9 @@ import ( "strings" "testing" - "github.com/getkin/kin-openapi/openapi3" "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" ) func Test_validateResponseHeader(t *testing.T) {