From 56a3f7f6a818e9b4211066983d060cfc9c3fcf8e Mon Sep 17 00:00:00 2001 From: Dave Shanley Date: Mon, 28 Sep 2026 23:04:01 -0400 Subject: [PATCH] fix: resolve open validation issues (#313 #316 #319 #320 #321 #322 #323) - router/paths: strip server base paths when the host has a variable, try every server whose base path prefixes the request, and read path parameters from the path the router matched (fixes a panic with strict server matching and a variable in the base path) (#313) - requests/responses: share one media-range matcher that prefers the most specific key and no longer panics on a Content-Type without a slash (#316) - responses: validate every response header that is present, by the readings its schema allows; match a default response without content; ignore Content-Type header declarations (#321) - parameters: accept whole numbers written as 1.0 or 1e3 for integers, compare integer enums numerically, schema-validate header and cookie numbers, reject NaN/Inf and empty matrix values (both panicked), and compile parameter schemas with their document's OpenAPI version (#319, #320) - parameters: ignore a path template's #fragment when the request has none, as the router does, instead of reporting the parameter missing - helpers: enforce OpenAPI int32/int64 formats with WithFormatAssertions (#320) - schema_validation: never marshal document-owned yaml nodes in place (#322) - tests: stop relying on json.Marshal rejecting map[interface{}]interface{}, which Go 1.27 accepts (#323) Fixes #313, fixes #316, fixes #319, fixes #320, fixes #321, fixes #322, fixes #323 Co-authored-by: adrienyhuel <14916484+adrienyhuel@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- helpers/media_types.go | 62 ++++ helpers/media_types_test.go | 67 ++++ helpers/number_utilities.go | 42 +++ helpers/number_utilities_test.go | 52 +++ helpers/parameter_utilities.go | 10 +- helpers/schema_compiler.go | 49 +++ helpers/schema_compiler_test.go | 49 +++ internal/requeststate/route.go | 5 + internal/requeststate/route_test.go | 10 + internal/serverurl/serverurl.go | 33 ++ internal/serverurl/serverurl_test.go | 32 ++ kin_parity_test.go | 93 ++++++ parameters/cookie_parameters.go | 27 +- parameters/enum_values.go | 27 ++ parameters/header_parameters.go | 27 +- parameters/integer_values_test.go | 318 +++++++++++++++++++ parameters/path_parameters.go | 55 +++- parameters/path_parameters_test.go | 53 ++++ parameters/query_parameters.go | 6 +- parameters/validate_parameter.go | 19 +- parameters/validation_functions.go | 20 +- paths/paths.go | 31 +- paths/paths_test.go | 27 ++ request_defaults.go | 20 +- requests/validate_body.go | 21 +- requests/validate_body_test.go | 51 +++ responses/validate_body.go | 60 ++-- responses/validate_body_test.go | 112 +++++++ responses/validate_headers.go | 102 +++++- responses/validate_headers_test.go | 230 ++++++++++++++ router/router.go | 26 +- router/router_test.go | 83 ++++- router/server.go | 46 +-- schema_validation/directional_schema.go | 4 + schema_validation/directional_schema_test.go | 59 ++++ schema_validation/validate_document.go | 4 +- schema_validation/validate_document_test.go | 50 ++- validator_examples_test.go | 5 +- 38 files changed, 1797 insertions(+), 190 deletions(-) create mode 100644 helpers/media_types.go create mode 100644 helpers/media_types_test.go create mode 100644 helpers/number_utilities.go create mode 100644 helpers/number_utilities_test.go create mode 100644 internal/serverurl/serverurl.go create mode 100644 internal/serverurl/serverurl_test.go create mode 100644 parameters/enum_values.go create mode 100644 parameters/integer_values_test.go diff --git a/helpers/media_types.go b/helpers/media_types.go new file mode 100644 index 0000000..6f5ad4b --- /dev/null +++ b/helpers/media_types.go @@ -0,0 +1,62 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package helpers + +import ( + "strings" + + "github.com/pb33f/libopenapi/orderedmap" + + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" +) + +// FindMediaType returns the content entry that applies to contentType, a Content-Type header value. +// +// Content keys may be media ranges, and OpenAPI applies only the most specific key that matches: +// an exact media type, then a structured syntax range (application/*+json), then a type range +// (application/*), then a subtype range (*/json), then */*. Keys are compared without their +// parameters, and case is ignored. Keys of equal specificity keep document order. +func FindMediaType(content *orderedmap.Map[string, *v3.MediaType], contentType string) (*v3.MediaType, bool) { + if content == nil { + return nil, false + } + mediaType, _, _ := ExtractContentType(contentType) + if found, ok := content.Get(mediaType); ok { + return found, true + } + + typ, subtype, _ := strings.Cut(mediaType, "/") + suffix := "" + if plus := strings.LastIndexByte(subtype, '+'); plus >= 0 { + suffix = subtype[plus:] + } + + var found *v3.MediaType + foundRank := 0 + for pair := content.First(); pair != nil; pair = pair.Next() { + var rank int + switch key := normalizeMediaRange(pair.Key()); key { + case mediaType: + rank = 5 + case typ + "/*" + suffix: + rank = 4 // the same key as the type range below when there is no suffix + case typ + "/*": + rank = 3 + case "*/" + subtype: + rank = 2 + case "*/*": + rank = 1 + } + if rank > foundRank { + found, foundRank = pair.Value(), rank + } + } + return found, foundRank > 0 +} + +// normalizeMediaRange lowercases a media type or range and drops its parameters. +func normalizeMediaRange(value string) string { + base, _, _ := strings.Cut(value, ";") + return strings.ToLower(strings.TrimSpace(base)) +} diff --git a/helpers/media_types_test.go b/helpers/media_types_test.go new file mode 100644 index 0000000..9ecb193 --- /dev/null +++ b/helpers/media_types_test.go @@ -0,0 +1,67 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package helpers + +import ( + "testing" + + "github.com/pb33f/libopenapi/orderedmap" + "github.com/pb33f/testify/assert" + + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" +) + +func mediaTypeContent(keys ...string) *orderedmap.Map[string, *v3.MediaType] { + content := orderedmap.New[string, *v3.MediaType]() + for _, key := range keys { + content.Set(key, &v3.MediaType{}) + } + return content +} + +func TestFindMediaType(t *testing.T) { + all := mediaTypeContent("*/*", "*/json", "application/*", "application/*+json", "application/json") + + for _, test := range []struct { + name string + content *orderedmap.Map[string, *v3.MediaType] + contentType string + expected string + }{ + {"exact beats every range", all, "application/json; charset=utf-8", "application/json"}, + {"structured syntax range beats type range", all, "application/problem+json", "application/*+json"}, + {"type range beats subtype range", all, "application/xml", "application/*"}, + {"subtype range beats */*", all, "text/json", "*/json"}, + {"*/* matches anything", all, "image/png", "*/*"}, + {"type range without suffix range", mediaTypeContent("*/*", "application/*"), "application/problem+json", "application/*"}, + {"keys ignore case and parameters", mediaTypeContent("Text/Plain; charset=utf-8"), "text/plain", "Text/Plain; charset=utf-8"}, + {"content type ignores case", mediaTypeContent("application/json"), "Application/JSON", "application/json"}, + {"equal specificity keeps document order", mediaTypeContent("text/*", "TEXT/*"), "text/csv", "text/*"}, + {"no slash only matches ranges it fits", mediaTypeContent("application/json", "application/*"), "application", "application/*"}, + {"unparseable content type only matches */*", mediaTypeContent("application/json", "*/*"), "application/", "*/*"}, + } { + t.Run(test.name, func(t *testing.T) { + found, ok := FindMediaType(test.content, test.contentType) + assert.True(t, ok) + assert.Same(t, test.content.GetOrZero(test.expected), found) + }) + } + + for _, test := range []struct { + name string + content *orderedmap.Map[string, *v3.MediaType] + contentType string + }{ + {"nil content", nil, "application/json"}, + {"no matching key", mediaTypeContent("application/json", "text/*"), "image/png"}, + {"no slash and no range", mediaTypeContent("application/json"), "application"}, + {"ranges are not matched in reverse", mediaTypeContent("application/json"), "application/*"}, + } { + t.Run(test.name, func(t *testing.T) { + found, ok := FindMediaType(test.content, test.contentType) + assert.False(t, ok) + assert.Nil(t, found) + }) + } +} diff --git a/helpers/number_utilities.go b/helpers/number_utilities.go new file mode 100644 index 0000000..99312c2 --- /dev/null +++ b/helpers/number_utilities.go @@ -0,0 +1,42 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package helpers + +import ( + "errors" + "math" + "strconv" + "strings" +) + +// ParseInteger parses a parameter value as a JSON Schema integer: any number with a zero +// fractional part, so "1.0" and "1e3" are integers as well as "1". Values outside the int64 +// range, and anything that is not a decimal number, return an error. A value written with a +// fraction or exponent is read as a float64, so it is exact up to 2^53. +func ParseInteger(value string) (int64, error) { + parsed, err := strconv.ParseInt(value, 10, 64) + if err == nil || errors.Is(err, strconv.ErrRange) { + return parsed, err + } + f, floatErr := ParseNumber(value) + if floatErr != nil || f != math.Trunc(f) || math.Abs(f) >= math.MaxInt64 { + return 0, err + } + return int64(f), nil +} + +// ParseNumber parses a parameter value as a JSON Schema number, which is always finite. +// strconv.ParseFloat alone also reads NaN, Inf, hex and underscores, none of which are JSON numbers. +func ParseNumber(value string) (float64, error) { + if strings.ContainsFunc(value, notDecimalNumberRune) { + return 0, &strconv.NumError{Func: "ParseFloat", Num: value, Err: strconv.ErrSyntax} + } + // a decimal number too large for a float64 returns an ErrRange error, never an infinity + return strconv.ParseFloat(value, 64) +} + +// notDecimalNumberRune reports whether r cannot appear in a decimal number such as "-1.5e3". +func notDecimalNumberRune(r rune) bool { + return (r < '0' || r > '9') && !strings.ContainsRune("+-.eE", r) +} diff --git a/helpers/number_utilities_test.go b/helpers/number_utilities_test.go new file mode 100644 index 0000000..392436d --- /dev/null +++ b/helpers/number_utilities_test.go @@ -0,0 +1,52 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package helpers + +import ( + "testing" + + "github.com/pb33f/testify/assert" +) + +func TestParseInteger(t *testing.T) { + for value, expected := range map[string]int64{ + "1": 1, + "-7": -7, + "+3": 3, + "007": 7, + "1.0": 1, + "-2.000": -2, + "1e3": 1000, + "2.5E1": 25, + "-0.0": 0, + "9223372036854775807": 9223372036854775807, + } { + parsed, err := ParseInteger(value) + assert.NoError(t, err, value) + assert.Equal(t, expected, parsed, value) + } + + for _, value := range []string{ + "", "abc", "1.5", "1e-3", "0x10", "0x1p3", "1_000", "NaN", "Inf", "-infinity", + "9223372036854775808", "-9223372036854775809", "1e19", "-1e19", "1e400", + } { + _, err := ParseInteger(value) + assert.Error(t, err, value) + } +} + +func TestParseNumber(t *testing.T) { + for value, expected := range map[string]float64{ + "1": 1, "-2.5": -2.5, "+3": 3, "1e3": 1000, "2.5E-1": 0.25, "1e-400": 0, + } { + parsed, err := ParseNumber(value) + assert.NoError(t, err, value) + assert.Equal(t, expected, parsed, value) + } + + for _, value := range []string{"", "abc", "NaN", "Inf", "+Inf", "-infinity", "0x1p3", "1_000", "1e400", "-1e400", "1.2.3"} { + _, err := ParseNumber(value) + assert.Error(t, err, value) + } +} diff --git a/helpers/parameter_utilities.go b/helpers/parameter_utilities.go index 46de99c..9111819 100644 --- a/helpers/parameter_utilities.go +++ b/helpers/parameter_utilities.go @@ -205,14 +205,12 @@ func cast(v string) any { b, _ := strconv.ParseBool(v) return b } - if i, err := strconv.ParseFloat(v, 64); err == nil { - // check if this is an int or not - if !strings.Contains(v, Period) { - iv, _ := strconv.ParseInt(v, 10, 64) - return iv - } + if i, err := strconv.ParseInt(v, 10, 64); err == nil { return i } + if f, err := ParseNumber(v); err == nil { + return f + } return v } diff --git a/helpers/schema_compiler.go b/helpers/schema_compiler.go index 4c52889..c90d951 100644 --- a/helpers/schema_compiler.go +++ b/helpers/schema_compiler.go @@ -3,8 +3,11 @@ package helpers import ( "bytes" "encoding/json" + "errors" "fmt" + "math" "sort" + "strconv" "github.com/santhosh-tekuri/jsonschema/v6" @@ -24,12 +27,16 @@ func ConfigureCompiler(c *jsonschema.Compiler, o *config.ValidationOptions) { if o.FormatAssertions { c.AssertFormat() + for _, format := range openAPIFormats { + c.RegisterFormat(format) + } } if o.ContentAssertions { c.AssertContent() } + // custom formats are registered last, so they replace built-in formats of the same name. for n, v := range o.Formats { c.RegisterFormat(&jsonschema.Format{ Name: n, @@ -38,6 +45,48 @@ func ConfigureCompiler(c *jsonschema.Compiler, o *config.ValidationOptions) { } } +// openAPIFormats are the integer formats in the OpenAPI format registry, which JSON Schema +// does not define. See https://spec.openapis.org/registry/format/ +var openAPIFormats = []*jsonschema.Format{ + {Name: "int32", Validate: integerFormat(math.MinInt32, math.MaxInt32)}, + {Name: "int64", Validate: integerFormat(math.MinInt64, math.MaxInt64)}, +} + +// integerFormat returns a format validator that requires numbers to be whole and within the +// given bounds. Values of other types are left to the rest of the schema. +func integerFormat(minimum, maximum int64) func(any) error { + return func(v any) error { + var valid bool + switch n := v.(type) { + case int64: + valid = n >= minimum && n <= maximum + case float64: + valid = wholeNumberInRange(n, minimum, maximum) + case json.Number: + i, err := n.Int64() + if err == nil { + valid = i >= minimum && i <= maximum + } else if !errors.Is(err, strconv.ErrRange) { + // not an integer literal, but "1.0" and "1e3" are whole numbers + f, floatErr := n.Float64() + valid = floatErr == nil && wholeNumberInRange(f, minimum, maximum) + } + default: + return nil + } + if !valid { + return fmt.Errorf("must be a whole number from %d to %d", minimum, maximum) + } + return nil + } +} + +// wholeNumberInRange reports whether f is a whole number from minimum to maximum. The upper bound +// is compared as maximum+1, exclusive, which stays exact when float64(maximum) rounds up (int64). +func wholeNumberInRange(f float64, minimum, maximum int64) bool { + return f == math.Trunc(f) && f >= float64(minimum) && f < float64(maximum)+1 +} + // NewCompilerWithOptions mints a new JSON schema compiler with custom configuration. func NewCompilerWithOptions(o *config.ValidationOptions) *jsonschema.Compiler { c := jsonschema.NewCompiler() diff --git a/helpers/schema_compiler_test.go b/helpers/schema_compiler_test.go index 021f1cb..8b65845 100644 --- a/helpers/schema_compiler_test.go +++ b/helpers/schema_compiler_test.go @@ -6,6 +6,7 @@ package helpers import ( "encoding/json" "fmt" + "math" "testing" "unicode" @@ -981,3 +982,51 @@ func TestTransformNullableSchema_EnumWithNull(t *testing.T) { } assert.Equal(t, 1, nullCount, "enum should contain exactly one null value") } + +func TestNewCompiledSchema_OpenAPIIntegerFormats(t *testing.T) { + schema := []byte(`{"type": "object", "properties": {"small": {"format": "int32"}, "large": {"format": "int64"}}}`) + + asserting, err := NewCompiledSchema("formats", schema, config.NewValidationOptions(config.WithFormatAssertions())) + require.NoError(t, err) + annotating, err := NewCompiledSchema("formats", schema, config.NewValidationOptions()) + require.NoError(t, err) + + for _, test := range []struct { + property string + value any + valid bool + }{ + {"small", int64(math.MaxInt32), true}, + {"small", int64(math.MaxInt32) + 1, false}, + {"small", float64(math.MinInt32), true}, + {"small", float64(math.MinInt32) - 1, false}, + {"small", 1.5, false}, + {"small", json.Number("7.0"), true}, + {"small", json.Number("2147483648"), false}, + {"small", "not a number", true}, + {"large", int64(math.MaxInt64), true}, + {"large", json.Number("9223372036854775807"), true}, + {"large", json.Number("9223372036854775808"), false}, + {"large", json.Number("9223372036854775808.0"), false}, + {"large", float64(1 << 63), false}, + {"large", float64(math.MinInt64), true}, + {"large", json.Number("1e19"), false}, + {"large", json.Number("not-a-number"), false}, + } { + t.Run(fmt.Sprintf("%s=%v", test.property, test.value), func(t *testing.T) { + instance := map[string]any{test.property: test.value} + assert.Equal(t, test.valid, asserting.Validate(instance) == nil) + assert.NoError(t, annotating.Validate(instance), "formats are annotations unless asserted") + }) + } +} + +func TestNewCompiledSchema_CustomFormatReplacesOpenAPIFormat(t *testing.T) { + schema := []byte(`{"format": "int32"}`) + options := config.NewValidationOptions(config.WithFormatAssertions(), + config.WithCustomFormat("int32", func(any) error { return nil })) + + compiled, err := NewCompiledSchema("custom", schema, options) + require.NoError(t, err) + assert.NoError(t, compiled.Validate(int64(math.MaxInt64))) +} diff --git a/internal/requeststate/route.go b/internal/requeststate/route.go index e46f25f..16e48ef 100644 --- a/internal/requeststate/route.go +++ b/internal/requeststate/route.go @@ -21,6 +21,11 @@ func Route(request *http.Request) *router.Route { return route } +// WithRoute returns a shallow copy of request that carries route. The request itself is not changed. +func WithRoute(request *http.Request, route *router.Route) *http.Request { + return request.WithContext(context.WithValue(request.Context(), routeContextKey{}, route)) +} + // AttachRoute scopes a resolved route to a request and returns an idempotent restoration function. func AttachRoute(request *http.Request, route *router.Route) func() { if request == nil || route == nil { diff --git a/internal/requeststate/route_test.go b/internal/requeststate/route_test.go index 1aebf1d..49965ba 100644 --- a/internal/requeststate/route_test.go +++ b/internal/requeststate/route_test.go @@ -36,3 +36,13 @@ func TestAttachRouteScopesAndRestoresRequestContext(t *testing.T) { AttachRoute(request, nil)() assert.Nil(t, Route(nil)) } + +func TestWithRouteLeavesRequestUnchanged(t *testing.T) { + request, _ := http.NewRequest(http.MethodGet, "http://example.com/items", nil) + route := &router.Route{Path: "/items"} + + routed := WithRoute(request, route) + assert.Same(t, route, Route(routed)) + assert.Nil(t, Route(request)) + assert.Same(t, request.URL, routed.URL) +} diff --git a/internal/serverurl/serverurl.go b/internal/serverurl/serverurl.go new file mode 100644 index 0000000..6d9a20b --- /dev/null +++ b/internal/serverurl/serverurl.go @@ -0,0 +1,33 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +// Package serverurl reads OpenAPI server URLs, which may be templates that url.Parse rejects. +package serverurl + +import ( + "net/url" + "strings" +) + +// BasePath returns the path of an OpenAPI server URL, as url.Parse reports it. +// +// A server variable in the scheme, host, or port (for example "https://{host}/api/v1") makes +// url.Parse fail, so the path is then read from after the host instead. +func BasePath(serverURL string) string { + u, err := url.Parse(serverURL) + if err == nil { + return u.Path + } + // drop the scheme separator, then split at the first slash after the host + _, serverPath, found := strings.Cut(strings.Replace(serverURL, "//", "", 1), "/") + if !found { + return "" + } + if !strings.HasPrefix(serverPath, "/") { + serverPath = "/" + serverPath + } + if u, err = url.Parse(serverPath); err != nil { + return "" + } + return u.Path +} diff --git a/internal/serverurl/serverurl_test.go b/internal/serverurl/serverurl_test.go new file mode 100644 index 0000000..485de59 --- /dev/null +++ b/internal/serverurl/serverurl_test.go @@ -0,0 +1,32 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package serverurl + +import ( + "testing" + + "github.com/pb33f/testify/assert" +) + +func TestBasePath(t *testing.T) { + for serverURL, expected := range map[string]string{ + "https://api.example.com/api/v1": "/api/v1", + "https://api.example.com": "", + "https://api.example.com/": "/", + "/api/v1": "/api/v1", + "": "", + "https://{host}/api/v1": "/api/v1", + "https://{host}": "", + "https://{host}/": "/", + "https://{host}//more//paths": "/more//paths", + "https://api-{env}.example.com/v2": "/v2", + "http://localhost:{port}/v1": "/v1", + "{scheme}://api.example.com/v1": "/v1", + "https://api.example.com/{version}/api": "/{version}/api", + "https://{host}/bad%zzpath": "", + "https://{host}/encoded%20path/resource": "/encoded path/resource", + } { + assert.Equal(t, expected, BasePath(serverURL), serverURL) + } +} diff --git a/kin_parity_test.go b/kin_parity_test.go index 0faed44..597d4cf 100644 --- a/kin_parity_test.go +++ b/kin_parity_test.go @@ -763,6 +763,99 @@ paths: strict.Release() } +func TestRequestDefaultsUseTheMediaTypeValidationUses(t *testing.T) { + spec := `openapi: 3.1.0 +info: {title: ranges, version: 1.0.0} +paths: + /items: + post: + requestBody: + content: + "*/*": + schema: + type: object + properties: + kind: {type: string, default: any} + application/*: + schema: + type: object + properties: + kind: {type: string, enum: [app], default: app} + application/*+json: + schema: + type: object + required: [kind] + properties: + kind: {type: string, enum: [suffix], default: suffix} + responses: {"204": {description: ok}}` + v := parityValidator(t, spec, config.WithRequestDefaults()) + t.Cleanup(v.Release) + + for contentType, kind := range map[string]string{"application/json": "app", "application/problem+json": "suffix"} { + request, _ := http.NewRequest(http.MethodPost, "http://example.com/items", strings.NewReader(`{}`)) + request.Header.Set("Content-Type", contentType) + valid, validationErrors := v.ValidateHttpRequestSync(request) + require.True(t, valid, validationErrors) + body, readErr := io.ReadAll(request.Body) + require.NoError(t, readErr) + assert.JSONEq(t, `{"kind":"`+kind+`"}`, string(body), contentType) + } +} + +func TestHighLevelPathOnlyTemplatedServerHost(t *testing.T) { + // url.Parse rejects a variable in the server host; the base path must still be stripped + spec := `openapi: 3.1.0 +info: {title: templated, version: 1.0.0} +servers: + - url: https://{host}/api/v1 + variables: + host: {default: api.example.com} +paths: + /widgets/{id}: + get: + parameters: + - {name: id, in: path, required: true, schema: {type: integer}} + responses: {"204": {description: ok}}` + v := parityValidator(t, spec) + request, _ := http.NewRequest(http.MethodGet, "https://api.example.com/api/v1/widgets/5", nil) + valid, validationErrors := v.ValidateHttpRequest(request) + require.True(t, valid, validationErrors) + + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/api/v1/widgets/five", nil) + valid, validationErrors = v.ValidateHttpRequest(request) + assert.False(t, valid) + require.Len(t, validationErrors, 1) + assert.Equal(t, "Path parameter 'id' is not a valid integer", validationErrors[0].Message) + v.Release() +} + +func TestHighLevelStrictServerVariableInBasePath(t *testing.T) { + // path parameters must be read from the path the router matched, after the server's variables + spec := `openapi: 3.1.0 +info: {title: strict, version: 1.0.0} +servers: + - url: https://api.example.com/{version}/api + variables: + version: {default: v1} +paths: + /widgets/{id}: + get: + parameters: + - {name: id, in: path, required: true, schema: {type: integer}} + responses: {"204": {description: ok}}` + v := parityValidator(t, spec, config.WithStrictServerMatching()) + request, _ := http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/widgets/5", nil) + valid, validationErrors := v.ValidateHttpRequest(request) + require.True(t, valid, validationErrors) + + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/widgets/five", nil) + valid, validationErrors = v.ValidateHttpRequest(request) + assert.False(t, valid) + require.Len(t, validationErrors, 1) + assert.Equal(t, "Path parameter 'id' is not a valid integer", validationErrors[0].Message) + v.Release() +} + func mustURL(t *testing.T, value string) *url.URL { t.Helper() parsed, err := url.Parse(value) diff --git a/parameters/cookie_parameters.go b/parameters/cookie_parameters.go index add208c..1417d76 100644 --- a/parameters/cookie_parameters.go +++ b/parameters/cookie_parameters.go @@ -6,6 +6,7 @@ package parameters import ( "fmt" "net/http" + "slices" "strconv" "strings" @@ -74,10 +75,15 @@ func (v *paramValidator) ValidateCookieParamsWithPathItem(request *http.Request, pType := sch.Type + // a number is checked against the schema once; a value that may be a string is + // checked as the string it was sent as, below. + checkNumber := !slices.Contains(pType, helpers.String) + for _, ty := range pType { switch ty { case helpers.Integer: - if _, err := strconv.ParseInt(cookie.Value, 10, 64); err != nil { + parsed, err := helpers.ParseInteger(cookie.Value) + if err != nil { validationErrors = append(validationErrors, errors.InvalidCookieParamInteger(p, strings.ToLower(cookie.Value), sch, pathValue, operation, renderedSchema)) break @@ -86,7 +92,7 @@ func (v *paramValidator) ValidateCookieParamsWithPathItem(request *http.Request, if sch.Enum != nil { matchFound := false for _, enumVal := range sch.Enum { - if strings.TrimSpace(cookie.Value) == fmt.Sprint(enumVal.Value) { + if enumValueMatches(cookie.Value, parsed, enumVal.Value) { matchFound = true break } @@ -94,10 +100,18 @@ func (v *paramValidator) ValidateCookieParamsWithPathItem(request *http.Request, if !matchFound { validationErrors = append(validationErrors, errors.IncorrectCookieParamEnum(p, strings.ToLower(cookie.Value), sch, pathValue, operation, renderedSchema)) + break } } + if checkNumber { + checkNumber = false + validationErrors = append(validationErrors, ValidateSingleParameterSchema(sch, parsed, + "Cookie parameter", "The cookie parameter", p.Name, helpers.ParameterValidation, + helpers.ParameterValidationCookie, v.options, pathValue, operation)...) + } case helpers.Number: - if _, err := strconv.ParseFloat(cookie.Value, 64); err != nil { + parsed, err := helpers.ParseNumber(cookie.Value) + if err != nil { validationErrors = append(validationErrors, errors.InvalidCookieParamNumber(p, strings.ToLower(cookie.Value), sch, pathValue, operation, renderedSchema)) break @@ -114,8 +128,15 @@ func (v *paramValidator) ValidateCookieParamsWithPathItem(request *http.Request, if !matchFound { validationErrors = append(validationErrors, errors.IncorrectCookieParamEnum(p, strings.ToLower(cookie.Value), sch, pathValue, operation, renderedSchema)) + break } } + if checkNumber { + checkNumber = false + validationErrors = append(validationErrors, ValidateSingleParameterSchema(sch, parsed, + "Cookie parameter", "The cookie parameter", p.Name, helpers.ParameterValidation, + helpers.ParameterValidationCookie, v.options, pathValue, operation)...) + } case helpers.Boolean: if _, err := strconv.ParseBool(cookie.Value); err != nil { validationErrors = append(validationErrors, diff --git a/parameters/enum_values.go b/parameters/enum_values.go new file mode 100644 index 0000000..915f543 --- /dev/null +++ b/parameters/enum_values.go @@ -0,0 +1,27 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package parameters + +import ( + "fmt" + "strings" + + "github.com/pb33f/libopenapi-validator/helpers" +) + +// enumValueMatches reports whether a parameter value matches an enum value. Values are compared +// as text, and an integer parameter (parsed as an int64) is also compared as a number, so that +// "1.0" and "01" both match an enum value of 1. +func enumValueMatches(value string, parsed any, enumValue any) bool { + enumText := fmt.Sprint(enumValue) + if strings.TrimSpace(value) == enumText { + return true + } + integer, isInteger := parsed.(int64) + if !isInteger { + return false + } + enumInteger, err := helpers.ParseInteger(enumText) + return err == nil && enumInteger == integer +} diff --git a/parameters/header_parameters.go b/parameters/header_parameters.go index 9dea41d..dd45abc 100644 --- a/parameters/header_parameters.go +++ b/parameters/header_parameters.go @@ -6,6 +6,7 @@ package parameters import ( "fmt" "net/http" + "slices" "strconv" "strings" @@ -67,10 +68,15 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, pType := sch.Type + // a number is checked against the schema once; a value that may be a string is + // checked as the string it was sent as, below. + checkNumber := !slices.Contains(pType, helpers.String) + for _, ty := range pType { switch ty { case helpers.Integer: - if _, err := strconv.ParseInt(param, 10, 64); err != nil { + parsed, err := helpers.ParseInteger(param) + if err != nil { validationErrors = append(validationErrors, errors.InvalidHeaderParamInteger(p, strings.ToLower(param), sch, pathValue, operation, renderedSchema)) break @@ -79,7 +85,7 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, if sch.Enum != nil { matchFound := false for _, enumVal := range sch.Enum { - if strings.TrimSpace(param) == fmt.Sprint(enumVal.Value) { + if enumValueMatches(param, parsed, enumVal.Value) { matchFound = true break } @@ -87,11 +93,19 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, if !matchFound { validationErrors = append(validationErrors, errors.IncorrectHeaderParamEnum(p, strings.ToLower(param), sch, pathValue, operation, renderedSchema)) + break } } + if checkNumber { + checkNumber = false + validationErrors = append(validationErrors, ValidateSingleParameterSchema(sch, parsed, + "Header parameter", "The header parameter", p.Name, helpers.ParameterValidation, + helpers.ParameterValidationHeader, v.options, pathValue, operation)...) + } case helpers.Number: - if _, err := strconv.ParseFloat(param, 64); err != nil { + parsed, err := helpers.ParseNumber(param) + if err != nil { validationErrors = append(validationErrors, errors.InvalidHeaderParamNumber(p, strings.ToLower(param), sch, pathValue, operation, renderedSchema)) break @@ -108,8 +122,15 @@ func (v *paramValidator) ValidateHeaderParamsWithPathItem(request *http.Request, if !matchFound { validationErrors = append(validationErrors, errors.IncorrectHeaderParamEnum(p, strings.ToLower(param), sch, pathValue, operation, renderedSchema)) + break } } + if checkNumber { + checkNumber = false + validationErrors = append(validationErrors, ValidateSingleParameterSchema(sch, parsed, + "Header parameter", "The header parameter", p.Name, helpers.ParameterValidation, + helpers.ParameterValidationHeader, v.options, pathValue, operation)...) + } case helpers.Boolean: if _, err := strconv.ParseBool(param); err != nil { diff --git a/parameters/integer_values_test.go b/parameters/integer_values_test.go new file mode 100644 index 0000000..baee51e --- /dev/null +++ b/parameters/integer_values_test.go @@ -0,0 +1,318 @@ +// Copyright 2023-2026 Princess Beef Heavy Industries, LLC / Dave Shanley +// SPDX-License-Identifier: MIT + +package parameters + +import ( + "net/http" + "sort" + "testing" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" + + "github.com/pb33f/libopenapi-validator/config" + "github.com/pb33f/libopenapi-validator/errors" + "github.com/pb33f/libopenapi-validator/internal/requeststate" + "github.com/pb33f/libopenapi-validator/router" +) + +// JSON Schema defines an integer as any number with a zero fractional part, so "1.0" is an integer. +func TestIntegerParameters_AcceptZeroFraction(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Integers + version: 1.0.0 +paths: + /things/{id}/{.label}/{;matrix}/{ids}: + get: + parameters: + - {name: id, in: path, required: true, schema: {type: integer, enum: [1, 2]}} + - {name: label, in: path, required: true, style: label, schema: {type: integer}} + - {name: matrix, in: path, required: true, style: matrix, schema: {type: integer}} + - {name: ids, in: path, required: true, schema: {type: array, items: {type: integer}}} + - {name: limit, in: query, schema: {type: integer, maximum: 10}} + - {name: state, in: query, schema: {type: integer, enum: [1, 2]}} + - {name: pages, in: query, explode: false, schema: {type: array, items: {type: integer, enum: [1, 2]}}} + - {name: X-Count, in: header, schema: {type: integer, enum: [1, 2]}} + - {name: session, in: cookie, schema: {type: integer, enum: [1, 2]}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + v := NewParameterValidator(&m.Model) + + // failing returns the sorted names of the parameters that fail validation + failing := func(url, header, cookie string) []string { + request, _ := http.NewRequest(http.MethodGet, url, nil) + request.Header.Set("X-Count", header) + request.AddCookie(&http.Cookie{Name: "session", Value: cookie}) + + var names []string + for _, validate := range []func(*http.Request) (bool, []*errors.ValidationError){ + v.ValidatePathParams, v.ValidateQueryParams, v.ValidateHeaderParams, v.ValidateCookieParams, + } { + _, validationErrors := validate(request) + for _, validationError := range validationErrors { + names = append(names, validationError.ParameterName) + } + } + sort.Strings(names) + return names + } + + assert.Empty(t, failing( + "https://things.com/things/1.0/.2.0/;matrix=3.0/1.0,2e1?limit=5.0&state=2.0&pages=1.0,2", "1.0", "2.0")) + + assert.Equal(t, []string{"X-Count", "id", "ids", "label", "limit", "matrix", "pages", "session", "state"}, failing( + "https://things.com/things/1.5/.2.5/;matrix=3.5/1.5?limit=5.5&state=2.5&pages=1.5", "1.5", "2.5")) + + // a whole number still has to satisfy the rest of its schema, including enums + assert.Equal(t, []string{"X-Count", "id", "limit", "pages", "session", "state"}, failing( + "https://things.com/things/3.0/.1/;matrix=1/1?limit=11.0&state=3.0&pages=3.0", "3.0", "3.0")) +} + +// The OpenAPI integer formats are enforced once format assertions are enabled. +func TestIntegerParameters_OpenAPIFormats(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Formats + version: 1.0.0 +paths: + /things: + get: + parameters: + - {name: offset, in: query, schema: {type: integer, format: int32, minimum: 0}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + + asserting := NewParameterValidator(&m.Model, config.WithFormatAssertions()) + annotating := NewParameterValidator(&m.Model) + + for value, valid := range map[string]bool{"2147483647": true, "2147483648": false, "2147483647.0": true} { + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things?offset="+value, nil) + ok, _ := asserting.ValidateQueryParams(request) + assert.Equal(t, valid, ok, value) + + ok, _ = annotating.ValidateQueryParams(request) + assert.True(t, ok, value) + } +} + +// Integer and number headers and cookies are validated against their whole schema, not just their type. +func TestHeaderAndCookieNumbers_ValidateSchema(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Numbers + version: 1.0.0 +paths: + /things: + get: + parameters: + - {name: X-Int, in: header, schema: {type: integer, maximum: 10}} + - {name: X-Num, in: header, schema: {type: number, maximum: 10, enum: [2.5, 20]}} + - {name: X-Id, in: header, schema: {type: integer, format: int32}} + - {name: int, in: cookie, schema: {type: integer, maximum: 10}} + - {name: num, in: cookie, schema: {type: number, maximum: 10, enum: [2.5, 20]}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + v := NewParameterValidator(&m.Model, config.WithFormatAssertions()) + + for _, test := range []struct { + header, cookie, value string + valid bool + }{ + {header: "X-Int", value: "10", valid: true}, + {header: "X-Int", value: "11", valid: false}, + {header: "X-Num", value: "2.5", valid: true}, + {header: "X-Num", value: "3", valid: false}, // not in the enum + {header: "X-Num", value: "20", valid: false}, // in the enum, over the maximum + {header: "X-Id", value: "2147483647", valid: true}, + {header: "X-Id", value: "2147483648", valid: false}, + {cookie: "int", value: "10", valid: true}, + {cookie: "int", value: "11", valid: false}, + {cookie: "num", value: "2.5", valid: true}, + {cookie: "num", value: "3", valid: false}, + {cookie: "num", value: "20", valid: false}, + } { + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + var valid bool + var validationErrors []*errors.ValidationError + if test.header != "" { + request.Header.Set(test.header, test.value) + valid, validationErrors = v.ValidateHeaderParams(request) + } else { + request.AddCookie(&http.Cookie{Name: test.cookie, Value: test.value}) + valid, validationErrors = v.ValidateCookieParams(request) + } + assert.Equal(t, test.valid, valid, "%s%s=%s", test.header, test.cookie, test.value) + if !test.valid { + assert.Len(t, validationErrors, 1, "%s%s=%s", test.header, test.cookie, test.value) + } + } +} + +// Path parameters are read from the path the router matched, after a server variable in the base path. +func TestPathParameters_UseRouterMatch(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Router + version: 1.0.0 +servers: + - url: https://api.example.com/{version}/api + variables: + version: {default: v1} +paths: + /widgets/{id}: + get: + parameters: + - {name: id, in: path, required: true, schema: {type: integer}} + responses: + '200': + description: ok + /{kind}/{id}: + get: + parameters: + - {name: kind, in: path, required: true, schema: {type: string, enum: [widgets]}} + - {name: id, in: path, required: true, schema: {type: integer}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + + options := config.NewValidationOptions() + options.Router = router.NewRouter(&m.Model) // strict server matching + v := NewParameterValidator(&m.Model, config.WithExistingOpts(options)) + + request, _ := http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/widgets/5", nil) + valid, validationErrors := v.ValidatePathParams(request) + assert.True(t, valid, validationErrors) + assert.Nil(t, requeststate.Route(request), "the caller's request is not changed") + + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/widgets/five", nil) + valid, validationErrors = v.ValidatePathParams(request) + assert.False(t, valid) + require.Len(t, validationErrors, 1) + assert.Equal(t, "Path parameter 'id' is not a valid integer", validationErrors[0].Message) + + // a path item the router did not choose is validated against the path as sent + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/widgets/5", nil) + valid, validationErrors = v.ValidatePathParamsWithPathItem(request, + m.Model.Paths.PathItems.GetOrZero("/{kind}/{id}"), "/{kind}/{id}") + assert.True(t, valid, validationErrors) +} + +// NaN and Inf are not JSON numbers, and schema validation cannot compare them, so they are rejected. +func TestNumberParameters_RejectNonFiniteValues(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Numbers + version: 1.0.0 +paths: + /things/{simple}/{.label}/{;matrix}: + get: + parameters: + - {name: simple, in: path, required: true, schema: {type: number, maximum: 10}} + - {name: label, in: path, required: true, style: label, schema: {type: number, maximum: 10}} + - {name: matrix, in: path, required: true, style: matrix, schema: {type: number, maximum: 10}} + - {name: q, in: query, schema: {type: number, maximum: 10}} + - {name: X-Num, in: header, schema: {type: number, maximum: 10}} + - {name: num, in: cookie, schema: {type: number, maximum: 10}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + v := NewParameterValidator(&m.Model) + + for _, value := range []string{"NaN", "Inf", "-Inf"} { + request, _ := http.NewRequest(http.MethodGet, + "https://things.com/things/"+value+"/."+value+"/;matrix="+value+"?q="+value, nil) + request.Header.Set("X-Num", value) + request.AddCookie(&http.Cookie{Name: "num", Value: value}) + + for name, validate := range map[string]func(*http.Request) (bool, []*errors.ValidationError){ + "path": v.ValidatePathParams, "query": v.ValidateQueryParams, "header": v.ValidateHeaderParams, "cookie": v.ValidateCookieParams, + } { + var valid bool + require.NotPanics(t, func() { valid, _ = validate(request) }, "%s=%s", name, value) + assert.False(t, valid, "%s=%s", name, value) + } + } + + // an empty matrix value is reported with the value that was sent, not a panic + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things/1/.1/;matrix=", nil) + var validationErrors []*errors.ValidationError + require.NotPanics(t, func() { _, validationErrors = v.ValidatePathParams(request) }) + require.NotEmpty(t, validationErrors) +} + +// A value is checked against a multi-type schema once, and integer enums compare as numbers. +func TestHeaderParameters_MultiTypeAndIntegerEnums(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + parameters: + - {name: X-Either, in: header, schema: {type: [integer, number], maximum: 10}} + - {name: X-Enum, in: header, schema: {type: integer, enum: [1, 2]}} + - {name: X-Padded, in: header, schema: {type: [integer, string], enum: ["007"]}} + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + v := NewParameterValidator(&m.Model) + + validate := func(header, value string) (bool, []*errors.ValidationError) { + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + request.Header.Set(header, value) + return v.ValidateHeaderParams(request) + } + + valid, validationErrors := validate("X-Either", "11") + assert.False(t, valid) + assert.Len(t, validationErrors, 1) + + for _, value := range []string{"1", "01", "1.0", "+2"} { + valid, validationErrors = validate("X-Enum", value) + assert.True(t, valid, "%s: %v", value, validationErrors) + } + valid, validationErrors = validate("X-Enum", "3.0") + assert.False(t, valid) + require.Len(t, validationErrors, 1) + assert.Contains(t, validationErrors[0].Reason, "'3.0'", "errors report the value that was sent") + + valid, validationErrors = validate("X-Padded", "007") + assert.True(t, valid, validationErrors) +} diff --git a/parameters/path_parameters.go b/parameters/path_parameters.go index 2a1a0f0..d676a10 100644 --- a/parameters/path_parameters.go +++ b/parameters/path_parameters.go @@ -17,6 +17,7 @@ import ( "github.com/pb33f/libopenapi-validator/errors" "github.com/pb33f/libopenapi-validator/helpers" + "github.com/pb33f/libopenapi-validator/internal/requeststate" "github.com/pb33f/libopenapi-validator/paths" ) @@ -29,6 +30,7 @@ func (v *paramValidator) ValidatePathParams(request *http.Request) (bool, []*err } func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, pathItem *v3.PathItem, pathValue string) (bool, []*errors.ValidationError) { + request = v.withRoute(request, pathItem) if pathItem == nil { return false, []*errors.ValidationError{{ ValidationType: helpers.PathValidation, @@ -45,8 +47,14 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p // split the path into segments, dropping empty segments so that a request // path containing a double slash (e.g. //test/path) does not shift the // index alignment between submitted and template segments. - submittedSegments := nonEmptyPathSegments(paths.StripRequestPath(request, v.document)) - pathSegments := nonEmptyPathSegments(pathValue) + submittedPath := paths.StripRequestPath(request, v.document) + templatePath := pathValue + if !strings.Contains(submittedPath, "#") { + // like the router, ignore a template's fragment when the request has none; servers never receive one. + templatePath, _, _ = strings.Cut(pathValue, "#") + } + submittedSegments := nonEmptyPathSegments(submittedPath) + pathSegments := nonEmptyPathSegments(templatePath) // get the operation method for error reporting operation := strings.ToLower(request.Method) @@ -166,10 +174,10 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p renderedSchema := GetRenderedSchema(sch, v.options) // check enum (if present) - enumCheck := func(decodedValue string) { + enumCheck := func(decodedValue string, parsedValue any) { matchFound := false for _, enumVal := range sch.Enum { - if strings.TrimSpace(decodedValue) == fmt.Sprint(enumVal.Value) { + if enumValueMatches(decodedValue, parsedValue, enumVal.Value) { matchFound = true break } @@ -190,7 +198,7 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p // check if the param is within the enum if sch.Enum != nil { - enumCheck(decodedParamValue) + enumCheck(decodedParamValue, nil) break } validationErrors = append(validationErrors, @@ -216,7 +224,7 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p } // check if the param is within the enum if sch.Enum != nil { - enumCheck(rawParamValue) + enumCheck(rawParamValue, paramValueParsed) break } validationErrors = append(validationErrors, ValidateSingleParameterSchema( @@ -241,7 +249,7 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p } // check if the param is within the enum if sch.Enum != nil { - enumCheck(rawParamValue) + enumCheck(rawParamValue, paramValueParsed) break } validationErrors = append(validationErrors, ValidateSingleParameterSchema( @@ -352,14 +360,14 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p switch iSch.Type[n] { case helpers.Integer: for pv := range arrayValues { - if _, err := strconv.ParseInt(arrayValues[pv], 10, 64); err != nil { + if _, err := helpers.ParseInteger(arrayValues[pv]); err != nil { validationErrors = append(validationErrors, errors.IncorrectPathParamArrayInteger(p, arrayValues[pv], sch, iSch, pathValue, renderedItemsSchema)) } } case helpers.Number: for pv := range arrayValues { - if _, err := strconv.ParseFloat(arrayValues[pv], 64); err != nil { + if _, err := helpers.ParseNumber(arrayValues[pv]); err != nil { validationErrors = append(validationErrors, errors.IncorrectPathParamArrayNumber(p, arrayValues[pv], sch, iSch, pathValue, renderedItemsSchema)) } @@ -448,7 +456,7 @@ func nonEmptyPathSegments(path string) []string { func (v *paramValidator) resolveNumber(sch *base.Schema, p *v3.Parameter, isLabel bool, isMatrix bool, paramValue string, pathValue string, renderedSchema string) (string, float64, []*errors.ValidationError) { if isLabel && p.Style == helpers.LabelStyle { - paramValueParsed, err := strconv.ParseFloat(paramValue[1:], 64) + paramValueParsed, err := helpers.ParseNumber(paramValue[1:]) if err != nil { return "", 0, []*errors.ValidationError{errors.IncorrectPathParamNumber(p, paramValue[1:], sch, pathValue, renderedSchema)} } @@ -457,13 +465,13 @@ func (v *paramValidator) resolveNumber(sch *base.Schema, p *v3.Parameter, isLabe if isMatrix && p.Style == helpers.MatrixStyle { // strip off the colon and the parameter name paramValue = strings.Replace(paramValue[1:], fmt.Sprintf("%s=", p.Name), "", 1) - paramValueParsed, err := strconv.ParseFloat(paramValue, 64) + paramValueParsed, err := helpers.ParseNumber(paramValue) if err != nil { - return "", 0, []*errors.ValidationError{errors.IncorrectPathParamNumber(p, paramValue[1:], sch, pathValue, renderedSchema)} + return "", 0, []*errors.ValidationError{errors.IncorrectPathParamNumber(p, paramValue, sch, pathValue, renderedSchema)} } return paramValue, paramValueParsed, nil } - paramValueParsed, err := strconv.ParseFloat(paramValue, 64) + paramValueParsed, err := helpers.ParseNumber(paramValue) if err != nil { return "", 0, []*errors.ValidationError{errors.IncorrectPathParamNumber(p, paramValue, sch, pathValue, renderedSchema)} } @@ -472,7 +480,7 @@ func (v *paramValidator) resolveNumber(sch *base.Schema, p *v3.Parameter, isLabe func (v *paramValidator) resolveInteger(sch *base.Schema, p *v3.Parameter, isLabel bool, isMatrix bool, paramValue string, pathValue string, renderedSchema string) (string, int64, []*errors.ValidationError) { if isLabel && p.Style == helpers.LabelStyle { - paramValueParsed, err := strconv.ParseInt(paramValue[1:], 10, 64) + paramValueParsed, err := helpers.ParseInteger(paramValue[1:]) if err != nil { return "", 0, []*errors.ValidationError{errors.IncorrectPathParamInteger(p, paramValue[1:], sch, pathValue, renderedSchema)} } @@ -481,15 +489,28 @@ func (v *paramValidator) resolveInteger(sch *base.Schema, p *v3.Parameter, isLab if isMatrix && p.Style == helpers.MatrixStyle { // strip off the colon and the parameter name paramValue = strings.Replace(paramValue[1:], fmt.Sprintf("%s=", p.Name), "", 1) - paramValueParsed, err := strconv.ParseInt(paramValue, 10, 64) + paramValueParsed, err := helpers.ParseInteger(paramValue) if err != nil { - return "", 0, []*errors.ValidationError{errors.IncorrectPathParamInteger(p, paramValue[1:], sch, pathValue, renderedSchema)} + return "", 0, []*errors.ValidationError{errors.IncorrectPathParamInteger(p, paramValue, sch, pathValue, renderedSchema)} } return paramValue, paramValueParsed, nil } - paramValueParsed, err := strconv.ParseInt(paramValue, 10, 64) + paramValueParsed, err := helpers.ParseInteger(paramValue) if err != nil { return "", 0, []*errors.ValidationError{errors.IncorrectPathParamInteger(p, paramValue, sch, pathValue, renderedSchema)} } return paramValue, paramValueParsed, nil } + +// withRoute returns request carrying the route the router matched for it, unless one is attached +// already, so path parameters are read from the path the router matched after the server base path +// (and any server variables in it) was removed. The caller's request is not changed. +func (v *paramValidator) withRoute(request *http.Request, pathItem *v3.PathItem) *http.Request { + if pathItem == nil || v.options == nil || v.options.Router == nil || requeststate.Route(request) != nil { + return request + } + if route, err := v.options.Router.FindRoute(request); err == nil && route.PathItem == pathItem { + return requeststate.WithRoute(request, route) + } + return request +} diff --git a/parameters/path_parameters_test.go b/parameters/path_parameters_test.go index 9ae7bfe..0d3ac51 100644 --- a/parameters/path_parameters_test.go +++ b/parameters/path_parameters_test.go @@ -17,6 +17,7 @@ import ( "github.com/pb33f/libopenapi-validator/helpers" "github.com/pb33f/libopenapi-validator/paths" "github.com/pb33f/libopenapi-validator/radix" + "github.com/pb33f/libopenapi-validator/router" ) func TestNewValidator_SimpleArrayEncodedPath(t *testing.T) { @@ -2475,3 +2476,55 @@ func TestSegmentReferencesParam(t *testing.T) { assert.False(t, segmentReferencesParam("static", "id")) assert.False(t, segmentReferencesParam("{unbalanced", "unbalanced")) } + +func TestValidatePathParams_TemplateWithFragment(t *testing.T) { + spec := `openapi: 3.1.0 +servers: + - url: https://api.example.com +paths: + /pages/{id}#section: + get: + parameters: + - name: id + in: path + required: true + schema: + type: integer + responses: + '200': + description: ok` + + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + + strict := config.NewValidationOptions() + strict.Router = router.NewRouter(&m.Model) + + for _, tc := range []struct { + mode string + v ParameterValidator + }{ + {"path-only", NewParameterValidator(&m.Model)}, + {"strict", NewParameterValidator(&m.Model, config.WithExistingOpts(strict))}, + } { + // servers never receive the fragment, so the template's fragment is ignored + request, _ := http.NewRequest(http.MethodGet, "https://api.example.com/pages/5", nil) + valid, validationErrors := tc.v.ValidatePathParams(request) + assert.True(t, valid, tc.mode) + assert.Empty(t, validationErrors, tc.mode) + + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/pages/five", nil) + valid, validationErrors = tc.v.ValidatePathParams(request) + assert.False(t, valid, tc.mode) + require.Len(t, validationErrors, 1, tc.mode) + assert.Equal(t, "Path parameter 'id' is not a valid integer", validationErrors[0].Message, tc.mode) + + // a request that carries the fragment is still matched against the whole template + request, _ = http.NewRequest(http.MethodGet, "https://api.example.com/pages/5#section", nil) + valid, validationErrors = tc.v.ValidatePathParams(request) + assert.True(t, valid, tc.mode) + assert.Empty(t, validationErrors, tc.mode) + } +} diff --git a/parameters/query_parameters.go b/parameters/query_parameters.go index 974ad82..a77bdd9 100644 --- a/parameters/query_parameters.go +++ b/parameters/query_parameters.go @@ -155,7 +155,7 @@ doneLooking: case helpers.String: validationErrors = append(validationErrors, v.validateSimpleParam(sch, ef, ef, params[p], pathValue, operation, renderedSchema)...) case helpers.Integer: - efF, err := strconv.ParseInt(ef, 10, 64) + efF, err := helpers.ParseInteger(ef) if err != nil { validationErrors = append(validationErrors, errors.InvalidQueryParamInteger(params[p], ef, sch, pathValue, operation, renderedSchema)) @@ -163,7 +163,7 @@ doneLooking: } validationErrors = append(validationErrors, v.validateSimpleParam(sch, ef, efF, params[p], pathValue, operation, renderedSchema)...) case helpers.Number: - efF, err := strconv.ParseFloat(ef, 64) + efF, err := helpers.ParseNumber(ef) if err != nil { validationErrors = append(validationErrors, errors.InvalidQueryParamNumber(params[p], ef, sch, pathValue, operation, renderedSchema)) @@ -309,7 +309,7 @@ func (v *paramValidator) validateSimpleParam(sch *base.Schema, rawParam string, if sch.Enum != nil { matchFound := false for _, enumVal := range sch.Enum { - if strings.TrimSpace(rawParam) == fmt.Sprint(enumVal.Value) { + if enumValueMatches(rawParam, parsedParam, enumVal.Value) { matchFound = true break } diff --git a/parameters/validate_parameter.go b/parameters/validate_parameter.go index 816f71a..c4e7799 100644 --- a/parameters/validate_parameter.go +++ b/parameters/validate_parameter.go @@ -23,7 +23,17 @@ import ( "github.com/pb33f/libopenapi-validator/schema_validation" ) -const parameterSchemaVersion = 3.1 +// parameterSchemaVersion returns the OpenAPI version of the document a parameter or header +// schema belongs to, so it compiles with the keywords it was written with (3.0's nullable, for +// example). It is 3.1 when no version can be found. +func parameterSchemaVersion(schema *base.Schema) float32 { + if schema != nil && schema.GoLow() != nil { + if version, ok := schema.GoLow().GetIndex().ResolveDocumentVersion(); ok { + return version + } + } + return 3.1 +} func ValidateSingleParameterSchema( schema *base.Schema, @@ -211,10 +221,11 @@ func compileParameterValidationSchema( validationType string, subValType string, ) (*jsonschema.Schema, string, []*errors.ValidationError) { + version := parameterSchemaVersion(schema) if validationOptions != nil && validationOptions.SchemaCache != nil && schema != nil && schema.GoLow() != nil { hash := schema_validation.SchemaCacheKey( schema.GoLow().Hash(), - parameterSchemaVersion, + version, schema_validation.SchemaValidationPurposeGeneric, ) if cached, ok := validationOptions.SchemaCache.Load(hash); ok && cached != nil && cached.CompiledSchema != nil { @@ -226,7 +237,7 @@ func compileParameterValidationSchema( schema, schema_validation.SchemaValidationPurposeGeneric, validationOptions, - parameterSchemaVersion, + version, ) if err != nil { return nil, "", []*errors.ValidationError{ @@ -240,7 +251,7 @@ func compileParameterValidationSchema( if validationOptions != nil && validationOptions.SchemaCache != nil && schema != nil && schema.GoLow() != nil { hash := schema_validation.SchemaCacheKey( schema.GoLow().Hash(), - parameterSchemaVersion, + version, schema_validation.SchemaValidationPurposeGeneric, ) validationOptions.SchemaCache.Store(hash, compiled.ToCacheEntry(schema)) diff --git a/parameters/validation_functions.go b/parameters/validation_functions.go index 91f7e91..47652f1 100644 --- a/parameters/validation_functions.go +++ b/parameters/validation_functions.go @@ -5,7 +5,6 @@ package parameters import ( "encoding/json" - "fmt" "strconv" "strings" @@ -40,7 +39,7 @@ func ValidateCookieArray( for _, itemType := range itemsSchema.Type { switch itemType { case helpers.Integer, helpers.Number: - if _, err := strconv.ParseFloat(item, 64); err != nil { + if _, err := helpers.ParseNumber(item); err != nil { validationErrors = append(validationErrors, errors.IncorrectCookieParamArrayNumber(param, item, sch, itemsSchema, pathTemplate, operation, renderedItemsSchema)) } @@ -87,7 +86,7 @@ func ValidateHeaderArray( for _, itemType := range itemsSchema.Type { switch itemType { case helpers.Integer, helpers.Number: - if _, err := strconv.ParseFloat(item, 64); err != nil { + if _, err := helpers.ParseNumber(item); err != nil { validationErrors = append(validationErrors, errors.IncorrectHeaderParamArrayNumber(param, item, sch, itemsSchema, pathTemplate, operation, renderedItemsSchema)) } @@ -144,14 +143,14 @@ func ValidateQueryArray( } // check if the param is within an enum - checkEnum := func(item string) { + checkEnum := func(item string, parsed any) { // check if the array param is within an enum if sch.Items.IsA() { itemsSch := sch.Items.A.Schema() if itemsSch.Enum != nil { matchFound := false for _, enumVal := range itemsSch.Enum { - if strings.TrimSpace(item) == fmt.Sprint(enumVal.Value) { + if enumValueMatches(item, parsed, enumVal.Value) { matchFound = true break } @@ -180,21 +179,22 @@ func ValidateQueryArray( for _, itemType := range itemsSchema.Type { switch itemType { case helpers.Integer: - if _, err := strconv.ParseInt(item, 10, 64); err != nil { + parsed, err := helpers.ParseInteger(item) + if err != nil { validationErrors = append(validationErrors, errors.IncorrectQueryParamArrayInteger(param, item, sch, itemsSchema, pathTemplate, operation, renderedItemsSchema)) break } // will it blend? - checkEnum(item) + checkEnum(item, parsed) case helpers.Number: - if _, err := strconv.ParseFloat(item, 64); err != nil { + if _, err := helpers.ParseNumber(item); err != nil { validationErrors = append(validationErrors, errors.IncorrectQueryParamArrayNumber(param, item, sch, itemsSchema, pathTemplate, operation, renderedItemsSchema)) break } // will it blend? - checkEnum(item) + checkEnum(item, nil) case helpers.Boolean: if _, err := strconv.ParseBool(item); err != nil { @@ -215,7 +215,7 @@ func ValidateQueryArray( case helpers.String: // will it float? - checkEnum(item) + checkEnum(item, nil) } } } diff --git a/paths/paths.go b/paths/paths.go index e15ae59..ad9d8a1 100644 --- a/paths/paths.go +++ b/paths/paths.go @@ -7,7 +7,6 @@ import ( stderrors "errors" "fmt" "net/http" - "net/url" "path/filepath" "regexp" "strings" @@ -18,6 +17,8 @@ import ( "github.com/pb33f/libopenapi-validator/config" "github.com/pb33f/libopenapi-validator/errors" "github.com/pb33f/libopenapi-validator/helpers" + "github.com/pb33f/libopenapi-validator/internal/requeststate" + "github.com/pb33f/libopenapi-validator/internal/serverurl" "github.com/pb33f/libopenapi-validator/router" ) @@ -176,29 +177,25 @@ func getBasePaths(document *v3.Document) []string { // extract base path from document to check against paths. var basePaths []string for _, s := range document.Servers { - u, err := url.Parse(s.URL) - // if the host contains special characters, we should attempt to split and parse only the relative path - if err != nil { - // split at first occurrence - _, serverPath, _ := strings.Cut(strings.Replace(s.URL, "//", "", 1), "/") - - if !strings.HasPrefix(serverPath, "/") { - serverPath = "/" + serverPath - } - - u, _ = url.Parse(serverPath) - } - - if u != nil && u.Path != "" { - basePaths = append(basePaths, u.Path) + if basePath := serverurl.BasePath(s.URL); basePath != "" { + basePaths = append(basePaths, basePath) } } return basePaths } -// StripRequestPath strips the base path from the request path, based on the server paths provided in the specification +// StripRequestPath strips the base path from the request path, based on the server paths provided in the specification. +// During high-level validation it returns the path the router matched, which accounts for server variables. func StripRequestPath(request *http.Request, document *v3.Document) string { + if route := requeststate.Route(request); route != nil && route.Document == document && route.RequestPath != "" { + // strict server matching ignores the fragment, path-only matching keeps it; always keep it here + stripped, _, _ := strings.Cut(route.RequestPath, "#") + if request.URL.Fragment != "" { + stripped += "#" + request.URL.Fragment + } + return stripped + } basePaths := getBasePaths(document) // strip any base path diff --git a/paths/paths_test.go b/paths/paths_test.go index e46ea49..2bd6deb 100644 --- a/paths/paths_test.go +++ b/paths/paths_test.go @@ -13,8 +13,12 @@ import ( "github.com/pb33f/libopenapi" "github.com/pb33f/libopenapi-validator/config" + "github.com/pb33f/libopenapi-validator/internal/requeststate" "github.com/pb33f/libopenapi-validator/radix" + "github.com/pb33f/libopenapi-validator/router" "github.com/pb33f/testify/assert" + + v3 "github.com/pb33f/libopenapi/datamodel/high/v3" ) func TestNewValidator_BadParam(t *testing.T) { @@ -771,6 +775,29 @@ paths: assert.Equal(t, expectedPaths, basePaths) } +func TestStripRequestPath_UsesAttachedRoute(t *testing.T) { + doc := &v3.Document{Servers: []*v3.Server{{URL: "https://api.example.com/{version}/api"}}} + request, _ := http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/widgets/5", nil) + + // a variable in the server path cannot be stripped without the router's match + assert.Equal(t, "/v2/api/widgets/5", StripRequestPath(request, doc)) + + restore := requeststate.AttachRoute(request, &router.Route{Document: doc, RequestPath: "/widgets/5"}) + defer restore() + assert.Equal(t, "/widgets/5", StripRequestPath(request, doc)) + + // a route matched against another document is ignored + assert.Equal(t, "/v2/api/widgets/5", StripRequestPath(request, &v3.Document{})) + + // the fragment is kept, whether or not the router matched with it + fragment, _ := http.NewRequest(http.MethodGet, "https://api.example.com/v2/api/pages/5#section", nil) + for _, requestPath := range []string{"/pages/5", "/pages/5#section"} { + restoreFragment := requeststate.AttachRoute(fragment, &router.Route{Document: doc, RequestPath: requestPath}) + assert.Equal(t, "/pages/5#section", StripRequestPath(fragment, doc)) + restoreFragment() + } +} + func TestNewValidator_FindPathWithEncodedArg(t *testing.T) { spec := `openapi: 3.1.0 paths: diff --git a/request_defaults.go b/request_defaults.go index 6d72679..848419a 100644 --- a/request_defaults.go +++ b/request_defaults.go @@ -254,24 +254,14 @@ func serializeDefault(value any, separator string) string { return fmt.Sprint(value) } +// requestMediaType returns the request body media type that applies to mediaType, the same one +// request body validation uses. func requestMediaType(operation *v3.Operation, mediaType string) *v3.MediaType { - if operation == nil || operation.RequestBody == nil || operation.RequestBody.Content == nil { + if operation == nil || operation.RequestBody == nil { return nil } - if exact := operation.RequestBody.Content.GetOrZero(mediaType); exact != nil { - return exact - } - parts := strings.SplitN(mediaType, "/", 2) - if len(parts) != 2 { - return nil - } - for pair := operation.RequestBody.Content.First(); pair != nil; pair = pair.Next() { - declared := strings.SplitN(strings.ToLower(pair.Key()), "/", 2) - if len(declared) == 2 && (declared[0] == "*" || declared[0] == parts[0]) && (declared[1] == "*" || declared[1] == parts[1]) { - return pair.Value() - } - } - return nil + found, _ := helpers.FindMediaType(operation.RequestBody.Content, mediaType) + return found } func applySchemaDefaults(value any, schema *base.Schema) (bool, error) { diff --git a/requests/validate_body.go b/requests/validate_body.go index 22010ab..5a7aa03 100644 --- a/requests/validate_body.go +++ b/requests/validate_body.go @@ -83,7 +83,7 @@ func (v *requestBodyValidator) ValidateRequestBodyWithPathItem(request *http.Req } // extract the media type from the content type header. - mediaType, ok := v.extractContentType(contentType, operation) + mediaType, ok := helpers.FindMediaType(operation.RequestBody.Content, contentType) if !ok { return false, []*errors.ValidationError{errors.RequestContentTypeNotFound(operation, request, pathValue)} } @@ -168,22 +168,3 @@ func (v *requestBodyValidator) ValidateRequestBodyWithPathItem(request *http.Req return validationSucceeded, validationErrors } - -func (v *requestBodyValidator) extractContentType(contentType string, operation *v3.Operation) (*v3.MediaType, bool) { - ct, _, _ := helpers.ExtractContentType(contentType) - mediaType, ok := operation.RequestBody.Content.Get(ct) - if ok { - return mediaType, true - } - ctMediaRange := strings.SplitN(ct, "/", 2) - for contentPair := operation.RequestBody.Content.First(); contentPair != nil; contentPair = contentPair.Next() { - s := contentPair.Key() - mediaTypeValue := contentPair.Value() - opMediaRange := strings.SplitN(s, "/", 2) - if (opMediaRange[0] == "*" || opMediaRange[0] == ctMediaRange[0]) && - (opMediaRange[1] == "*" || opMediaRange[1] == ctMediaRange[1]) { - return mediaTypeValue, true - } - } - return nil, false -} diff --git a/requests/validate_body_test.go b/requests/validate_body_test.go index 3667443..9677716 100644 --- a/requests/validate_body_test.go +++ b/requests/validate_body_test.go @@ -2117,3 +2117,54 @@ paths: assert.True(t, valid) assert.Len(t, errors, 0) } + +func TestValidateBody_ContentTypeWithoutSubtype(t *testing.T) { + spec := `openapi: 3.1.0 +paths: + /burgers/createBurger: + post: + requestBody: + content: + application/json: + schema: + type: object` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewRequestBodyValidator(&m.Model) + + request, _ := http.NewRequest(http.MethodPost, "https://things.com/burgers/createBurger", + bytes.NewBufferString(`{}`)) + request.Header.Set(helpers.ContentTypeHeader, "application") + + valid, errs := v.ValidateRequestBody(request) + assert.False(t, valid) + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Message, "content type 'application' does not exist") +} + +func TestValidateBody_MostSpecificMediaRangeApplies(t *testing.T) { + spec := `openapi: 3.1.0 +paths: + /burgers/createBurger: + post: + requestBody: + content: + "*/*": + schema: + type: string + application/*: + schema: + type: object` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewRequestBodyValidator(&m.Model) + + request, _ := http.NewRequest(http.MethodPost, "https://things.com/burgers/createBurger", + bytes.NewBufferString(`{"name":"Big Mac"}`)) + request.Header.Set(helpers.ContentTypeHeader, helpers.JSONContentType) + + valid, errs := v.ValidateRequestBody(request) + assert.True(t, valid, errs) +} diff --git a/responses/validate_body.go b/responses/validate_body.go index 06c7c7a..d363f2f 100644 --- a/responses/validate_body.go +++ b/responses/validate_body.go @@ -15,6 +15,7 @@ import ( "github.com/pb33f/libopenapi/orderedmap" v3 "github.com/pb33f/libopenapi/datamodel/high/v3" + lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" "github.com/pb33f/libopenapi-validator/config" "github.com/pb33f/libopenapi-validator/content" @@ -59,9 +60,6 @@ func (v *responseBodyValidator) ValidateResponseBodyWithPathItem(request *http.R contentType := response.Header.Get(helpers.ContentTypeHeader) codeStr := strconv.Itoa(httpCode) - // extract the media type from the content type header. - mediaTypeSting, _, _ := helpers.ExtractContentType(contentType) - // check if operation has responses defined if operation.Responses == nil || operation.Responses.Codes == nil { return true, nil @@ -77,51 +75,39 @@ func (v *responseBodyValidator) ValidateResponseBodyWithPathItem(request *http.R } } - if foundResponse != nil { - if v.options.ValidateResponseBody && foundResponse.Content != nil { // only validate if we have content types. - // check content type has been defined in the contract - if mediaType, ok := foundResponse.Content.Get(mediaTypeSting); ok { - validationErrors = append(validationErrors, - v.checkResponseSchema(request, response, contentType, mediaType, operation)...) - } else { - // check that the operation *actually* returns a body. (i.e. a 204 response) - if foundResponse.Content != nil && orderedmap.Len(foundResponse.Content) > 0 { - // content type not found in the contract - validationErrors = append(validationErrors, - errors.ResponseContentTypeNotFound(operation, request, response, codeStr, false)) - } - } + // the default response covers every code without a match, whether or not it declares content. + isDefault := false + if foundResponse == nil && operation.Responses.Default != nil { + foundResponse = operation.Responses.Default + isDefault = true + } + + if foundResponse == nil { + if v.options.ValidateResponseStatus { + // no default, no code match, nothing! + validationErrors = append(validationErrors, + errors.ResponseCodeNotFound(operation, request, httpCode)) } } else { - // no code match, check for default response - if operation.Responses.Default != nil && operation.Responses.Default.Content != nil { + // only validate if the response declares content (a 204 response does not). + if v.options.ValidateResponseBody && orderedmap.Len(foundResponse.Content) > 0 { // check content type has been defined in the contract - if !v.options.ValidateResponseBody { - foundResponse = operation.Responses.Default - } else if mediaType, ok := operation.Responses.Default.Content.Get(mediaTypeSting); ok { - foundResponse = operation.Responses.Default + if mediaType, ok := helpers.FindMediaType(foundResponse.Content, contentType); ok { validationErrors = append(validationErrors, v.checkResponseSchema(request, response, contentType, mediaType, operation)...) } else { - // check that the operation *actually* returns a body. (i.e. a 204 response) - if operation.Responses.Default.Content != nil && orderedmap.Len(operation.Responses.Default.Content) > 0 { - // content type not found in the contract - validationErrors = append(validationErrors, - errors.ResponseContentTypeNotFound(operation, request, response, codeStr, true)) - } + validationErrors = append(validationErrors, + errors.ResponseContentTypeNotFound(operation, request, response, codeStr, isDefault)) } - } else if v.options.ValidateResponseStatus { - // TODO: add support for '2XX' and '3XX' responses in the contract - // no default, no code match, nothing! - validationErrors = append(validationErrors, - errors.ResponseCodeNotFound(operation, request, httpCode)) } - } - if foundResponse != nil { // check for headers in the response if foundResponse.Headers != nil { - if ok, hErrs := ValidateResponseHeaders(request, response, foundResponse.Headers, pathFound, codeStr, config.WithExistingOpts(v.options)); !ok { + headerCode := codeStr + if isDefault { + headerCode = lowv3.DefaultLabel + } + if ok, hErrs := ValidateResponseHeaders(request, response, foundResponse.Headers, pathFound, headerCode, config.WithExistingOpts(v.options)); !ok { validationErrors = append(validationErrors, hErrs...) } } diff --git a/responses/validate_body_test.go b/responses/validate_body_test.go index 8de2675..6786ed3 100644 --- a/responses/validate_body_test.go +++ b/responses/validate_body_test.go @@ -1982,3 +1982,115 @@ func (er *errorReader) Read(p []byte) (n int, err error) { func (er *errorReader) Close() error { return nil } + +func newResponseBodyValidatorForSpec(t *testing.T, spec string) ResponseBodyValidator { + t.Helper() + doc, err := libopenapi.NewDocument([]byte(spec)) + require.NoError(t, err) + m, errs := doc.BuildV3Model() + require.NoError(t, errs) + return NewResponseBodyValidator(&m.Model) +} + +func jsonResponse(statusCode int, contentType, body string) *http.Response { + return &http.Response{ + StatusCode: statusCode, + Header: http.Header{helpers.ContentTypeHeader: {contentType}}, + Body: io.NopCloser(strings.NewReader(body)), + } +} + +func TestValidateBody_MediaTypeRanges(t *testing.T) { + // each range declares a different "kind" so the test can see which one validated the body + v := newResponseBodyValidatorForSpec(t, `openapi: 3.1.0 +info: {title: ranges, version: 1.0.0} +paths: + /things: + get: + responses: + '200': + description: ok + content: + "*/*": + schema: {type: object, properties: {kind: {const: any}}} + "application/*": + schema: {type: object, properties: {kind: {const: application}}} + "application/*+json": + schema: {type: object, properties: {kind: {const: suffix}}} + "application/json": + schema: {type: object, properties: {kind: {const: exact}}}`) + + for _, test := range []struct { + contentType string + kind string + }{ + {"application/json; charset=utf-8", "exact"}, + {"application/problem+json", "suffix"}, + {"application/x-custom-json", "application"}, + {"text/x-json", "any"}, + } { + t.Run(test.contentType, func(t *testing.T) { + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + valid, errs := v.ValidateResponseBody(request, + jsonResponse(http.StatusOK, test.contentType, fmt.Sprintf(`{"kind":%q}`, test.kind))) + assert.True(t, valid, errs) + + request, _ = http.NewRequest(http.MethodGet, "https://things.com/things", nil) + valid, _ = v.ValidateResponseBody(request, + jsonResponse(http.StatusOK, test.contentType, `{"kind":"wrong"}`)) + assert.False(t, valid) + }) + } +} + +func TestValidateBody_ContentTypeWithoutSubtype(t *testing.T) { + v := newResponseBodyValidatorForSpec(t, `openapi: 3.1.0 +info: {title: ranges, version: 1.0.0} +paths: + /things: + get: + responses: + '200': + description: ok + content: + application/json: + schema: {type: object}`) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + valid, errs := v.ValidateResponseBody(request, jsonResponse(http.StatusOK, "application", `{}`)) + assert.False(t, valid) + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Message, "content type 'application' does not exist") +} + +func TestValidateBody_DefaultResponseWithoutContent(t *testing.T) { + v := newResponseBodyValidatorForSpec(t, `openapi: 3.1.0 +info: {title: default, version: 1.0.0} +paths: + /things: + get: + responses: + '200': + description: ok + default: + description: an error, described by headers only + headers: + X-Error-Code: + required: true + schema: {type: integer}`) + + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + response := &http.Response{StatusCode: http.StatusInternalServerError, Header: http.Header{"X-Error-Code": {"42"}}} + valid, errs := v.ValidateResponseBody(request, response) + assert.True(t, valid, errs) + + request, _ = http.NewRequest(http.MethodGet, "https://things.com/things", nil) + response = &http.Response{StatusCode: http.StatusInternalServerError, Header: http.Header{}} + valid, errs = v.ValidateResponseBody(request, response) + assert.False(t, valid) + require.Len(t, errs, 1) + assert.Equal(t, "Missing required header", errs[0].Message) + require.Len(t, errs[0].SchemaValidationErrors, 1) + assert.Equal(t, "/paths/things/get/responses/default/headers/X-Error-Code/required", + errs[0].SchemaValidationErrors[0].KeywordLocation) +} diff --git a/responses/validate_headers.go b/responses/validate_headers.go index 67a263d..f12680c 100644 --- a/responses/validate_headers.go +++ b/responses/validate_headers.go @@ -4,10 +4,13 @@ package responses import ( + "encoding/json" "fmt" "net/http" + "strconv" "strings" + "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/pb33f/libopenapi/orderedmap" v3 "github.com/pb33f/libopenapi/datamodel/high/v3" @@ -45,7 +48,7 @@ func ValidateResponseHeaders( for pair := headers.First(); pair != nil; pair = pair.Next() { k := pair.Key() header := pair.Value() - if strings.EqualFold(k, name) { + if strings.EqualFold(k, name) && !ignoredResponseHeader(k) { locatedHeaders[strings.ToLower(name)] = headerPair{ name: k, value: v, @@ -59,7 +62,7 @@ func ValidateResponseHeaders( for pair := headers.First(); pair != nil; pair = pair.Next() { name := pair.Key() header := pair.Value() - if header.Required { + if header.Required && !ignoredResponseHeader(name) { if _, ok := locatedHeaders[strings.ToLower(name)]; !ok { keywordLocation := helpers.ConstructResponseHeaderJSONPointer(pathTemplate, request.Method, statusCode, name, "required") @@ -90,15 +93,13 @@ func ValidateResponseHeaders( } } - // validate the model schemas if they are set. + // validate every header that is present against its schema, whether it is required or not. for h, header := range locatedHeaders { if header.model.Schema != nil { - schema := header.model.Schema.Schema() - if schema != nil && header.model.Required { + if schema := header.model.Schema.Schema(); schema != nil { for _, headerValue := range header.value { validationErrors = append(validationErrors, - parameters.ValidateParameterSchema(schema, nil, headerValue, "header", - "response header", h, helpers.ResponseBodyValidation, lowv3.HeadersLabel, options)...) + validateHeaderValue(headerValue, schema, header.model.Explode, h, options)...) } } } @@ -135,3 +136,90 @@ func ValidateResponseHeaders( } return true, nil } + +// ignoredResponseHeader reports whether a declared response header is ignored: OpenAPI says a +// header named Content-Type SHALL be ignored, because the response's content map describes it. +func ignoredResponseHeader(name string) bool { + return strings.EqualFold(name, helpers.ContentTypeHeader) +} + +// validateHeaderValue validates a header value against its schema. Header values are text that can +// be read more than one way ("5" is the number 5 and the string "5"), so every reading the schema +// allows is tried, and the value is valid when any of them is. Otherwise the errors of the first +// reading are returned. +func validateHeaderValue(value string, schema *base.Schema, explode bool, name string, options *config.ValidationOptions) []*errors.ValidationError { + var firstErrors []*errors.ValidationError + for i, reading := range headerValueReadings(value, schema, explode) { + readingErrors := parameters.ValidateSingleParameterSchema(schema, reading, "header", "response header", + name, helpers.ResponseBodyValidation, lowv3.HeadersLabel, options, "", "") + if len(readingErrors) == 0 { + return nil + } + if i == 0 { + firstErrors = readingErrors + } + } + return firstErrors +} + +// headerValueReadings returns the ways a header value can be read: as each type the schema declares, +// in order (an array or object is split as the simple style serializes it), then as JSON, then as the +// string it was sent as. +func headerValueReadings(value string, schema *base.Schema, explode bool) []any { + var readings []any + readAsString := false + for _, schemaType := range schema.Type { + switch schemaType { + case helpers.String: + readings = append(readings, value) + readAsString = true + case helpers.Integer: + if parsed, err := helpers.ParseInteger(value); err == nil { + readings = append(readings, parsed) + } + case helpers.Number: + if parsed, err := helpers.ParseNumber(value); err == nil { + readings = append(readings, parsed) + } + case helpers.Boolean: + if parsed, err := strconv.ParseBool(value); err == nil { + readings = append(readings, parsed) + } + case helpers.Array: + readings = append(readings, headerArrayItems(value, schema)) + case helpers.Object: + if explode { + readings = append(readings, helpers.ConstructKVFromCSVWithSchema(value, schema)) + } else { + readings = append(readings, helpers.ConstructMapFromCSVWithSchema(value, schema)) + } + } + } + var decoded any + if err := json.Unmarshal([]byte(value), &decoded); err == nil { + readings = append(readings, decoded) + } + if !readAsString { + readings = append(readings, value) + } + return readings +} + +// headerArrayItems splits a simple style array header into its items, each read as the first +// reading its items schema allows. +func headerArrayItems(value string, schema *base.Schema) []any { + var itemSchema *base.Schema + if schema.Items != nil && schema.Items.IsA() && schema.Items.A != nil { + itemSchema = schema.Items.A.Schema() + } + items := strings.Split(value, helpers.Comma) + decoded := make([]any, len(items)) + for i, item := range items { + item = strings.TrimSpace(item) + decoded[i] = item + if itemSchema != nil { + decoded[i] = headerValueReadings(item, itemSchema, false)[0] + } + } + return decoded +} diff --git a/responses/validate_headers_test.go b/responses/validate_headers_test.go index b8bf5c3..0b113a1 100644 --- a/responses/validate_headers_test.go +++ b/responses/validate_headers_test.go @@ -9,7 +9,9 @@ import ( "testing" "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "github.com/pb33f/libopenapi-validator/config" ) @@ -220,3 +222,231 @@ paths: assert.True(t, valid) assert.Len(t, errors, 0) } + +func TestValidateResponseHeaders_OptionalHeaderIsValidatedWhenPresent(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + headers: + X-Rate-Limit: + schema: + type: integer` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + headers := m.Model.Paths.PathItems.GetOrZero("/things").Get.Responses.Codes.GetOrZero("200").Headers + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + + response := &http.Response{StatusCode: http.StatusOK, Header: http.Header{"X-Rate-Limit": {"abc"}}} + valid, errors := ValidateResponseHeaders(request, response, headers, "/things", "200") + assert.False(t, valid) + require.Len(t, errors, 1) + assert.Equal(t, "header 'x-rate-limit' failed to validate", errors[0].Message) + + response = &http.Response{StatusCode: http.StatusOK, Header: http.Header{}} + valid, errors = ValidateResponseHeaders(request, response, headers, "/things", "200") + assert.True(t, valid) + assert.Empty(t, errors) +} + +func TestValidateResponseHeaders_ContentHeaderIsNotSchemaValidated(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + headers: + X-Payload: + content: + application/json: + schema: + type: integer` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + headers := m.Model.Paths.PathItems.GetOrZero("/things").Get.Responses.Codes.GetOrZero("200").Headers + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + + response := &http.Response{StatusCode: http.StatusOK, Header: http.Header{"X-Payload": {`{"a":1}`}}} + valid, errors := ValidateResponseHeaders(request, response, headers, "/things", "200") + assert.True(t, valid) + assert.Empty(t, errors) +} + +func TestValidateResponseHeaders_ValuesDecodeBySchemaType(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + headers: + X-String: + schema: {type: string, maxLength: 5} + X-Integer: + schema: {type: integer, maximum: 10} + X-Number: + schema: {type: number, minimum: 1.5} + X-Boolean: + schema: {type: boolean} + X-Array: + schema: {type: array, items: {type: integer}, maxItems: 3} + X-Object: + schema: + type: object + properties: + id: {type: integer} + X-Exploded: + explode: true + schema: + type: object + properties: + id: {type: integer} + X-Enum: + schema: {enum: [1, 2]} + X-NaN: + schema: {type: number, maximum: 10} + X-Either: + schema: {type: [number, boolean]} + X-Nullable: + schema: {type: [integer, "null"]} + X-Short: + schema: {type: [string, integer], maxLength: 2} + X-StringEnum: + schema: {enum: ["1", "2"]} + X-Wrapped: + schema: + allOf: + - type: string` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + headers := m.Model.Paths.PathItems.GetOrZero("/things").Get.Responses.Codes.GetOrZero("200").Headers + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + + for _, test := range []struct { + header string + value string + valid bool + }{ + {"X-String", "123", true}, + {"X-String", "true", true}, + {"X-String", "null", true}, + {"X-String", "too long", false}, + {"X-Integer", "7", true}, + {"X-Integer", "7.0", true}, + {"X-Integer", "11", false}, + {"X-Integer", "seven", false}, + {"X-Number", "2.5", true}, + {"X-Number", "1", false}, + {"X-Boolean", "false", true}, + {"X-Boolean", "maybe", false}, + {"X-Array", "1", true}, + {"X-Array", "1, 2,3", true}, + {"X-Array", "1,two", false}, + {"X-Array", "1,2,3,4", false}, + {"X-Object", "id,5", true}, + {"X-Object", "id,five", false}, + {"X-Exploded", "id=5", true}, + {"X-Exploded", "id=five", false}, + {"X-Enum", "2", true}, + {"X-Enum", "3", false}, + {"X-NaN", "NaN", false}, + {"X-Either", "1.5", true}, + {"X-Either", "true", true}, + {"X-Either", "maybe", false}, + {"X-Nullable", "null", true}, + {"X-Nullable", "7", true}, + {"X-Short", "12345", true}, + {"X-StringEnum", "1", true}, + {"X-Wrapped", "123", true}, + {"X-Array", "[1,2]", true}, + } { + t.Run(test.header+"="+test.value, func(t *testing.T) { + response := &http.Response{StatusCode: http.StatusOK, Header: http.Header{}} + response.Header.Set(test.header, test.value) + + valid, errors := ValidateResponseHeaders(request, response, headers, "/things", "200") + assert.Equal(t, test.valid, valid, errors) + }) + } +} + +func TestHeaderValueReadings(t *testing.T) { + assert.Equal(t, []any{"5", float64(5)}, headerValueReadings("5", &base.Schema{Type: []string{"string"}}, false)) + assert.Equal(t, []any{int64(5), 5.0, float64(5), "5"}, + headerValueReadings("5", &base.Schema{Type: []string{"integer", "number"}}, false)) + assert.Equal(t, []any{[]any{"a", "b"}, "a,b"}, headerValueReadings("a,b", &base.Schema{Type: []string{"array"}}, false)) + assert.Equal(t, []any{"not-json"}, headerValueReadings("not-json", &base.Schema{}, false)) + assert.Equal(t, []any{map[string]any{"a": float64(1)}, `{"a":1}`}, headerValueReadings(`{"a":1}`, &base.Schema{}, false)) +} + +func TestValidateResponseHeaders_ContentTypeIsIgnored(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + headers: + Content-Type: + required: true + schema: {type: string, enum: [application/json]}` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + headers := m.Model.Paths.PathItems.GetOrZero("/things").Get.Responses.Codes.GetOrZero("200").Headers + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + + for _, header := range []http.Header{{"Content-Type": {"application/json; charset=utf-8"}}, {}} { + valid, errors := ValidateResponseHeaders(request, &http.Response{StatusCode: http.StatusOK, Header: header}, headers, "/things", "200") + assert.True(t, valid, errors) + } +} + +func TestValidateResponseHeaders_OpenAPI30Keywords(t *testing.T) { + spec := `openapi: 3.0.3 +info: + title: Headers + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + headers: + X-Rate: + schema: {type: integer, nullable: true, minimum: 1, exclusiveMinimum: true}` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + headers := m.Model.Paths.PathItems.GetOrZero("/things").Get.Responses.Codes.GetOrZero("200").Headers + request, _ := http.NewRequest(http.MethodGet, "https://things.com/things", nil) + + for value, valid := range map[string]bool{"5": true, "null": true, "1": false, "abc": false} { + response := &http.Response{StatusCode: http.StatusOK, Header: http.Header{"X-Rate": {value}}} + ok, errors := ValidateResponseHeaders(request, response, headers, "/things", "200") + assert.Equal(t, valid, ok, "%s: %v", value, errors) + } +} diff --git a/router/router.go b/router/router.go index 89d07ef..fd964e4 100644 --- a/router/router.go +++ b/router/router.go @@ -70,6 +70,9 @@ type Route struct { PathItem *v3.PathItem // PathItem is the matched OpenAPI path item. Method string // Method is the request method, including additional methods. Operation *v3.Operation // Operation is nil on a method mismatch. + // RequestPath is the escaped request path, relative to Server, that matched Path. Path-only + // matching includes the request fragment ("/pets/1#top"); strict server matching does not. + RequestPath string // RawPathParams contains escaped parameter values exactly as matched in the URL path. RawPathParams map[string]string // PathParams contains URL-decoded operation path parameter values. @@ -174,7 +177,7 @@ func (r *routeFinder) FindRoute(request *http.Request) (*Route, error) { } if r.pathOnly { - return r.find(request, compatibilityPath(request, r.document), compatibilityServer(request, r.document), nil) + return r.findPathOnly(request) } if len(r.servers) == 0 { @@ -214,6 +217,26 @@ func (r *routeFinder) FindRoute(request *http.Request) (*Route, error) { return nil, &RouteError{Kind: ErrPathNotFound} } +// findPathOnly matches the request path, without its server base path, against the document paths. +// Each candidate path is tried in order; a method mismatch is returned only when no candidate matches. +func (r *routeFinder) findPathOnly(request *http.Request) (*Route, error) { + var partial *Route + var partialErr error + for _, candidate := range compatibilityCandidates(request, r.document) { + route, err := r.find(request, candidate.path, candidate.server, nil) + if err == nil { + return route, nil + } + if route != nil && partial == nil { + partial, partialErr = route, err + } + } + if partial != nil { + return partial, partialErr + } + return nil, &RouteError{Kind: ErrPathNotFound} +} + func usesImplicitServer(operation *v3.Operation, item *v3.PathItem, document *v3.Document) bool { return (operation == nil || len(operation.Servers) == 0) && (item == nil || len(item.Servers) == 0) && @@ -246,6 +269,7 @@ func (r *routeFinder) find(request *http.Request, requestPath string, server *v3 Document: r.document, Server: server, Path: path, + RequestPath: requestPath, PathItem: pathItem, Method: request.Method, Operation: operation, diff --git a/router/router_test.go b/router/router_test.go index 9e7572b..fa304ef 100644 --- a/router/router_test.go +++ b/router/router_test.go @@ -75,6 +75,7 @@ func TestRouterStrictMatchingAndContext(t *testing.T) { assert.Equal(t, "a/b", route.PathParams["id"]) assert.Equal(t, "v3", route.ServerParams["version"]) assert.Equal(t, "/operation/{version}", route.Server.URL) + assert.Equal(t, "/pets/a%2Fb", route.RequestPath) assert.Same(t, route.Document.Paths.PathItems.GetOrZero("/pets/{id}"), route.PathItem) } @@ -210,6 +211,28 @@ paths: route, err = compatible.FindRoute(httptestRequest(http.MethodGet, "https://wrong.example/doc/v1/pets/mine")) require.NoError(t, err) assert.Equal(t, "/pets/mine", route.Path) + assert.Equal(t, "/pets/mine", route.RequestPath) +} + +func TestRouterPathOnlyStripsTemplatedServerBasePath(t *testing.T) { + // url.Parse rejects a variable in the server host, but its base path must still be stripped + r := NewRouter(model(t, `openapi: 3.1.0 +info: {title: test, version: 1.0.0} +servers: + - url: https://{host}/api/v1 + variables: + host: {default: api.example.com} +paths: + /widgets/{id}: + get: {responses: {"200": {description: ok}}}`), WithPathOnlyMatching()) + t.Cleanup(r.Release) + + route, err := r.FindRoute(httptestRequest(http.MethodGet, "https://api.example.com/api/v1/widgets/5")) + require.NoError(t, err) + assert.Equal(t, "/widgets/{id}", route.Path) + assert.Equal(t, "/widgets/5", route.RequestPath) + assert.Equal(t, "https://{host}/api/v1", route.Server.URL) + assert.Equal(t, "5", route.PathParams["id"]) } func TestRouterImplicitServerSurvivesUnrelatedOperationOverride(t *testing.T) { @@ -475,15 +498,57 @@ func TestServerHelperEdges(t *testing.T) { func TestCompatibilityPathEdges(t *testing.T) { request := &http.Request{URL: &url.URL{Path: "plain", Fragment: "frag"}} - assert.Equal(t, "/plain#frag", compatibilityPath(request, nil)) - doc := &v3.Document{Servers: []*v3.Server{nil, {URL: "http://[::1"}, {URL: "/api"}}} - request.URL = &url.URL{Path: "/api/items"} - assert.Equal(t, "/items", compatibilityPath(request, doc)) - assert.Nil(t, compatibilityServer(nil, doc)) - assert.Nil(t, compatibilityServer(&http.Request{}, doc)) - assert.Nil(t, compatibilityServer(request, nil)) - assert.Same(t, doc.Servers[2], compatibilityServer(request, doc)) - assert.Nil(t, compatibilityServer(httptestRequest(http.MethodGet, "http://example.com/other"), doc)) + assert.Equal(t, []compatibilityCandidate{{path: "/plain#frag"}}, compatibilityCandidates(request, nil)) + + doc := &v3.Document{Servers: []*v3.Server{nil, {URL: "http://[::1"}, {URL: "/api"}, {URL: "/api/v2"}}} + request.URL = &url.URL{Path: "/api/v2/items"} + assert.Equal(t, []compatibilityCandidate{{"/v2/items", doc.Servers[2]}, {"/items", doc.Servers[3]}}, + compatibilityCandidates(request, doc)) + + request.URL = &url.URL{Path: "/other"} + assert.Equal(t, []compatibilityCandidate{{path: "/other"}}, compatibilityCandidates(request, doc)) +} + +func TestRouterPathOnlyTriesEveryMatchingServer(t *testing.T) { + r := NewRouter(model(t, `openapi: 3.1.0 +info: {title: test, version: 1.0.0} +servers: + - url: http://localhost:{port}/ + variables: + port: {default: "8080"} + - url: https://{tenant}.example.com/api + variables: + tenant: {default: acme} + - url: https://example.com/api/v2 +paths: + /widgets/{id}: + get: {responses: {"200": {description: ok}}} + /v2/things: + post: {responses: {"200": {description: ok}}}`), WithPathOnlyMatching()) + t.Cleanup(r.Release) + + // the first two servers leave paths the document does not declare, so the third applies + route, err := r.FindRoute(httptestRequest(http.MethodGet, "https://example.com/api/v2/widgets/5")) + require.NoError(t, err) + assert.Equal(t, "/widgets/{id}", route.Path) + assert.Equal(t, "/widgets/5", route.RequestPath) + assert.Equal(t, "https://example.com/api/v2", route.Server.URL) + + // an earlier server keeps precedence when the path it leaves is declared + route, err = r.FindRoute(httptestRequest(http.MethodPost, "https://acme.example.com/api/v2/things")) + require.NoError(t, err) + assert.Equal(t, "/v2/things", route.Path) + assert.Equal(t, "https://{tenant}.example.com/api", route.Server.URL) + + // a method mismatch is reported when no server leaves a path with the method + route, err = r.FindRoute(httptestRequest(http.MethodDelete, "https://example.com/api/v2/widgets/5")) + require.ErrorIs(t, err, ErrMethodNotAllowed) + require.NotNil(t, route) + assert.Equal(t, "/widgets/{id}", route.Path) + + route, err = r.FindRoute(httptestRequest(http.MethodGet, "https://example.com/api/v2/missing")) + assert.ErrorIs(t, err, ErrPathNotFound) + assert.Nil(t, route) } func TestConcurrentLookupAndRelease(t *testing.T) { diff --git a/router/server.go b/router/server.go index aaed245..f5fa8b7 100644 --- a/router/server.go +++ b/router/server.go @@ -12,6 +12,8 @@ import ( "github.com/pb33f/libopenapi/orderedmap" v3 "github.com/pb33f/libopenapi/datamodel/high/v3" + + "github.com/pb33f/libopenapi-validator/internal/serverurl" ) type compiledServer struct { @@ -192,20 +194,37 @@ func contains(values []string, target string) bool { return false } -func compatibilityPath(request *http.Request, document *v3.Document) string { +// compatibilityCandidate is a request path, and the server whose base path was removed from it. +type compatibilityCandidate struct { + path string + server *v3.Server +} + +// compatibilityCandidates returns the paths path-only matching tries for a request: the request path +// with the base path of each document server that prefixes it removed, in document order, or the +// request path as sent when no server does. Trying every server lets a later, more specific server +// match when an earlier one leaves a path the document does not declare. +func compatibilityCandidates(request *http.Request, document *v3.Document) []compatibilityCandidate { path := request.URL.EscapedPath() + var candidates []compatibilityCandidate if document != nil { for _, server := range document.Servers { if server == nil { continue } - parsed, err := url.Parse(server.URL) - if err == nil && parsed.Path != "" && strings.HasPrefix(path, parsed.Path) { - path = strings.TrimPrefix(path, parsed.Path) - break + if base := serverurl.BasePath(server.URL); base != "" && strings.HasPrefix(path, base) { + candidates = append(candidates, compatibilityCandidate{compatibilityPath(request, strings.TrimPrefix(path, base)), server}) } } } + if len(candidates) == 0 { + candidates = append(candidates, compatibilityCandidate{compatibilityPath(request, path), nil}) + } + return candidates +} + +// compatibilityPath appends the request fragment to path, and makes it absolute. +func compatibilityPath(request *http.Request, path string) string { if request.URL.Fragment != "" { path += "#" + request.URL.Fragment } @@ -214,20 +233,3 @@ func compatibilityPath(request *http.Request, document *v3.Document) string { } return path } - -func compatibilityServer(request *http.Request, document *v3.Document) *v3.Server { - if request == nil || request.URL == nil || document == nil { - return nil - } - path := request.URL.EscapedPath() - for _, server := range document.Servers { - if server == nil { - continue - } - parsed, err := url.Parse(server.URL) - if err == nil && parsed.Path != "" && strings.HasPrefix(path, parsed.Path) { - return server - } - } - return nil -} diff --git a/schema_validation/directional_schema.go b/schema_validation/directional_schema.go index 52ea352..c89aeec 100644 --- a/schema_validation/directional_schema.go +++ b/schema_validation/directional_schema.go @@ -54,6 +54,10 @@ func RenderSchemaForValidation(schema *base.Schema, purpose SchemaValidationPurp renderCtx := base.NewInlineRenderContextForValidation() nodeIface, err := schema.MarshalYAMLInlineWithContext(renderCtx) renderedNode, _ := nodeIface.(*yaml.Node) + // the inline renderer can hand back nodes owned by the source document (a circular $ref + // keeps its authored node), and both directional pruning and yaml.Marshal, which strips + // resolved tags from the nodes it encodes, edit the tree in place. Work on a copy. + renderedNode = cloneYAMLNode(renderedNode) if err != nil { renderedInline, _ := yaml.Marshal(renderedNode) return &RenderedValidationSchema{ diff --git a/schema_validation/directional_schema_test.go b/schema_validation/directional_schema_test.go index 52b950b..d5fd8ed 100644 --- a/schema_validation/directional_schema_test.go +++ b/schema_validation/directional_schema_test.go @@ -103,6 +103,43 @@ components: require.NotNil(t, rendered) } +func TestRenderSchemaForValidation_DoesNotMutateDocumentNodes(t *testing.T) { + // circular refs render as the authored $ref node, which belongs to the document + doc, err := libopenapi.NewDocument([]byte(`openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +components: + schemas: + Tree: + type: object + required: + - id + properties: + id: + type: string + readOnly: true + child: + $ref: '#/components/schemas/Tree'`)) + require.NoError(t, err) + model, errs := doc.BuildV3Model() + require.Empty(t, errs) + + root := doc.GetSpecInfo().RootNode + before := yamlNodeStates(root) + + schema := model.Model.Components.Schemas.GetOrZero("Tree").Schema() + for _, purpose := range []SchemaValidationPurpose{ + SchemaValidationPurposeGeneric, + SchemaValidationPurposeRequestBody, + SchemaValidationPurposeResponseBody, + } { + _, _ = RenderSchemaForValidation(schema, purpose) + } + + assert.Equal(t, before, yamlNodeStates(root)) +} + func TestRenderSchemaBytesForValidation_Errors(t *testing.T) { rendered, err := renderSchemaBytesForValidation([]byte(":\n"), SchemaValidationPurposeRequestBody) require.Error(t, err) @@ -213,6 +250,28 @@ func TestDirectionalSchemaHelpers_EdgeCases(t *testing.T) { assert.Empty(t, node.Content) } +// yamlNodeStates records the tag, style and child count of every node reachable from root, +// so tests can prove an operation left a caller's YAML tree untouched. +func yamlNodeStates(root *yaml.Node) map[*yaml.Node]string { + states := make(map[*yaml.Node]string) + var walk func(*yaml.Node) + walk = func(node *yaml.Node) { + if node == nil { + return + } + if _, seen := states[node]; seen { + return + } + states[node] = fmt.Sprintf("%s|%d|%d", node.Tag, node.Style, len(node.Content)) + for _, child := range node.Content { + walk(child) + } + walk(node.Alias) + } + walk(root) + return states +} + func renderedRequired(t *testing.T, renderedJSON []byte) []string { t.Helper() diff --git a/schema_validation/validate_document.go b/schema_validation/validate_document.go index 4db2fca..dff8fde 100644 --- a/schema_validation/validate_document.go +++ b/schema_validation/validate_document.go @@ -378,7 +378,9 @@ func ValidateOpenAPIDocumentWithPrecompiled(doc libopenapi.Document, compiledSch // location of the violation within the rendered schema. violation.Line = line violation.Column = located.Column - if source, err := yaml.Marshal(located); err == nil { + // located belongs to the caller's document, and yaml.Marshal strips + // resolved tags from the nodes it encodes, so render a copy. + if source, err := yaml.Marshal(cloneYAMLNode(located)); err == nil { violation.ReferenceObject = strings.TrimSpace(string(source)) } } else { diff --git a/schema_validation/validate_document_test.go b/schema_validation/validate_document_test.go index c5b10ad..bf2e77f 100644 --- a/schema_validation/validate_document_test.go +++ b/schema_validation/validate_document_test.go @@ -12,6 +12,7 @@ import ( "github.com/pb33f/libopenapi" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" "github.com/pb33f/libopenapi-validator/config" liberrors "github.com/pb33f/libopenapi-validator/errors" @@ -181,19 +182,48 @@ x-values: assert.Empty(t, errors[0].SchemaValidationErrors) } +// unmarshalableJSONValue is a value encoding/json rejects on every supported Go version. Maps with +// non-string keys are not: Go 1.27 marshals map[interface{}]interface{}{1: "one"} as {"1":"one"}. +func unmarshalableJSONValue() chan int { + return make(chan int) +} + func TestNormalizeJSON_ReturnsMarshalError(t *testing.T) { payload := map[string]interface{}{ "openapi": "3.1.0", - "invalid": map[interface{}]interface{}{ - 1: "one", - }, + "invalid": unmarshalableJSONValue(), } normalized, err := normalizeJSON(payload) assert.Nil(t, normalized) - assert.Error(t, err) - assert.Contains(t, err.Error(), "unsupported type: map[interface {}]interface {}") + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported type: chan int") +} + +func TestValidateDocument_ViolationDoesNotMutateDocumentNodes(t *testing.T) { + spec := `openapi: 3.1.0 +info: + title: Test + version: 1.0.0 +paths: + /things: + get: + responses: + '200': + description: ok + badProperty: 12` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + root := doc.GetSpecInfo().RootNode + before := yamlNodeStates(root) + + valid, errors := ValidateOpenAPIDocument(doc) + + assert.False(t, valid) + require.Len(t, errors, 1) + require.NotEmpty(t, errors[0].SchemaValidationErrors) + assert.Equal(t, before, yamlNodeStates(root)) } func TestValidateDocument_NormalizationErrorDoesNotValidateNil(t *testing.T) { @@ -206,9 +236,7 @@ paths: {}` doc, _ := libopenapi.NewDocument([]byte(spec)) badSpecJSON := map[string]interface{}{ "openapi": "3.1.0", - "invalid": map[interface{}]interface{}{ - 1: "one", - }, + "invalid": unmarshalableJSONValue(), } info := doc.GetSpecInfo() // the JSON view is built lazily behind a sync.Once; latch it before @@ -224,7 +252,7 @@ paths: {}` assert.Len(t, errors, 1) assert.Equal(t, "OpenAPI document validation failed", errors[0].Message) assert.Contains(t, errors[0].Reason, "cannot be converted to JSON") - assert.Contains(t, errors[0].Reason, "unsupported type: map[interface {}]interface {}") + assert.Contains(t, errors[0].Reason, "unsupported type: chan int") assert.NotContains(t, errors[0].Reason, "got null, want object") assert.Empty(t, errors[0].SchemaValidationErrors) } @@ -239,9 +267,7 @@ paths: {}` doc, _ := libopenapi.NewDocument([]byte(spec)) badSpecJSON := map[string]interface{}{ "openapi": "3.1.0", - "invalid": map[interface{}]interface{}{ - 1: "one", - }, + "invalid": unmarshalableJSONValue(), } corrupt := []byte(`{not valid json!!!}`) info := doc.GetSpecInfo() diff --git a/validator_examples_test.go b/validator_examples_test.go index 8dee1ad..2a4f00a 100644 --- a/validator_examples_test.go +++ b/validator_examples_test.go @@ -359,12 +359,11 @@ paths: // 6. Create a new *http.Request (normally, this would be where the host application will pass in the request) request, _ := http.NewRequest(http.MethodGet, "/health", nil) - // 7. Simulate a request/response, in this case the contract returns a 200 with an array of pets. + // 7. Simulate a request/response, in this case the contract returns a 200 without the optional header. // Normally, this would be where the host application would pass in the response. + // A header that is sent is still validated against its schema, required or not. recorder := httptest.NewRecorder() handler := func(w http.ResponseWriter, r *http.Request) { - // set return content type. - w.Header().Set("Chicken-Nuggets", "I am a chicken nugget, and not an integer") w.WriteHeader(http.StatusOK) _, _ = w.Write(nil) }