diff --git a/default_validator.go b/default_validator.go index ebcd807..d6da321 100644 --- a/default_validator.go +++ b/default_validator.go @@ -4,9 +4,6 @@ package validate import ( - "fmt" - "strings" - "github.com/go-openapi/spec" ) @@ -44,28 +41,16 @@ func (d *defaultValidator) resetVisited() { } } -func isVisited(path string, visitedSchemas map[string]struct{}) bool { - _, found := visitedSchemas[path] +func isVisited(path pathSegments, visitedSchemas map[string]struct{}) bool { + _, found := visitedSchemas[path.pointer()] if found { return true } - // search for overlapping paths - var ( - parent string - suffix string - ) - const backtrackFromEnd = 2 - for i := len(path) - backtrackFromEnd; i >= 0; i-- { - r := path[i] - if r != '.' { - continue - } - - parent = path[0:i] - suffix = path[i+1:] - - if strings.HasSuffix(parent, suffix) { + // search for overlapping paths: a trailing run of tokens that already + // appears at the end of what leads to it means we are going in circles. + for i := 1; i < len(path); i++ { + if path[:i].hasSuffix(path[i:]) { return true } } @@ -74,12 +59,12 @@ func isVisited(path string, visitedSchemas map[string]struct{}) bool { } // beingVisited asserts a schema is being visited. -func (d *defaultValidator) beingVisited(path string) { - d.visitedSchemas[path] = struct{}{} +func (d *defaultValidator) beingVisited(path pathSegments) { + d.visitedSchemas[path.pointer()] = struct{}{} } // isVisited tells if a path has already been visited. -func (d *defaultValidator) isVisited(path string) bool { +func (d *defaultValidator) isVisited(path pathSegments) bool { return isVisited(path, d.visitedSchemas) } @@ -96,7 +81,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { // parameters for _, param := range paramHelp.safeExpandedParamsFor(path, method, op.ID, res, s) { if param.Default != nil && param.Required { - res.AddWarnings(requiredHasDefaultMsg(param.Name, param.In)) + res.addWarningsAt(s.parameterPath(path, method, param.In, param.Name), requiredHasDefaultMsg(param.Name, param.In)) } // reset explored schemas to get depth-first recursive-proof exploration @@ -108,7 +93,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { // check param default value is valid red := newParamValidator(¶m, s.KnownFormats, d.schemaOptions).Validate(param.Default) //#nosec if red.HasErrorsOrWarnings() { - res.AddErrors(defaultValueDoesNotValidateMsg(param.Name, param.In)) + res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), defaultValueDoesNotValidateMsg(param.Name, param.In)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -117,9 +102,9 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { // Recursively follows Items and Schemas if param.Items != nil { - red := d.validateDefaultValueItemsAgainstSchema(param.Name, param.In, ¶m, param.Items) //#nosec + red := d.validateDefaultValueItemsAgainstSchema(s.parameterPath(path, method, param.In, param.Name), param.In, ¶m, param.Items) //#nosec if red.HasErrorsOrWarnings() { - res.AddErrors(defaultValueItemsDoesNotValidateMsg(param.Name, param.In)) + res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), defaultValueItemsDoesNotValidateMsg(param.Name, param.In)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -128,9 +113,9 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { if param.Schema != nil { // Validate default value against schema - red := d.validateDefaultValueSchemaAgainstSchema(param.Name, param.In, param.Schema) + red := d.validateDefaultValueSchemaAgainstSchema(s.parameterPath(path, method, param.In, param.Name), param.In, param.Schema) if red.HasErrorsOrWarnings() { - res.AddErrors(defaultValueDoesNotValidateMsg(param.Name, param.In)) + res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), defaultValueDoesNotValidateMsg(param.Name, param.In)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -141,17 +126,17 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { if op.Responses != nil { if op.Responses.Default != nil { // Same constraint on default Response - res.Merge(d.validateDefaultInResponse(op.Responses.Default, jsonDefault, path, 0, op.ID)) + res.Merge(d.validateDefaultInResponse(op.Responses.Default, jsonDefault, path, method, 0, op.ID)) } // Same constraint on regular Responses if op.Responses.StatusCodeResponses != nil { // Safeguard for code, r := range op.Responses.StatusCodeResponses { - res.Merge(d.validateDefaultInResponse(&r, "response", path, code, op.ID)) //#nosec + res.Merge(d.validateDefaultInResponse(&r, "response", path, method, code, op.ID)) //#nosec } } } else if op.ID != "" { // Empty op.ID means there is no meaningful operation: no need to report a specific message - res.AddErrors(noValidResponseMsg(op.ID)) + res.addErrorsAt(operationPath(path, method), noValidResponseMsg(op.ID)) } } } @@ -159,13 +144,15 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result { // reset explored schemas to get depth-first recursive-proof exploration d.resetVisited() for nm, sch := range s.spec.Spec().Definitions { - res.Merge(d.validateDefaultValueSchemaAgainstSchema("definitions."+nm, "body", &sch)) //#nosec + res.Merge(d.validateDefaultValueSchemaAgainstSchema(newPathSegments(swaggerDefinitions, nm), "body", &sch)) //#nosec } } return res } -func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, responseType, path string, responseCode int, operationID string) *Result { +func (d *defaultValidator) validateDefaultInResponse( + resp *spec.Response, responseType, path, method string, responseCode int, operationID string, +) *Result { s := d.SpecValidator response, res := responseHelp.expandResponseRef(resp, path, s) @@ -183,7 +170,7 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon if h.Default != nil { red := newHeaderValidator(nm, &h, s.KnownFormats, d.schemaOptions).Validate(h.Default) //#nosec if red.HasErrorsOrWarnings() { - res.AddErrors(defaultValueHeaderDoesNotValidateMsg(operationID, nm, responseName)) + res.addErrorsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), defaultValueHeaderDoesNotValidateMsg(operationID, nm, responseName)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -192,9 +179,9 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon // Headers have inline definition, like params if h.Items != nil { - red := d.validateDefaultValueItemsAgainstSchema(nm, "header", &h, h.Items) //#nosec + red := d.validateDefaultValueItemsAgainstSchema(responseHeaderPath(path, method, responseCodeAsStr, nm), "header", &h, h.Items) //#nosec if red.HasErrorsOrWarnings() { - res.AddErrors(defaultValueHeaderItemsDoesNotValidateMsg(operationID, nm, responseName)) + res.addErrorsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), defaultValueHeaderItemsDoesNotValidateMsg(operationID, nm, responseName)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -202,7 +189,7 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon } if _, err := compileRegexp(h.Pattern); err != nil { - res.AddErrors(invalidPatternInHeaderMsg(operationID, nm, responseName, h.Pattern, err)) + res.addErrorsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), invalidPatternInHeaderMsg(operationID, nm, responseName, h.Pattern, err)) } // Headers don't have schema @@ -212,10 +199,11 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon // reset explored schemas to get depth-first recursive-proof exploration d.resetVisited() - red := d.validateDefaultValueSchemaAgainstSchema(responseCodeAsStr, "response", response.Schema) + red := d.validateDefaultValueSchemaAgainstSchema( + responsePath(path, method, responseCodeAsStr).structuralChild(jsonSchema), "response", response.Schema) if red.HasErrorsOrWarnings() { // Additional message to make sure the context of the error is not lost - res.AddErrors(defaultValueInDoesNotValidateMsg(operationID, responseName)) + res.addErrorsAt(responsePath(path, method, responseCodeAsStr), defaultValueInDoesNotValidateMsg(operationID, responseName)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -224,7 +212,7 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon return res } -func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in string, schema *spec.Schema) *Result { +func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path pathSegments, in string, schema *spec.Schema) *Result { if schema == nil || d.isVisited(path) { // Avoids recursing if we are already done with that check return nil @@ -235,39 +223,39 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri if schema.Default != nil { res.Merge( - newSchemaValidator(schema, s.spec.Spec(), path+".default", s.KnownFormats, d.schemaOptions).Validate(schema.Default), + newSchemaValidator(schema, s.spec.Spec(), path.child(jsonDefault), s.KnownFormats, d.schemaOptions).Validate(schema.Default), ) } if schema.Items != nil { if schema.Items.Schema != nil { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(path+".items.default", in, schema.Items.Schema)) + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonItems), in, schema.Items.Schema)) } // Multiple schemas in items if schema.Items.Schemas != nil { // Safeguard for i, sch := range schema.Items.Schemas { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(fmt.Sprintf("%s.items[%d].default", path, i), in, &sch)) //#nosec + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonItems).item(i), in, &sch)) //#nosec } } } if _, err := compileRegexp(schema.Pattern); err != nil { - res.AddErrors(invalidPatternInMsg(path, in, schema.Pattern)) + res.addErrorsAt(path, invalidPatternInMsg(path.dotted(), in, schema.Pattern)) } if schema.AdditionalItems != nil && schema.AdditionalItems.Schema != nil { // NOTE: we keep validating values, even though additionalItems is not supported by Swagger 2.0 (and 3.0 as well) - res.Merge(d.validateDefaultValueSchemaAgainstSchema(path+".additionalItems", in, schema.AdditionalItems.Schema)) + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonAdditionalItems), in, schema.AdditionalItems.Schema)) } for propName, prop := range schema.Properties { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(path+"."+propName, in, &prop)) //#nosec + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonProperties).child(propName), in, &prop)) //#nosec } for propName, prop := range schema.PatternProperties { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(path+"."+propName, in, &prop)) //#nosec + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonPatternProperties).child(propName), in, &prop)) //#nosec } if schema.AdditionalProperties != nil && schema.AdditionalProperties.Schema != nil { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(path+".additionalProperties", in, schema.AdditionalProperties.Schema)) + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonAdditionalProperties), in, schema.AdditionalProperties.Schema)) } if schema.AllOf != nil { for i, aoSch := range schema.AllOf { - res.Merge(d.validateDefaultValueSchemaAgainstSchema(fmt.Sprintf("%s.allOf[%d]", path, i), in, &aoSch)) //#nosec + res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonAllOf).item(i), in, &aoSch)) //#nosec } } return res @@ -275,7 +263,7 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri // NOTE: Temporary duplicated code. Need to refactor with examples -func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path, in string, root any, items *spec.Items) *Result { +func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path pathSegments, in string, root any, items *spec.Items) *Result { res := pools.poolOfResults.BorrowResult() s := d.SpecValidator if items != nil { @@ -285,10 +273,10 @@ func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path, in strin ) } if items.Items != nil { - res.Merge(d.validateDefaultValueItemsAgainstSchema(path+"[0].default", in, root, items.Items)) + res.Merge(d.validateDefaultValueItemsAgainstSchema(path.item(0), in, root, items.Items)) } if _, err := compileRegexp(items.Pattern); err != nil { - res.AddErrors(invalidPatternInMsg(path, in, items.Pattern)) + res.addErrorsAt(path, invalidPatternInMsg(path.dotted(), in, items.Pattern)) } } return res diff --git a/error_location_test.go b/error_location_test.go new file mode 100644 index 0000000..e21e7a8 --- /dev/null +++ b/error_location_test.go @@ -0,0 +1,213 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "encoding/json" + "testing" + + "github.com/go-openapi/loads" + "github.com/go-openapi/spec" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +const ( + nameProp = "name" + dummyProp = "dummy" +) + +// The tests below pin down where a validation error says it happened. +// +// They exist because locations used to be assembled by string concatenation, +// which lost array indices altogether, mixed dotted and bracketed notations, +// and duplicated keyword suffixes. + +func TestErrorLocation_ArrayItemsCarryTheirIndex(t *testing.T) { + t.Parallel() + + // the item index used to be dropped: the sub-validators that report the + // error were built before the index was known. + schema := new(spec.Schema) + require.NoError(t, json.Unmarshal([]byte(`{ + "type": "object", + "properties": { + "friends": { + "type": "array", + "items": { + "type": "object", + "properties": {"name": {"type": "string"}}, + "required": ["age"] + } + } + } + }`), schema)) + + data := map[string]any{ + "friends": []any{ + map[string]any{nameProp: "ok", "age": 1}, + map[string]any{nameProp: 42}, + }, + } + + res := NewSchemaValidator(schema, nil, "", strfmt.Default).Validate(data) + require.False(t, res.IsValid()) + + messages := errorMessages(res) + assert.SliceContainsT(t, messages, "friends.1.name in body must be of type string: \"integer\"") + assert.SliceContainsT(t, messages, "friends.1.age in body is required") + + for _, msg := range messages { + assert.StringNotContainsT(t, msg, "friends.name", "expected no location to elide the item index") + } +} + +func TestErrorLocation_TupleItemsCarryTheirIndex(t *testing.T) { + t.Parallel() + + schema := new(spec.Schema) + require.NoError(t, json.Unmarshal([]byte(`{ + "type": "array", + "items": [{"type": "string"}, {"type": "integer"}] + }`), schema)) + + res := NewSchemaValidator(schema, nil, "", strfmt.Default).Validate([]any{1, "two"}) + require.False(t, res.IsValid()) + + messages := errorMessages(res) + assert.SliceContainsT(t, messages, "0 in body must be of type string: \"integer\"") + assert.SliceContainsT(t, messages, "1 in body must be of type integer: \"string\"") +} + +func TestErrorLocation_RootRequiredHasNoLeadingSeparator(t *testing.T) { + t.Parallel() + + // a missing property at the root used to be reported as ".swagger", + // because the empty root was concatenated with a separator. + schema := new(spec.Schema) + require.NoError(t, json.Unmarshal([]byte(`{ + "type": "object", + "required": ["swagger"], + "properties": {"swagger": {"type": "string"}} + }`), schema)) + + res := NewSchemaValidator(schema, nil, "", strfmt.Default).Validate(map[string]any{}) + require.False(t, res.IsValid()) + require.Len(t, res.Errors, 1) + + assert.EqualError(t, res.Errors[0], "swagger in body is required") +} + +func TestErrorLocation_SpecDefaultsAndExamples(t *testing.T) { + t.Parallel() + + const raw = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": { + "/pets/{id}": { + "get": { + "operationId": "getPet", + "parameters": [ + {"name": "id", "in": "path", "required": true, "type": "string"} + ], + "responses": { + "200": { + "description": "ok", + "schema": {"$ref": "#/definitions/Pet"}, + "examples": { + "application/json": {"friends": [{"name": 7}]} + } + } + } + } + } + }, + "definitions": { + "Pet": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "friends": {"type": "array", "items": {"$ref": "#/definitions/Pet"}}, + "tuple": { + "type": "array", + "items": [{"type": "string", "default": 1}, {"type": "integer", "default": "x"}] + } + } + } + } + }` + + doc, err := loads.Analyzed(json.RawMessage(raw), "") + require.NoError(t, err) + + res, _ := NewSpecValidator(doc.Schema(), strfmt.Default).Validate(doc) + require.False(t, res.IsValid()) + + errs := errorMessages(res) + warns := warningMessages(res) + + t.Run("tuple item defaults use an index token, not a bracket", func(t *testing.T) { + assert.SliceContainsT(t, errs, + "definitions.Pet.tuple.items.0.default in body must be of type string: \"number\"") + assert.SliceContainsT(t, errs, + "definitions.Pet.tuple.items.1.default in body must be of type integer: \"string\"") + + for _, msg := range errs { + assert.StringNotContainsT(t, msg, "items[", "expected no bracketed index notation") + } + }) + + t.Run("the default keyword is not appended twice", func(t *testing.T) { + for _, msg := range append(errs, warns...) { + assert.StringNotContainsT(t, msg, ".default.default") + assert.StringNotContainsT(t, msg, ".example.example") + } + }) + + t.Run("an example inside an array carries the item index", func(t *testing.T) { + assert.SliceContainsT(t, warns, + "paths./pets/{id}.get.responses.200.examples.friends.0.name in body must be of type string: \"number\"") + }) + + t.Run("a response is located by its operation, not by the bare status code", func(t *testing.T) { + for _, msg := range append(errs, warns...) { + assert.NotEqualT(t, "200 in response has invalid pattern", msg) + } + assert.SliceContainsT(t, warns, + "paths./pets/{id}.get.responses.200.examples.friends.0.name in body must be of type string: \"number\"") + }) + + t.Run("the method is spelled as the document spells it", func(t *testing.T) { + for _, msg := range append(errs, warns...) { + assert.StringNotContainsT(t, msg, ".GET.", "expected the lower-case path item key") + } + }) +} + +func TestErrorLocation_ExampleItemsSkipSchemaOnlyChecks(t *testing.T) { + t.Parallel() + + // restoring the item index moved example data from "x.example" to + // "x.example.0", which must still count as being inside an example: + // the array-must-have-items check does not apply to plain data. + validator := newObjectValidator( + newPathSegments("itemsparam", swaggerExample, "0"), + "body", nil, nil, nil, nil, nil, nil, nil, nil, + &SchemaValidatorOptions{EnableObjectArrayTypeCheck: true, EnableArrayMustHaveItemsCheck: true}, + ) + + res := validator.Validate(map[string]any{jsonItems: dummyProp}) + assert.Empty(t, res.Errors, "expected schema-only checks to be skipped inside an example") +} + +func errorMessages(res *Result) []string { + messages := make([]string, 0, len(res.Errors)) + for _, err := range res.Errors { + messages = append(messages, err.Error()) + } + + return messages +} diff --git a/example_validator.go b/example_validator.go index eb6b5ee..07272a7 100644 --- a/example_validator.go +++ b/example_validator.go @@ -4,8 +4,6 @@ package validate import ( - "fmt" - "github.com/go-openapi/spec" ) @@ -50,12 +48,12 @@ func (ex *exampleValidator) resetVisited() { } // beingVisited asserts a schema is being visited. -func (ex *exampleValidator) beingVisited(path string) { - ex.visitedSchemas[path] = struct{}{} +func (ex *exampleValidator) beingVisited(path pathSegments) { + ex.visitedSchemas[path.pointer()] = struct{}{} } // isVisited tells if a path has already been visited. -func (ex *exampleValidator) isVisited(path string) bool { +func (ex *exampleValidator) isVisited(path pathSegments) bool { return isVisited(path, ex.visitedSchemas) } @@ -85,7 +83,7 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result { // check param default value is valid red := newParamValidator(¶m, s.KnownFormats, ex.schemaOptions).Validate(param.Example) //#nosec if red.HasErrorsOrWarnings() { - res.AddWarnings(exampleValueDoesNotValidateMsg(param.Name, param.In)) + res.addWarningsAt(s.parameterPath(path, method, param.In, param.Name), exampleValueDoesNotValidateMsg(param.Name, param.In)) res.MergeAsWarnings(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -94,9 +92,9 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result { // Recursively follows Items and Schemas if param.Items != nil { - red := ex.validateExampleValueItemsAgainstSchema(param.Name, param.In, ¶m, param.Items) //#nosec + red := ex.validateExampleValueItemsAgainstSchema(s.parameterPath(path, method, param.In, param.Name), param.In, ¶m, param.Items) //#nosec if red.HasErrorsOrWarnings() { - res.AddWarnings(exampleValueItemsDoesNotValidateMsg(param.Name, param.In)) + res.addWarningsAt(s.parameterPath(path, method, param.In, param.Name), exampleValueItemsDoesNotValidateMsg(param.Name, param.In)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -105,9 +103,9 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result { if param.Schema != nil { // Validate example value against schema - red := ex.validateExampleValueSchemaAgainstSchema(param.Name, param.In, param.Schema) + red := ex.validateExampleValueSchemaAgainstSchema(s.parameterPath(path, method, param.In, param.Name), param.In, param.Schema) if red.HasErrorsOrWarnings() { - res.AddWarnings(exampleValueDoesNotValidateMsg(param.Name, param.In)) + res.addWarningsAt(s.parameterPath(path, method, param.In, param.Name), exampleValueDoesNotValidateMsg(param.Name, param.In)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -118,17 +116,17 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result { if op.Responses != nil { if op.Responses.Default != nil { // Same constraint on default Response - res.Merge(ex.validateExampleInResponse(op.Responses.Default, jsonDefault, path, 0, op.ID)) + res.Merge(ex.validateExampleInResponse(op.Responses.Default, jsonDefault, path, method, 0, op.ID)) } // Same constraint on regular Responses if op.Responses.StatusCodeResponses != nil { // Safeguard for code, r := range op.Responses.StatusCodeResponses { - res.Merge(ex.validateExampleInResponse(&r, "response", path, code, op.ID)) //#nosec + res.Merge(ex.validateExampleInResponse(&r, "response", path, method, code, op.ID)) //#nosec } } } else if op.ID != "" { // Empty op.ID means there is no meaningful operation: no need to report a specific message - res.AddErrors(noValidResponseMsg(op.ID)) + res.addErrorsAt(operationPath(path, method), noValidResponseMsg(op.ID)) } } } @@ -136,13 +134,15 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result { // reset explored schemas to get depth-first recursive-proof exploration ex.resetVisited() for nm, sch := range s.spec.Spec().Definitions { - res.Merge(ex.validateExampleValueSchemaAgainstSchema("definitions."+nm, "body", &sch)) //#nosec + res.Merge(ex.validateExampleValueSchemaAgainstSchema(newPathSegments(swaggerDefinitions, nm), "body", &sch)) //#nosec } } return res } -func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, responseType, path string, responseCode int, operationID string) *Result { +func (ex *exampleValidator) validateExampleInResponse( + resp *spec.Response, responseType, path, method string, responseCode int, operationID string, +) *Result { s := ex.SpecValidator response, res := responseHelp.expandResponseRef(resp, path, s) @@ -160,7 +160,7 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo if h.Example != nil { red := newHeaderValidator(nm, &h, s.KnownFormats, ex.schemaOptions).Validate(h.Example) //#nosec if red.HasErrorsOrWarnings() { - res.AddWarnings(exampleValueHeaderDoesNotValidateMsg(operationID, nm, responseName)) + res.addWarningsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), exampleValueHeaderDoesNotValidateMsg(operationID, nm, responseName)) res.MergeAsWarnings(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -169,9 +169,9 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo // Headers have inline definition, like params if h.Items != nil { - red := ex.validateExampleValueItemsAgainstSchema(nm, "header", &h, h.Items) //#nosec + red := ex.validateExampleValueItemsAgainstSchema(responseHeaderPath(path, method, responseCodeAsStr, nm), "header", &h, h.Items) //#nosec if red.HasErrorsOrWarnings() { - res.AddWarnings(exampleValueHeaderItemsDoesNotValidateMsg(operationID, nm, responseName)) + res.addWarningsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), exampleValueHeaderItemsDoesNotValidateMsg(operationID, nm, responseName)) res.MergeAsWarnings(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -179,7 +179,7 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo } if _, err := compileRegexp(h.Pattern); err != nil { - res.AddErrors(invalidPatternInHeaderMsg(operationID, nm, responseName, h.Pattern, err)) + res.addErrorsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), invalidPatternInHeaderMsg(operationID, nm, responseName, h.Pattern, err)) } // Headers don't have schema @@ -189,10 +189,11 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo // reset explored schemas to get depth-first recursive-proof exploration ex.resetVisited() - red := ex.validateExampleValueSchemaAgainstSchema(responseCodeAsStr, "response", response.Schema) + red := ex.validateExampleValueSchemaAgainstSchema( + responsePath(path, method, responseCodeAsStr).structuralChild(jsonSchema), "response", response.Schema) if red.HasErrorsOrWarnings() { // Additional message to make sure the context of the error is not lost - res.AddWarnings(exampleValueInDoesNotValidateMsg(operationID, responseName)) + res.addWarningsAt(responsePath(path, method, responseCodeAsStr), exampleValueInDoesNotValidateMsg(operationID, responseName)) res.Merge(red) } else if red.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(red) @@ -201,22 +202,26 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo if response.Examples != nil { if response.Schema != nil { - if example, ok := response.Examples["application/json"]; ok { + if example, ok := response.Examples[jsonMimeApplicationJSON]; ok { + exampleAt := responsePath(path, method, responseCodeAsStr). + child(swaggerExamples). + structuralChild(jsonMimeApplicationJSON) res.MergeAsWarnings( - newSchemaValidator(response.Schema, s.spec.Spec(), path+".examples", s.KnownFormats, s.schemaOptions).Validate(example), + newSchemaValidator(response.Schema, s.spec.Spec(), + exampleAt, s.KnownFormats, s.schemaOptions).Validate(example), ) } else { // Proposal for enhancement: validate other media types too - res.AddWarnings(examplesMimeNotSupportedMsg(operationID, responseName)) + res.addWarningsAt(responsePath(path, method, responseCodeAsStr).child(swaggerExamples), examplesMimeNotSupportedMsg(operationID, responseName)) } } else { - res.AddWarnings(examplesWithoutSchemaMsg(operationID, responseName)) + res.addWarningsAt(responsePath(path, method, responseCodeAsStr).child(swaggerExamples), examplesWithoutSchemaMsg(operationID, responseName)) } } return res } -func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in string, schema *spec.Schema) *Result { +func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path pathSegments, in string, schema *spec.Schema) *Result { if schema == nil || ex.isVisited(path) { // Avoids recursing if we are already done with that check return nil @@ -227,39 +232,39 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str if schema.Example != nil { res.MergeAsWarnings( - newSchemaValidator(schema, s.spec.Spec(), path+".example", s.KnownFormats, ex.schemaOptions).Validate(schema.Example), + newSchemaValidator(schema, s.spec.Spec(), path.child(swaggerExample), s.KnownFormats, ex.schemaOptions).Validate(schema.Example), ) } if schema.Items != nil { if schema.Items.Schema != nil { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(path+".items.example", in, schema.Items.Schema)) + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.child(jsonItems), in, schema.Items.Schema)) } // Multiple schemas in items if schema.Items.Schemas != nil { // Safeguard for i, sch := range schema.Items.Schemas { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(fmt.Sprintf("%s.items[%d].example", path, i), in, &sch)) //#nosec + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.child(jsonItems).item(i), in, &sch)) //#nosec } } } if _, err := compileRegexp(schema.Pattern); err != nil { - res.AddErrors(invalidPatternInMsg(path, in, schema.Pattern)) + res.addErrorsAt(path, invalidPatternInMsg(path.dotted(), in, schema.Pattern)) } if schema.AdditionalItems != nil && schema.AdditionalItems.Schema != nil { // NOTE: we keep validating values, even though additionalItems is unsupported in Swagger 2.0 (and 3.0 as well) - res.Merge(ex.validateExampleValueSchemaAgainstSchema(path+".additionalItems", in, schema.AdditionalItems.Schema)) + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.child(jsonAdditionalItems), in, schema.AdditionalItems.Schema)) } for propName, prop := range schema.Properties { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(path+"."+propName, in, &prop)) //#nosec + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.structuralChild(jsonProperties).child(propName), in, &prop)) //#nosec } for propName, prop := range schema.PatternProperties { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(path+"."+propName, in, &prop)) //#nosec + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.structuralChild(jsonPatternProperties).child(propName), in, &prop)) //#nosec } if schema.AdditionalProperties != nil && schema.AdditionalProperties.Schema != nil { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(path+".additionalProperties", in, schema.AdditionalProperties.Schema)) + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.child(jsonAdditionalProperties), in, schema.AdditionalProperties.Schema)) } if schema.AllOf != nil { for i, aoSch := range schema.AllOf { - res.Merge(ex.validateExampleValueSchemaAgainstSchema(fmt.Sprintf("%s.allOf[%d]", path, i), in, &aoSch)) //#nosec + res.Merge(ex.validateExampleValueSchemaAgainstSchema(path.child(jsonAllOf).item(i), in, &aoSch)) //#nosec } } return res @@ -268,7 +273,7 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str // NOTE: Temporary duplicated code. Need to refactor with examples // -func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in string, root any, items *spec.Items) *Result { +func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path pathSegments, in string, root any, items *spec.Items) *Result { res := pools.poolOfResults.BorrowResult() s := ex.SpecValidator if items != nil { @@ -278,10 +283,10 @@ func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in stri ) } if items.Items != nil { - res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items)) + res.Merge(ex.validateExampleValueItemsAgainstSchema(path.item(0), in, root, items.Items)) } if _, err := compileRegexp(items.Pattern); err != nil { - res.AddErrors(invalidPatternInMsg(path, in, items.Pattern)) + res.addErrorsAt(path, invalidPatternInMsg(path.dotted(), in, items.Pattern)) } } diff --git a/formats.go b/formats.go index eab2615..a050b39 100644 --- a/formats.go +++ b/formats.go @@ -11,14 +11,14 @@ import ( ) type formatValidator struct { - Path string + Path pathSegments In string Format string KnownFormats strfmt.Registry Options *SchemaValidatorOptions } -func newFormatValidator(path, in, format string, formats strfmt.Registry, opts *SchemaValidatorOptions) *formatValidator { +func newFormatValidator(path pathSegments, in, format string, formats strfmt.Registry, opts *SchemaValidatorOptions) *formatValidator { if opts == nil { opts = new(SchemaValidatorOptions) } @@ -39,10 +39,6 @@ func newFormatValidator(path, in, format string, formats strfmt.Registry, opts * return f } -func (f *formatValidator) SetPath(path string) { - f.Path = path -} - func (f *formatValidator) Applies(source any, kind reflect.Kind) bool { if source == nil || f.KnownFormats == nil { return false @@ -81,13 +77,17 @@ func (f *formatValidator) Validate(val any) *Result { return result } - if err := FormatOf(f.Path, f.In, f.Format, str, f.KnownFormats); err != nil { + if err := FormatOf(f.Path.dotted(), f.In, f.Format, str, f.KnownFormats); err != nil { result.AddErrors(err) } return result } +func (f *formatValidator) setPath(path pathSegments) { + f.Path = path +} + func (f *formatValidator) redeem() { pools.poolOfFormatValidators.RedeemValidator(f) } diff --git a/formats_test.go b/formats_test.go index 3af44c1..b21dad7 100644 --- a/formats_test.go +++ b/formats_test.go @@ -17,9 +17,9 @@ import ( func TestFormatValidator_EdgeCases(t *testing.T) { // Apply v := newFormatValidator( - "", "", "", strfmt.Default, nil, + nil, "", "", strfmt.Default, nil, ) - v.SetPath("a.b.c") + v.setPath(newPathSegments("a", "b", "c")) // formatValidator applies to: Items, Parameter,Schema diff --git a/go.mod b/go.mod index 74fe533..230ce38 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/go-openapi/validate require ( - github.com/go-openapi/analysis v0.25.5 + github.com/go-openapi/analysis v0.26.0 github.com/go-openapi/errors v0.22.8 github.com/go-openapi/jsonpointer v1.0.0 github.com/go-openapi/loads v0.25.0 @@ -18,7 +18,7 @@ require ( require ( github.com/go-openapi/jsonreference v1.0.0 // indirect - github.com/go-openapi/swag/mangling v0.27.3 // indirect + github.com/go-openapi/swag/mangling v0.28.0 // indirect github.com/go-openapi/swag/pools v0.28.0 // indirect github.com/go-openapi/swag/typeutils v0.28.0 // indirect github.com/go-openapi/swag/yamlutils v0.28.0 // indirect diff --git a/go.sum b/go.sum index 1991b83..36049fc 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/go-openapi/analysis v0.25.5 h1:xPYEvTb90o1y0epuiOPAoG4QqahjP3cdp5xNlHeKJRI= -github.com/go-openapi/analysis v0.25.5/go.mod h1:d3UGtQC5uq5Kqqqis2VH09Km/v3vwsWrYkbp4gdm+Rc= +github.com/go-openapi/analysis v0.26.0 h1:1xECln1iMMmQnTjgcknC1vi1hA4KISt6IHpSwnqcuwI= +github.com/go-openapi/analysis v0.26.0/go.mod h1:40gERFi/2dyXA1FaqRRLxkv1IlC6X+GPDNd1xrYAjZE= github.com/go-openapi/errors v0.22.8 h1:oP7sW7TWc3wFFjrzzj0nI83H2qMBkNjNfSd+XRejk/I= github.com/go-openapi/errors v0.22.8/go.mod h1:BuUoHcYrU6E7V9gfj1I5wLQqgtIHnup/alXZ8KdgQ0w= github.com/go-openapi/jsonpointer v1.0.0 h1:kR9tHqY0CtZaOPVFm622dPVNhrvYpwr4uCxgL3h1H8s= @@ -22,8 +22,8 @@ github.com/go-openapi/swag/jsonutils/fixtures_test v0.28.0 h1:qV+VVUAx5Oro8WjVWp github.com/go-openapi/swag/jsonutils/fixtures_test v0.28.0/go.mod h1:mofwUWx70wvskwESqRJ//k/9kURmCgyJl5m5Ppoh5kY= github.com/go-openapi/swag/loading v0.28.0 h1:td8QZdZC9MIYGGSnSPKShKiK22I2tU5UQvuUhIBPRLU= github.com/go-openapi/swag/loading v0.28.0/go.mod h1:rXB0QiQX5mMveXEA7ouM4KiiM9jVJe4K6BVbwhD1M4k= -github.com/go-openapi/swag/mangling v0.27.3 h1:gRzzD1PAUoLTtGMgI3KpBmCSOlTuLTFWnviLxLcTnyg= -github.com/go-openapi/swag/mangling v0.27.3/go.mod h1:jtBE2+V+3pILxOR7Vgce+Cwp6A2PgZbvVqfNntbVs0w= +github.com/go-openapi/swag/mangling v0.28.0 h1:pH8eyeNO9SLYsTMWJrurnNfKmDa28XrlA+HePVD53VM= +github.com/go-openapi/swag/mangling v0.28.0/go.mod h1:jtBE2+V+3pILxOR7Vgce+Cwp6A2PgZbvVqfNntbVs0w= github.com/go-openapi/swag/pools v0.28.0 h1:HPMZWSAfce3rdVTFcjFiCIBtDg9h4x2QlRrHipwhxeU= github.com/go-openapi/swag/pools v0.28.0/go.mod h1:kVQefhSK5RWuRe7BXsL8htgBPAMpN7HDGpGEknqugeE= github.com/go-openapi/swag/stringutils v0.28.0 h1:ixsc9iYgDPubHL/8nSkbnryEHpD2VRlBMLKpQyPXcDU= diff --git a/helpers.go b/helpers.go index 7cc254e..027f134 100644 --- a/helpers.go +++ b/helpers.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/go-openapi/errors" + "github.com/go-openapi/jsonpointer" "github.com/go-openapi/spec" ) @@ -33,13 +34,74 @@ const ( ) const ( - jsonProperties = "properties" - jsonItems = "items" - jsonType = "type" - // jsonSchema = "schema". - jsonDefault = "default" + jsonProperties = "properties" + jsonPatternProperties = "patternProperties" + jsonItems = "items" + jsonType = "type" + jsonSchema = "schema" + jsonRequired = "required" + jsonRef = "$ref" + jsonDefault = "default" + + jsonAllOf = "allOf" + jsonAdditionalItems = "additionalItems" + jsonAdditionalProperties = "additionalProperties" + + swaggerPaths = "paths" + swaggerDefinitions = "definitions" + swaggerResponses = "responses" + swaggerParameters = "parameters" + swaggerHeaders = "headers" + + jsonMimeApplicationJSON = "application/json" ) +// operationPath locates an operation in the spec document. +func operationPath(path, method string) pathSegments { + return newPathSegments(swaggerPaths, path, methodToken(method)) +} + +// parameterPath locates a parameter of an operation in the spec document. +func (s *SpecValidator) parameterPath(path, method, in, name string) pathSegments { + return s.paramLocations.at(path, method, in, name) +} + +// responsePath locates a response of an operation in the spec document. +func responsePath(path, method, responseCode string) pathSegments { + return operationPath(path, method).children(swaggerResponses, responseCode) +} + +// responseHeaderPath locates a header declared by a response. +func responseHeaderPath(path, method, responseCode, header string) pathSegments { + return responsePath(path, method, responseCode).children(swaggerHeaders, header) +} + +// methodToken normalizes an HTTP method into the key under which the operation +// is found in the document: the analyzer hands them over in upper case, but a +// path item spells them in lower case. +func methodToken(method string) string { + return strings.ToLower(method) +} + +// localRefPath turns a local JSON reference such as "#/definitions/Pet" into +// the location of what it points to. +// +// It yields the document root for anything that does not address a local +// fragment, a remote reference in particular. +func localRefPath(ref string) pathSegments { + rest, isLocal := strings.CutPrefix(ref, "#/") + if !isLocal { + return rootPath() + } + + tokens := strings.Split(rest, "/") + for i, token := range tokens { + tokens[i] = jsonpointer.Unescape(token) + } + + return newPathSegments(tokens...) +} + const ( stringFormatDate = "date" stringFormatDateTime = "date-time" @@ -91,24 +153,33 @@ type errorHelper struct { } func (h *errorHelper) sErr(err errors.Error, recycle bool) *Result { - // Builds a Result from standard errors.Error + return h.sErrAt(nil, err, recycle) +} + +// sErrAt builds a Result from a standard errors.Error reported at a known location. +func (h *errorHelper) sErrAt(at pathSegments, err errors.Error, recycle bool) *Result { var result *Result if recycle { result = pools.poolOfResults.BorrowResult() } else { result = new(Result) } - result.Errors = []error{err} + result.addErrorsAt(at, err) return result } func (h *errorHelper) addPointerError(res *Result, err error, ref string, fromPath string) *Result { - // Provides more context on error messages - // reported by the jsoinpointer package by altering the passed Result + return h.addPointerErrorAt(res, nil, err, ref, fromPath) +} + +// addPointerErrorAt provides more context on error messages reported by the +// jsonpointer package, by altering the passed Result. +func (h *errorHelper) addPointerErrorAt(res *Result, at pathSegments, err error, ref string, fromPath string) *Result { if err != nil { - res.AddErrors(cannotResolveRefMsg(fromPath, ref, err)) + res.addErrorsAt(at, cannotResolveRefMsg(fromPath, ref, err)) } + return res } @@ -225,9 +296,9 @@ func (h *paramHelper) safeExpandedParamsFor(path, method, operationID string, re for _, ppr := range s.expandedAnalyzer().SafeParamsFor(method, path, func(_ spec.Parameter, err error) bool { // since params have already been expanded, there are few causes for error - res.AddErrors(someParametersBrokenMsg(path, method, operationID)) + res.addErrorsAt(operationPath(path, method), someParametersBrokenMsg(path, method, operationID)) // original error from analyzer - res.AddErrors(err) + res.addErrorsAt(operationPath(path, method), err) return true }) { params = append(params, ppr) @@ -249,14 +320,16 @@ func (h *paramHelper) resolveParam(path, method, operationID string, param *spec if err != nil { // Safeguard // NOTE: we may enter here when the whole parameter is an unresolved $ref refPath := strings.Join([]string{"\"" + path + "\"", method}, ".") - errorHelp.addPointerError(res, err, param.Ref.String(), refPath) + errorHelp.addPointerErrorAt(res, s.parameterPath(path, method, param.In, param.Name), err, param.Ref.String(), refPath) return nil, res } - res.Merge(h.checkExpandedParam(param, param.Name, param.In, operationID, isRef)) + res.Merge(h.checkExpandedParam(param, param.Name, param.In, operationID, s.parameterPath(path, method, param.In, param.Name), isRef)) return param, res } -func (h *paramHelper) checkExpandedParam(pr *spec.Parameter, path, in, operation string, isRef bool) *Result { +func (h *paramHelper) checkExpandedParam( + pr *spec.Parameter, path, in, operation string, at pathSegments, isRef bool, +) *Result { // Secure parameter structure after $ref resolution res := new(Result) simpleZero := spec.SimpleSchema{} @@ -267,17 +340,17 @@ func (h *paramHelper) checkExpandedParam(pr *spec.Parameter, path, in, operation // Most likely, a $ref with a sibling is an unwanted situation: in itself this is a warning... // but we detect it because of the following error: // schema took over Parameter for an unexplained reason - res.AddWarnings(refShouldNotHaveSiblingsMsg(path, operation)) + res.addWarningsAt(at, refShouldNotHaveSiblingsMsg(path, operation)) } - res.AddErrors(invalidParameterDefinitionMsg(path, in, operation)) + res.addErrorsAt(at, invalidParameterDefinitionMsg(path, in, operation)) case pr.In != swaggerBody && pr.Schema != nil: if isRef { - res.AddWarnings(refShouldNotHaveSiblingsMsg(path, operation)) + res.addWarningsAt(at, refShouldNotHaveSiblingsMsg(path, operation)) } - res.AddErrors(invalidParameterDefinitionAsSchemaMsg(path, in, operation)) + res.addErrorsAt(at, invalidParameterDefinitionAsSchemaMsg(path, in, operation)) case (pr.In == swaggerBody && pr.Schema == nil) || (pr.In != swaggerBody && pr.SimpleSchema == simpleZero): // Other unexpected mishaps - res.AddErrors(invalidParameterDefinitionMsg(path, in, operation)) + res.addErrorsAt(at, invalidParameterDefinitionMsg(path, in, operation)) } return res } diff --git a/items_validator_test.go b/items_validator_test.go index d8abd3d..56e95d1 100644 --- a/items_validator_test.go +++ b/items_validator_test.go @@ -106,7 +106,7 @@ func TestNumberItemsValidation(t *testing.T) { items.Typed("integer", "int32") parent := spec.QueryParam("factors").CollectionOf(items, "") path := fmt.Sprintf("factors.%d", i) - validator := newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator := newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) // MultipleOf err := validator.Validate(i, v[0]) @@ -125,7 +125,7 @@ func TestNumberItemsValidation(t *testing.T) { // ExclusiveMaximum items.ExclusiveMaximum = true // requires a new items validator because this is set a creation time - validator = newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator = newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) err = validator.Validate(i, v[1]) assert.TrueT(t, err.HasErrors()) require.NotEmpty(t, err.Errors) @@ -142,7 +142,7 @@ func TestNumberItemsValidation(t *testing.T) { // ExclusiveMinimum items.ExclusiveMinimum = true // requires a new items validator because this is set a creation time - validator = newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator = newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) err = validator.Validate(i, v[3]) assert.TrueT(t, err.HasErrors()) require.NotEmpty(t, err.Errors) @@ -165,7 +165,7 @@ func TestStringItemsValidation(t *testing.T) { items.WithEnum("aaa", "bbb", "ccc") parent := spec.QueryParam("tags").CollectionOf(items, "") path := parent.Name + ".1" - validator := newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator := newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) // required data := "" @@ -212,7 +212,7 @@ func TestArrayItemsValidation(t *testing.T) { items.WithEnum("aaa", "bbb", "ccc") parent := spec.QueryParam("tags").CollectionOf(items, "") path := parent.Name + ".1" - validator := newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator := newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) // MinItems data := []string{} @@ -242,7 +242,7 @@ func TestArrayItemsValidation(t *testing.T) { // Items strItems := spec.NewItems().WithMinLength(3).WithMaxLength(5).WithPattern(`^[a-z]+$`).Typed(stringType, "") items = spec.NewItems().CollectionOf(strItems, "").WithMinItems(1).WithMaxItems(5).UniqueValues() - validator = newItemsValidator(parent.Name, parent.In, items, parent, strfmt.Default, nil) + validator = newItemsValidator(newPathSegments(parent.Name), parent.In, items, parent, strfmt.Default, nil) data = []string{"aa", "bbb", "ccc"} err = validator.Validate(1, data) diff --git a/object_validator.go b/object_validator.go index e651b3f..2cc8dce 100644 --- a/object_validator.go +++ b/object_validator.go @@ -4,7 +4,6 @@ package validate import ( - "fmt" "reflect" "strings" @@ -14,7 +13,7 @@ import ( ) type objectValidator struct { - Path string + Path pathSegments In string MaxProperties *int64 MinProperties *int64 @@ -25,10 +24,9 @@ type objectValidator struct { Root any KnownFormats strfmt.Registry Options *SchemaValidatorOptions - splitPath []string } -func newObjectValidator(path, in string, +func newObjectValidator(path pathSegments, in string, maxProperties, minProperties *int64, required []string, properties spec.SchemaProperties, additionalProperties *spec.SchemaOrBool, patternProperties spec.SchemaProperties, root any, formats strfmt.Registry, opts *SchemaValidatorOptions, @@ -55,7 +53,6 @@ func newObjectValidator(path, in string, v.Root = root v.KnownFormats = formats v.Options = opts - v.splitPath = strings.Split(v.Path, ".") return v } @@ -72,16 +69,16 @@ func (o *objectValidator) Validate(data any) *Result { var ok bool val, ok = data.(map[string]any) if !ok { - return errorHelp.sErr(invalidObjectMsg(o.Path, o.In), o.Options.recycleResult) + return errorHelp.sErrAt(o.Path, invalidObjectMsg(o.Path.dotted(), o.In), o.Options.recycleResult) } } numKeys := int64(len(val)) if o.MinProperties != nil && numKeys < *o.MinProperties { - return errorHelp.sErr(errors.TooFewProperties(o.Path, o.In, *o.MinProperties), o.Options.recycleResult) + return errorHelp.sErrAt(o.Path, errors.TooFewProperties(o.Path.dotted(), o.In, *o.MinProperties), o.Options.recycleResult) } if o.MaxProperties != nil && numKeys > *o.MaxProperties { - return errorHelp.sErr(errors.TooManyProperties(o.Path, o.In, *o.MaxProperties), o.Options.recycleResult) + return errorHelp.sErrAt(o.Path, errors.TooManyProperties(o.Path.dotted(), o.In, *o.MaxProperties), o.Options.recycleResult) } var res *Result @@ -115,7 +112,7 @@ func (o *objectValidator) Validate(data any) *Result { for _, pName := range patterns { if v, ok := o.PatternProperties[pName]; ok { - r := newSchemaValidator(&v, o.Root, o.Path+"."+key, o.KnownFormats, o.Options).Validate(value) + r := newSchemaValidator(&v, o.Root, o.Path.child(key), o.KnownFormats, o.Options).Validate(value) res.mergeForField(data.(map[string]any), key, r) //nolint:forcetypeassert // data is always map[string]any at this point } } @@ -124,11 +121,6 @@ func (o *objectValidator) Validate(data any) *Result { return res } -func (o *objectValidator) SetPath(path string) { - o.Path = path - o.splitPath = strings.Split(path, ".") -} - func (o *objectValidator) Applies(source any, kind reflect.Kind) bool { // NOTE: this should also work for structs // there is a problem in the type validator where it will be unhappy about null values @@ -137,19 +129,29 @@ func (o *objectValidator) Applies(source any, kind reflect.Kind) bool { return isSchema && (kind == reflect.Map || kind == reflect.Struct) } +// The three predicates below tell what kind of content the validated object +// is, so that schema-only checks are not run against plain data. +// +// Array indices are trimmed first: an element of an example is example data +// just as much as the example itself. + func (o *objectValidator) isProperties() bool { - p := o.splitPath - return len(p) > 1 && p[len(p)-1] == jsonProperties && p[len(p)-2] != jsonProperties + p := o.Path.trimIndexes() + + return p.last() == jsonProperties && p.beforeLast() != jsonProperties } func (o *objectValidator) isDefault() bool { - p := o.splitPath - return len(p) > 1 && p[len(p)-1] == jsonDefault && p[len(p)-2] != jsonDefault + p := o.Path.trimIndexes() + + return p.last() == jsonDefault && p.beforeLast() != jsonDefault } func (o *objectValidator) isExample() bool { - p := o.splitPath - return len(p) > 1 && (p[len(p)-1] == swaggerExample || p[len(p)-1] == swaggerExamples) && p[len(p)-2] != swaggerExample + p := o.Path.trimIndexes() + last := p.last() + + return (last == swaggerExample || last == swaggerExamples) && p.beforeLast() != swaggerExample } func (o *objectValidator) checkArrayMustHaveItems(res *Result, val map[string]any) { @@ -174,7 +176,7 @@ func (o *objectValidator) checkArrayMustHaveItems(res *Result, val map[string]an return } - res.AddErrors(errors.Required(jsonItems, o.Path, item)) + res.addErrorsAt(o.Path, errors.Required(jsonItems, o.Path.dotted(), item)) } func (o *objectValidator) checkItemsMustBeTypeArray(res *Result, val map[string]any) { @@ -194,11 +196,11 @@ func (o *objectValidator) checkItemsMustBeTypeArray(res *Result, val map[string] t, typeFound := val[jsonType] if !typeFound { // there is no type - res.AddErrors(errors.Required(jsonType, o.Path, t)) + res.addErrorsAt(o.Path, errors.Required(jsonType, o.Path.dotted(), t)) } if tpe, isString := t.(string); !isString || tpe != arrayType { - res.AddErrors(errors.InvalidType(o.Path, o.In, arrayType, nil)) + res.addErrorsAt(o.Path, errors.InvalidType(o.Path.dotted(), o.In, arrayType, nil)) } } @@ -238,7 +240,7 @@ func (o *objectValidator) validateNoAdditionalProperties(val map[string]any, res continue } - res.AddErrors(errors.PropertyNotAllowed(o.Path, o.In, k)) + res.addErrorsAt(o.Path.child(k), errors.PropertyNotAllowed(o.Path.dotted(), o.In, k)) // BUG(fredbi): This section should move to a part dedicated to spec validation as // it will conflict with regular schemas where a property "headers" is defined. @@ -282,7 +284,7 @@ func (o *objectValidator) validateNoAdditionalProperties(val map[string]any, res } msg := strings.Join([]string{", one may not use $ref=\":", refString, "\""}, "") - res.AddErrors(refNotAllowedInHeaderMsg(o.Path, headerKey, msg)) + res.addErrorsAt(o.Path, refNotAllowedInHeaderMsg(o.Path.dotted(), headerKey, msg)) /* case "$ref": if val[k] != nil { @@ -315,7 +317,7 @@ func (o *objectValidator) validateAdditionalProperties(val map[string]any, res * // Cases: properties which are not regular properties and have not been matched by the PatternProperties validator // AdditionalProperties as Schema - r := newSchemaValidator(o.AdditionalProperties.Schema, o.Root, o.Path+"."+key, o.KnownFormats, o.Options).Validate(value) + r := newSchemaValidator(o.AdditionalProperties.Schema, o.Root, o.Path.child(key), o.KnownFormats, o.Options).Validate(value) res.mergeForField(val, key, r) } // Valid cases: additionalProperties: true or undefined @@ -333,12 +335,7 @@ func (o *objectValidator) validatePropertiesSchema(val map[string]any, res *Resu for pName := range o.Properties { *pSchema = o.Properties[pName] - var rName string - if o.Path == "" { - rName = pName - } else { - rName = o.Path + "." + pName - } + rName := o.Path.child(pName) // Recursively validates each property against its schema v, ok := val[pName] @@ -374,7 +371,9 @@ func (o *objectValidator) validatePropertiesSchema(val map[string]any, res *Resu continue } - res.AddErrors(errors.Required(fmt.Sprintf("%s.%s", o.Path, k), o.In, v)) + // located on the object that lacks the property: the property itself + // has no node to point at, and the object is what has to be amended + res.addErrorsAt(o.Path, errors.Required(o.Path.child(k).dotted(), o.In, v)) } } @@ -407,7 +406,7 @@ func (o *objectValidator) validatePatternProperty(key string, value any, result *schema = o.PatternProperties[k] patterns = append(patterns, k) matched = true - validator := newSchemaValidator(schema, o.Root, fmt.Sprintf("%s.%s", o.Path, key), o.KnownFormats, o.Options) + validator := newSchemaValidator(schema, o.Root, o.Path.child(key), o.KnownFormats, o.Options) res := validator.Validate(value) result.Merge(res) @@ -416,6 +415,10 @@ func (o *objectValidator) validatePatternProperty(key string, value any, result return matched, succeededOnce, patterns } +func (o *objectValidator) setPath(path pathSegments) { + o.Path = path +} + func (o *objectValidator) redeem() { pools.poolOfObjectValidators.RedeemValidator(o) } diff --git a/object_validator_test.go b/object_validator_test.go index be31db8..86af1ee 100644 --- a/object_validator_test.go +++ b/object_validator_test.go @@ -27,6 +27,9 @@ import ( "github.com/go-openapi/testify/v2/require" ) +// wantedProp is the name of the required property exercised by these tests. +const wantedProp = "wanted" + func itemsFixture() map[string]any { return map[string]any{ "type": "array", @@ -51,7 +54,7 @@ func expectOnlyInvalid(t *testing.T, ov EntityValidator, dataValid, dataInvalid } func TestItemsMustBeTypeArray(t *testing.T) { - ov := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + ov := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) dataValid := itemsFixture() dataInvalid := map[string]any{ "type": "object", @@ -64,7 +67,7 @@ func TestItemsMustBeTypeArray(t *testing.T) { } func TestItemsMustHaveType(t *testing.T) { - ov := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + ov := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) dataValid := itemsFixture() dataInvalid := map[string]any{ "items": "dummy", @@ -76,7 +79,7 @@ func TestItemsMustHaveType(t *testing.T) { } func TestTypeArrayMustHaveItems(t *testing.T) { - ov := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + ov := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) dataValid := itemsFixture() dataInvalid := map[string]any{ "type": "array", @@ -92,13 +95,13 @@ func TestTypeArrayMustHaveItems(t *testing.T) { // to simulate with specs // (this one is a trivial, just to check all methods are filled). func TestObjectValidator_EdgeCases(t *testing.T) { - s := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) - s.SetPath("path") - assert.EqualT(t, "path", s.Path) + s := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + s.setPath(newPathSegments("path")) + assert.EqualT(t, "path", s.Path.dotted()) } func TestObjectValidatorApply(t *testing.T) { - s := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + s := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) require.TrueT(t, s.Applies(&spec.Schema{}, reflect.Map)) require.FalseT(t, s.Applies(&spec.Response{}, reflect.Map)) require.FalseT(t, s.Applies(&struct{}{}, reflect.Map)) @@ -127,7 +130,7 @@ func TestObjectValidatorPatternProperties(t *testing.T) { } t.Run("should ignore invalid regexp in pattern properties", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, nil, patternWithValid, nil, nil, nil) + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, nil, patternWithValid, nil, nil, nil) res := s.Validate(map[string]any{"valid": "test_string"}) require.NotNil(t, res) @@ -135,7 +138,7 @@ func TestObjectValidatorPatternProperties(t *testing.T) { }) t.Run("should report forbidden property when invalid regexp in pattern properties", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, nil, patternGarbled, nil, nil, nil) + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, nil, patternGarbled, nil, nil, nil) res := s.Validate(map[string]any{"valid": "test_string"}) require.NotNil(t, res) @@ -143,7 +146,7 @@ func TestObjectValidatorPatternProperties(t *testing.T) { }) t.Run("should ignore invalid regexp in pattern properties of additional properties", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, &spec.SchemaOrBool{ + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, &spec.SchemaOrBool{ Schema: &spec.Schema{}, Allows: false, }, patternWithValid, nil, nil, nil) @@ -154,7 +157,7 @@ func TestObjectValidatorPatternProperties(t *testing.T) { }) t.Run("should report forbidden property when invalid regexp in pattern properties of additional properties", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, &spec.SchemaOrBool{ + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, &spec.SchemaOrBool{ Schema: &spec.Schema{}, Allows: false, }, patternGarbled, nil, nil, nil) @@ -168,7 +171,7 @@ func TestObjectValidatorPatternProperties(t *testing.T) { func TestObjectValidatorNilData(t *testing.T) { t.Run("object Validate should NOT panic on nil data", func(t *testing.T) { - s := newObjectValidator("", "", nil, nil, nil, nil, nil, nil, nil, nil, nil) + s := newObjectValidator(nil, "", nil, nil, nil, nil, nil, nil, nil, nil, nil) require.NotPanics(t, func() { _ = s.Validate(nil) }) @@ -179,35 +182,35 @@ func TestObjectValidatorNilData(t *testing.T) { }) t.Run("object Validate should validate required on nil data", func(t *testing.T) { - s := newObjectValidator("", "", nil, nil, []string{"wanted"}, nil, nil, nil, nil, nil, nil) + s := newObjectValidator(nil, "", nil, nil, []string{wantedProp}, nil, nil, nil, nil, nil, nil) res := s.Validate(nil) require.NotNil(t, res) require.NotEmpty(t, res.Errors) }) t.Run("object Validate should NOT panic on unexpected input", func(t *testing.T) { - s := newObjectValidator("", "", nil, nil, []string{"wanted"}, nil, nil, nil, nil, nil, nil) - res := s.Validate(map[string]string{"wanted": "not expected"}) + s := newObjectValidator(nil, "", nil, nil, []string{wantedProp}, nil, nil, nil, nil, nil, nil) + res := s.Validate(map[string]string{wantedProp: "not expected"}) require.NotNil(t, res) require.Len(t, res.Errors, 1) require.ErrorContains(t, res.Errors[0], "expected an object") }) t.Run("object Validate should NOT panic on nil input (with array type check)", func(t *testing.T) { - s := newObjectValidator("", "", nil, nil, []string{"wanted"}, nil, nil, nil, nil, nil, &SchemaValidatorOptions{ + s := newObjectValidator(nil, "", nil, nil, []string{wantedProp}, nil, nil, nil, nil, nil, &SchemaValidatorOptions{ EnableArrayMustHaveItemsCheck: true, EnableObjectArrayTypeCheck: true, }) res := s.Validate(nil) require.NotNil(t, res) require.Len(t, res.Errors, 1) - require.ErrorContains(t, res.Errors[0], "wanted is required") + require.ErrorContains(t, res.Errors[0], wantedProp+" is required") }) } func TestObjectValidatorWithHeaderProperty(t *testing.T) { t.Run("should report extra information about forbidden $ref in this context", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, &spec.SchemaOrBool{ + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, &spec.SchemaOrBool{ Schema: &spec.Schema{}, Allows: false, }, nil, nil, nil, nil) @@ -234,7 +237,7 @@ func TestObjectValidatorWithHeaderProperty(t *testing.T) { }) t.Run("should NOT report extra information when header is not detected", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, nil, nil, &spec.SchemaOrBool{ + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, nil, nil, &spec.SchemaOrBool{ Schema: &spec.Schema{}, Allows: false, }, nil, nil, nil, nil) @@ -314,10 +317,10 @@ func TestObjectValidatorWithDefault(t *testing.T) { root interface{}, formats strfmt.Registry, opts *SchemaValidatorOptions) *objectValidator { */ t.Run("should accept required populated with a default", func(t *testing.T) { - s := newObjectValidator("test", "body", nil, nil, - []string{"wanted"}, + s := newObjectValidator(newPathSegments("test"), "body", nil, nil, + []string{wantedProp}, spec.SchemaProperties{ - "wanted": spec.Schema{ + wantedProp: spec.Schema{ SchemaProps: spec.SchemaProps{ Default: "default_value", }, diff --git a/param_locations.go b/param_locations.go new file mode 100644 index 0000000..e8b4b22 --- /dev/null +++ b/param_locations.go @@ -0,0 +1,122 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "strconv" + + "github.com/go-openapi/spec" +) + +// paramLocations tells where an operation declares a parameter. +// +// Parameters are held in an array, so a name is not how a document addresses +// one: only its index is. The index is not something a validator can work out +// from an expanded parameter either, because expansion merges the parameters +// an operation declares with those its path item declares, and resolves the +// ones written as a $ref. So the unexpanded document is indexed once, and +// looked up by what a validator does know: the operation, and the name and +// location of the parameter it is reporting on. +type paramLocations map[paramKey]pathSegments + +// paramKey identifies a parameter the way the swagger specification does: +// a name is unique only within an "in". +type paramKey struct { + path string + method string + in string + name string +} + +// newParamLocations indexes the parameters declared by an unexpanded document. +func newParamLocations(sp *spec.Swagger) paramLocations { + locations := make(paramLocations) + if sp == nil || sp.Paths == nil { + return locations + } + + for path, pathItem := range sp.Paths.Paths { + at := newPathSegments(swaggerPaths, path) + + // parameters declared by the path item are shared by all its + // operations: recorded once, without a method + locations.collect(sp, paramKey{path: path}, at, pathItem.Parameters) + + for method, op := range operationsOf(&pathItem) { //#nosec + if op == nil { + continue + } + + locations.collect(sp, paramKey{path: path, method: method}, at.child(method), op.Parameters) + } + } + + return locations +} + +// at returns where an operation declares a parameter. +// +// A parameter the operation does not declare itself may come from its path +// item. When neither knows it, which happens for a parameter too broken to be +// identified, the name stands in for the index: the location no longer +// resolves, but it still says what it is about. +func (l paramLocations) at(path, method, in, name string) pathSegments { + if found, isDeclared := l[paramKey{path: path, method: methodToken(method), in: in, name: name}]; isDeclared { + return found + } + + if found, isDeclared := l[paramKey{path: path, in: in, name: name}]; isDeclared { + return found + } + + return operationPath(path, method).children(swaggerParameters, name) +} + +func (l paramLocations) collect(sp *spec.Swagger, key paramKey, at pathSegments, params []spec.Parameter) { + for i := range params { + name, in, ok := parameterIdentity(sp, ¶ms[i]) + if !ok { + continue + } + + key.name = name + key.in = in + l[key] = at.child(swaggerParameters).childAs(strconv.Itoa(i), name) + } +} + +// parameterIdentity names a parameter as declared, resolving the one indirection +// a document may put in the way: an entry written as a local $ref. +func parameterIdentity(sp *spec.Swagger, param *spec.Parameter) (name, in string, ok bool) { + if param.Ref.String() == "" { + return param.Name, param.In, param.Name != "" + } + + shared := localRefPath(param.Ref.String()) + const sharedParameterDepth = 2 + if len(shared) != sharedParameterDepth || shared.beforeLast() != swaggerParameters { + return "", "", false + } + + declared, isDeclared := sp.Parameters[shared.last()] + if !isDeclared { + return "", "", false + } + + return declared.Name, declared.In, declared.Name != "" +} + +// operationsOf yields the operations of a path item, keyed as the document +// spells them. +func operationsOf(pathItem *spec.PathItem) map[string]*spec.Operation { + return map[string]*spec.Operation{ + "get": pathItem.Get, + "put": pathItem.Put, + "post": pathItem.Post, + "delete": pathItem.Delete, + "options": pathItem.Options, + "head": pathItem.Head, + "patch": pathItem.Patch, + } +} diff --git a/param_locations_test.go b/param_locations_test.go new file mode 100644 index 0000000..2f7971a --- /dev/null +++ b/param_locations_test.go @@ -0,0 +1,175 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/go-openapi/loads" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +const paramLocationsFixture = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "parameters": { + "sharedBody": {"name": "user", "in": "body", "schema": {"type": "object"}} + }, + "paths": { + "/pets": { + "parameters": [ + {"name": "onPathItem", "in": "query", "type": "string"} + ], + "get": { + "operationId": "list", + "parameters": [ + {"name": "first", "in": "query", "type": "string"}, + {"name": "second", "in": "header", "type": "string"}, + {"$ref": "#/parameters/sharedBody"} + ], + "responses": {"200": {"description": "ok"}} + }, + "post": { + "operationId": "create", + "parameters": [{"name": "only", "in": "query", "type": "string"}], + "responses": {"200": {"description": "ok"}} + } + } + } +}` + +const ( + methodGet = "GET" + methodPost = "POST" + inQuery = "query" + inHeader = "header" +) + +func paramLocationsOf(t *testing.T) paramLocations { + t.Helper() + + d, err := loads.Analyzed(json.RawMessage(paramLocationsFixture), "") + require.NoError(t, err) + + return newParamLocations(d.Spec()) +} + +func TestParamLocations_AddressedByIndex(t *testing.T) { + t.Parallel() + + locations := paramLocationsOf(t) + + for _, tt := range []struct { + name string + in string + method string + pointer string + }{ + {name: "first", in: inQuery, method: methodGet, pointer: "/paths/~1pets/get/parameters/0"}, + {name: "second", in: inHeader, method: methodGet, pointer: "/paths/~1pets/get/parameters/1"}, + {name: "only", in: inQuery, method: methodPost, pointer: "/paths/~1pets/post/parameters/0"}, + } { + assert.EqualT(t, tt.pointer, locations.at("/pets", tt.method, tt.in, tt.name).pointer(), + "unexpected location for %q", tt.name) + } +} + +func TestParamLocations_FallsBackToThePathItem(t *testing.T) { + t.Parallel() + + locations := paramLocationsOf(t) + + // declared once by the path item, shared by every operation under it + for _, method := range []string{methodGet, methodPost} { + assert.EqualT(t, "/paths/~1pets/parameters/0", + locations.at("/pets", method, inQuery, "onPathItem").pointer(), + "unexpected location under %s", method) + } +} + +func TestParamLocations_ResolvesASharedParameter(t *testing.T) { + t.Parallel() + + locations := paramLocationsOf(t) + + // the entry is written as a $ref, so its name comes from the definition, + // while the location stays the site the operation declares it at + assert.EqualT(t, "/paths/~1pets/get/parameters/2", + locations.at("/pets", methodGet, "body", "user").pointer()) +} + +func TestParamLocations_UnknownParameterKeepsItsName(t *testing.T) { + t.Parallel() + + locations := paramLocationsOf(t) + + // a parameter too broken to be identified has no index to point at, so + // the name stands in: the location no longer resolves, but a message + // built from it still says what it is about + unknown := locations.at("/pets", methodGet, inQuery, "neverDeclared") + assert.EqualT(t, "/paths/~1pets/get/parameters/neverDeclared", unknown.pointer()) + assert.EqualT(t, "paths./pets.get.parameters.neverDeclared", unknown.dotted()) +} + +func TestParamLocations_PointerIndexesButMessageNames(t *testing.T) { + t.Parallel() + + locations := paramLocationsOf(t) + located := locations.at("/pets", methodGet, inHeader, "second") + + assert.EqualT(t, "/paths/~1pets/get/parameters/1", located.pointer(), + "the document addresses a parameter by index") + assert.EqualT(t, "paths./pets.get.parameters.second", located.dotted(), + "a message is more useful naming it") +} + +func TestParamLocations_ReportedThroughSpecValidation(t *testing.T) { + t.Parallel() + + // a query parameter typed as an array without items: reported against the + // entry the operation declares, which is what a document can address + const raw = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": { + "/pets": { + "get": { + "operationId": "list", + "parameters": [ + {"name": "ok", "in": "query", "type": "string"}, + {"name": "tags", "in": "query", "type": "array"} + ], + "responses": {"200": {"description": "ok"}} + } + } + } + }` + + doc, err := loads.Analyzed(json.RawMessage(raw), "") + require.NoError(t, err) + + validator := NewSpecValidator(doc.Schema(), strfmt.Default) + validator.SetContinueOnErrors(true) + res, _ := validator.Validate(doc) + + const declaredAt = "/paths/~1pets/get/parameters/1" + + var found bool + for _, located := range res.LocatedErrors() { + if !strings.Contains(located.Err.Error(), "tags") && !strings.HasPrefix(located.Pointer, declaredAt) { + continue + } + + found = true + // the check may report the parameter itself or something inside it, + // but never the operation or the array holding it + assert.StringContainsT(t, located.Pointer, declaredAt, + "expected %q to be located in the offending parameter", located.Err) + } + require.True(t, found, "expected the array parameter without items to be reported") +} diff --git a/path.go b/path.go new file mode 100644 index 0000000..7bbe2ec --- /dev/null +++ b/path.go @@ -0,0 +1,242 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "slices" + "strconv" + "strings" + + "github.com/go-openapi/jsonpointer" +) + +// pathSegments is the location of a validated value inside a document, +// held as an ordered list of unescaped JSON pointer reference tokens. +// +// Validators build a location by appending tokens as they descend into +// properties and array items, then render it only when they report an error. +// Keeping the tokens apart until then is what makes it possible to produce a +// valid [RFC 6901] JSON pointer: a token is escaped when it is rendered, and +// the separator can never be confused with a token that contains one. +// +// The zero value is the location of the document root. +// +// [RFC 6901]: https://datatracker.ietf.org/doc/html/rfc6901 +type pathSegments []pathToken + +// pathToken is one step of a location. +// +// A token addresses a member the way the document does, which is not always +// the way a reader recognizes it: an operation addresses its parameters by +// index, while a message is far more useful naming them. When the two differ, +// display carries the readable form and only the message uses it. +type pathToken struct { + token string + display string + + // structural marks a token a document needs to address the value, but + // that messages have never shown: a "properties" between a schema and one + // of its members, say. It is part of the pointer and absent from the + // dotted form. + structural bool +} + +// readable renders a token the way a message should spell it. +func (t pathToken) readable() string { + if t.display != "" { + return t.display + } + + return t.token +} + +// newPathSegments builds a location from a list of unescaped tokens. +func newPathSegments(tokens ...string) pathSegments { + if len(tokens) == 0 { + return nil + } + + segments := make(pathSegments, len(tokens)) + for i, token := range tokens { + segments[i] = pathToken{token: token} + } + + return segments +} + +// rootPath is the location of the document root. +func rootPath() pathSegments { return nil } + +// String implements [fmt.Stringer] with the legacy dotted notation, so that a +// location interpolated into a message reads as it always has. +func (p pathSegments) String() string { return p.dotted() } + +// child returns the location of a named member of the value at p. +// +// The receiver is never modified: sibling children may be derived from the +// same parent without aliasing one another. +func (p pathSegments) child(token string) pathSegments { + return p.appendToken(pathToken{token: token}) +} + +// childAs returns the location of a member the document addresses as token, +// which messages should spell as display instead. +func (p pathSegments) childAs(token, display string) pathSegments { + return p.appendToken(pathToken{token: token, display: display}) +} + +// structuralChild returns the location of a member a document addresses but +// messages do not name. +func (p pathSegments) structuralChild(token string) pathSegments { + return p.appendToken(pathToken{token: token, structural: true}) +} + +func (p pathSegments) appendToken(token pathToken) pathSegments { + child := make(pathSegments, len(p)+1) + copy(child, p) + child[len(p)] = token + + return child +} + +// children returns the location of a chain of named members below p. +func (p pathSegments) children(tokens ...string) pathSegments { + child := make(pathSegments, len(p)+len(tokens)) + copy(child, p) + for i, token := range tokens { + child[len(p)+i] = pathToken{token: token} + } + + return child +} + +// item returns the location of the index'th element of the array at p. +func (p pathSegments) item(index int) pathSegments { + return p.child(strconv.Itoa(index)) +} + +// isEmpty tells if p locates the document root. +func (p pathSegments) isEmpty() bool { return len(p) == 0 } + +// last returns the trailing meaningful token, or an empty string at the +// document root. +// +// Structural tokens are skipped: they say how a document addresses the value, +// not what the value is, and the callers here are asking the latter. +func (p pathSegments) last() string { + if token, ok := p.meaningfulAt(0); ok { + return token + } + + return "" +} + +// beforeLast returns the meaningful token before the trailing one, or an empty +// string when p holds fewer than two of them. +func (p pathSegments) beforeLast() string { + if token, ok := p.meaningfulAt(1); ok { + return token + } + + return "" +} + +// meaningfulAt returns the nth token from the end, counting only the tokens a +// message would show. +func (p pathSegments) meaningfulAt(n int) (string, bool) { + seen := 0 + for _, token := range slices.Backward(p) { + if token.structural { + continue + } + + if seen == n { + return token.token, true + } + + seen++ + } + + return "", false +} + +// trimIndexes returns p without its trailing array index tokens. +// +// It answers "what is this value inside of", disregarding how deep into an +// array it sits: the items of an example are still an example. +func (p pathSegments) trimIndexes() pathSegments { + end := len(p) + for end > 0 && isIndexToken(p[end-1].token) { + end-- + } + + return p[:end] +} + +// isIndexToken tells if a token addresses an array element rather than a member. +func isIndexToken(token string) bool { + if token == "" { + return false + } + + for _, r := range token { + if r < '0' || r > '9' { + return false + } + } + + return true +} + +// hasSuffix tells if p ends with the given sequence of tokens. +func (p pathSegments) hasSuffix(suffix pathSegments) bool { + if len(suffix) > len(p) { + return false + } + + offset := len(p) - len(suffix) + for i, token := range suffix { + if p[offset+i].token != token.token { + return false + } + } + + return true +} + +// dotted renders the location in the legacy dot-separated notation, e.g. +// "definitions.Pet.friends.0.name". +// +// Tokens are emitted verbatim: a token containing a dot is indistinguishable +// from a separator. This notation is kept because it is what surfaces as the +// name of a validation error, and API consumers of go-swagger servers see it. +// Use [pathSegments.pointer] whenever the location needs to be unambiguous. +func (p pathSegments) dotted() string { + readable := make([]string, 0, len(p)) + for _, token := range p { + if token.structural { + continue + } + + readable = append(readable, token.readable()) + } + + return strings.Join(readable, ".") +} + +// pointer renders the location as an RFC 6901 JSON pointer, e.g. +// "/definitions/Pet/friends/0/name". The document root renders as "". +func (p pathSegments) pointer() string { + if len(p) == 0 { + return "" + } + + var w strings.Builder + for _, token := range p { + w.WriteByte('/') + w.WriteString(jsonpointer.Escape(token.token)) + } + + return w.String() +} diff --git a/path_test.go b/path_test.go new file mode 100644 index 0000000..292be1b --- /dev/null +++ b/path_test.go @@ -0,0 +1,217 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "testing" + + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +func TestPathSegmentsRendering(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path pathSegments + dotted string + pointer string + }{ + { + name: "document root", + path: rootPath(), + dotted: "", + pointer: "", + }, + { + name: "single token", + path: rootPath().child("definitions"), + dotted: "definitions", + pointer: "/definitions", + }, + { + name: "nested properties", + path: newPathSegments("definitions", "Pet", "name"), + dotted: "definitions.Pet.name", + pointer: "/definitions/Pet/name", + }, + { + name: "array item", + path: newPathSegments("friends").item(0).child("name"), + dotted: "friends.0.name", + pointer: "/friends/0/name", + }, + { + name: "token holding the dotted separator", + path: newPathSegments("a.b", "c"), + dotted: "a.b.c", // ambiguous, which is the whole point of the pointer form + pointer: "/a.b/c", + }, + { + name: "token needing RFC 6901 escaping", + path: newPathSegments("n~x/y", "0"), + dotted: "n~x/y.0", + pointer: "/n~0x~1y/0", + }, + { + name: "templated swagger path", + path: newPathSegments("paths", "/pets/{id}", "get"), + dotted: "paths./pets/{id}.get", + pointer: "/paths/~1pets~1{id}/get", + }, + { + name: "empty token", + path: newPathSegments("properties", ""), + dotted: "properties.", + pointer: "/properties/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.dotted, tt.path.dotted()) + assert.Equal(t, tt.dotted, tt.path.String()) + assert.Equal(t, tt.pointer, tt.path.pointer()) + }) + } +} + +func TestPathSegmentsDisplayDiffersFromPointer(t *testing.T) { + t.Parallel() + + // a document addresses an array member by index, while a message reads + // far better naming it + path := newPathSegments("paths", "/pets", "get", "parameters").childAs("1", "tags") + + assert.EqualT(t, "/paths/~1pets/get/parameters/1", path.pointer()) + assert.EqualT(t, "paths./pets.get.parameters.tags", path.dotted()) + + t.Run("structure reads the addressed token, not the readable one", func(t *testing.T) { + t.Parallel() + + assert.EqualT(t, "1", path.last(), "expected the token the document uses") + assert.True(t, path.hasSuffix(newPathSegments("parameters", "1"))) + assert.False(t, path.hasSuffix(newPathSegments("parameters", "tags"))) + assert.EqualT(t, "paths./pets.get.parameters", path.trimIndexes().dotted()) + }) + + t.Run("children of a renamed token keep both forms", func(t *testing.T) { + t.Parallel() + + child := path.child("items") + assert.EqualT(t, "/paths/~1pets/get/parameters/1/items", child.pointer()) + assert.EqualT(t, "paths./pets.get.parameters.tags.items", child.dotted()) + }) +} + +func TestPathSegmentsStructuralTokensAreAddressedNotShown(t *testing.T) { + t.Parallel() + + // a document needs "properties" to address a member of a schema, but no + // message has ever spelled it + path := newPathSegments("definitions", "Pet"). + structuralChild(jsonProperties). + child("name"). + child(jsonDefault) + + assert.EqualT(t, "/definitions/Pet/properties/name/default", path.pointer()) + assert.EqualT(t, "definitions.Pet.name.default", path.dotted()) + + t.Run("a structural token is not what the value is", func(t *testing.T) { + t.Parallel() + + // the predicates telling schema from data ask what the value is, and + // plumbing must not answer + inside := newPathSegments("responses", "200", "examples").structuralChild("application/json") + assert.EqualT(t, swaggerExamples, inside.last()) + assert.EqualT(t, "200", inside.beforeLast()) + }) + + t.Run("structural tokens still address", func(t *testing.T) { + t.Parallel() + + assert.True(t, path.hasSuffix(newPathSegments("properties", "name", "default")), + "expected structure to see the token a document uses") + }) +} + +func TestPathSegmentsAppendIsCopyOnWrite(t *testing.T) { + t.Parallel() + + // a parent spawning several children is the common case: each child must + // get its own backing array, or siblings overwrite one another. + parent := newPathSegments("definitions", "Pet") + require.Equal(t, 2, len(parent)) + + first := parent.child("name") + second := parent.child("age") + third := parent.item(3) + + assert.Equal(t, "definitions.Pet", parent.dotted()) + assert.Equal(t, "definitions.Pet.name", first.dotted()) + assert.Equal(t, "definitions.Pet.age", second.dotted()) + assert.Equal(t, "definitions.Pet.3", third.dotted()) +} + +func TestPathSegmentsChildren(t *testing.T) { + t.Parallel() + + parent := newPathSegments("paths") + + assert.Equal(t, "paths./pets.get.responses", parent.children("/pets", "get", "responses").dotted()) + assert.Equal(t, "/paths/~1pets/get/responses", parent.children("/pets", "get", "responses").pointer()) + assert.Equal(t, "paths", parent.children().dotted()) + assert.Equal(t, "paths", parent.dotted(), "expected the parent to be left alone") +} + +func TestPathSegmentsInspection(t *testing.T) { + t.Parallel() + + t.Run("with an empty path", func(t *testing.T) { + t.Parallel() + + empty := rootPath() + assert.True(t, empty.isEmpty()) + assert.Empty(t, empty.last()) + assert.Empty(t, empty.beforeLast()) + }) + + t.Run("with a single token", func(t *testing.T) { + t.Parallel() + + single := newPathSegments("properties") + assert.False(t, single.isEmpty()) + assert.Equal(t, "properties", single.last()) + assert.Empty(t, single.beforeLast(), "expected no token before the only one") + }) + + t.Run("with several tokens", func(t *testing.T) { + t.Parallel() + + path := newPathSegments("definitions", "Pet", "properties") + assert.Equal(t, "properties", path.last()) + assert.Equal(t, "Pet", path.beforeLast()) + }) +} + +func TestPathSegmentsHasSuffix(t *testing.T) { + t.Parallel() + + path := newPathSegments("definitions", "Pet", "friends", "Pet") + + assert.True(t, path.hasSuffix(newPathSegments("Pet"))) + assert.True(t, path.hasSuffix(newPathSegments("friends", "Pet"))) + assert.True(t, path.hasSuffix(path)) + assert.True(t, path.hasSuffix(rootPath()), "expected the empty suffix to always match") + + assert.False(t, path.hasSuffix(newPathSegments("friends"))) + assert.False(t, path.hasSuffix(newPathSegments("Dog"))) + assert.False(t, path.hasSuffix(newPathSegments("definitions", "Pet", "friends", "Pet", "name"))) + + // tokens are compared whole: no mid-token match like the dotted string used to allow + assert.False(t, newPathSegments("abc").hasSuffix(newPathSegments("bc"))) +} diff --git a/ref_locations.go b/ref_locations.go new file mode 100644 index 0000000..1853340 --- /dev/null +++ b/ref_locations.go @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "github.com/go-openapi/analysis" +) + +// refLocations tells where a $ref value is declared in a document. +// +// The analyzer indexes references the other way around, by the location each +// was found at, so the index is inverted here. Only declarations are indexed: +// a "$ref" member sitting in an example or in a default value is data, and the +// analyzer never walks into it. +type refLocations map[string]pathSegments + +// newRefLocations inverts the analyzer's reference index. +// +// A reference declared in several places keeps the smallest declaration, so +// that the answer does not depend on map iteration order. +func newRefLocations(analyzer *analysis.Spec) refLocations { + declarations := make(map[string]string) + for location, ref := range analyzer.AllRefsByLocation() { + value := ref.String() + if value == "" { + continue + } + + if known, isKnown := declarations[value]; isKnown && known <= location { + continue + } + + declarations[value] = location + } + + locations := make(refLocations, len(declarations)) + for value, location := range declarations { + locations[value] = localRefPath(location) + } + + return locations +} + +// at returns where a reference is declared, or the document root when the +// reference is not one the analyzer indexed. +func (l refLocations) at(ref string) pathSegments { + return l[ref] +} diff --git a/ref_locations_test.go b/ref_locations_test.go new file mode 100644 index 0000000..a8d6d5c --- /dev/null +++ b/ref_locations_test.go @@ -0,0 +1,224 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "encoding/json" + "testing" + + "github.com/go-openapi/analysis" + "github.com/go-openapi/loads" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// indexRefs analyzes a document body, wrapped into the smallest valid spec. +func indexRefs(t *testing.T, body string) refLocations { + t.Helper() + + doc := `{"swagger":"2.0","info":{"title":"t","version":"1"},` + body + `}` + d, err := loads.Analyzed(json.RawMessage(doc), "") + require.NoError(t, err) + + return newRefLocations(analysis.New(d.Spec())) +} + +func TestRefLocations_FindsDeclarations(t *testing.T) { + t.Parallel() + + locations := indexRefs(t, ` + "paths": { + "/pets": { + "get": { + "parameters": [ + {"name": "id", "in": "path"}, + {"$ref": "#/parameters/tagsParam"} + ], + "responses": {"200": {"schema": {"$ref": "#/definitions/Pet"}}} + } + } + }, + "definitions": { + "Pet": {"properties": {"owner": {"$ref": "#/definitions/Owner"}}}, + "Remote": {"$ref": "https://elsewhere.example/schema.json"} + }`) + + assert.EqualT(t, "/paths/~1pets/get/parameters/1", locations.at("#/parameters/tagsParam").pointer()) + assert.EqualT(t, "/paths/~1pets/get/responses/200/schema", locations.at("#/definitions/Pet").pointer()) + assert.EqualT(t, "/definitions/Pet/properties/owner", locations.at("#/definitions/Owner").pointer()) + assert.EqualT(t, "/definitions/Remote", locations.at("https://elsewhere.example/schema.json").pointer()) +} + +func TestRefLocations_UnknownReferenceIsTheRoot(t *testing.T) { + t.Parallel() + + locations := indexRefs(t, `"definitions": {"Pet": {"type": "object"}}`) + + assert.True(t, locations.at("#/definitions/Nope").isEmpty()) + assert.EqualT(t, "", locations.at("#/definitions/Nope").pointer()) +} + +func TestRefLocations_SkipsExampleData(t *testing.T) { + t.Parallel() + + // an example may legitimately hold a "$ref" member: it declares nothing + locations := indexRefs(t, ` + "definitions": { + "Pet": { + "example": {"$ref": "#/definitions/NotAReference"}, + "examples": {"application/json": {"$ref": "#/definitions/NotAReferenceEither"}} + } + }`) + + assert.True(t, locations.at("#/definitions/NotAReference").isEmpty()) + assert.True(t, locations.at("#/definitions/NotAReferenceEither").isEmpty()) +} + +func TestRefLocations_DefaultResponseIsADeclarationNotAValue(t *testing.T) { + t.Parallel() + + locations := indexRefs(t, ` + "paths": { + "/pets": { + "get": { + "responses": { + "default": {"$ref": "#/responses/operationError"} + } + } + } + }, + "definitions": { + "Pet": { + "default": {"$ref": "#/definitions/NotAReference"} + } + }`) + + assert.EqualT(t, "/paths/~1pets/get/responses/default", + locations.at("#/responses/operationError").pointer()) + assert.True(t, locations.at("#/definitions/NotAReference").isEmpty(), + "a default value holds data: a $ref member there declares nothing") +} + +func TestRefLocations_TieBreakIsStable(t *testing.T) { + t.Parallel() + + // the same reference declared twice: whichever is kept, it must be the + // same one on every run, since maps are walked in random order + const doc = ` + "definitions": { + "Zebra": {"$ref": "#/definitions/Pet"}, + "Ant": {"$ref": "#/definitions/Pet"} + }` + + first := indexRefs(t, doc).at("#/definitions/Pet").pointer() + for range 20 { + assert.EqualT(t, first, indexRefs(t, doc).at("#/definitions/Pet").pointer()) + } + assert.EqualT(t, "/definitions/Ant", first) +} + +func TestRefLocations_CoverEveryAnalyzedReference(t *testing.T) { + t.Parallel() + + // the index and the two diagnostics read the same analyzer, so every + // reference they can report is one the index knows where to find + d, err := loads.Analyzed(json.RawMessage(`{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "parameters": {"tagsParam": {"name": "tags", "in": "query", "type": "string"}}, + "paths": { + "/pets": { + "get": { + "parameters": [{"$ref": "#/parameters/tagsParam"}], + "responses": { + "200": {"description": "ok", "schema": {"$ref": "#/definitions/Pet"}}, + "default": {"$ref": "#/responses/oops"} + } + } + } + }, + "responses": {"oops": {"description": "oops"}}, + "definitions": { + "Pet": {"type": "object", "properties": {"owner": {"$ref": "#/definitions/Owner"}}}, + "Owner": {"type": "object"} + } + }`), "") + require.NoError(t, err) + + analyzer := analysis.New(d.Spec()) + locations := newRefLocations(analyzer) + + refs := analyzer.AllRefs() + require.NotEmpty(t, refs) + for _, found := range refs { + ref := found + assert.False(t, locations.at(ref.String()).isEmpty(), + "expected a location for %q", ref.String()) + } +} + +func TestValidateDubiousRefs_LocatesTheReference(t *testing.T) { + t.Parallel() + + doc := `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": {}, + "definitions": { + "A": {"$ref": "file:///etc/passwd"} + } + }` + + res := dubiousValidatorFromJSON(t, doc).validateDubiousRefs() + located := res.LocatedWarnings() + require.Len(t, located, 1) + + assert.StringContainsT(t, located[0].Err.Error(), "escapes the spec's base path") + assert.EqualT(t, "/definitions/A", located[0].Pointer) +} + +func TestRefLocations_ReportedThroughSpecValidation(t *testing.T) { + t.Parallel() + + // end to end: refLocations is built by Validate, so an invalid $ref + // reported to a caller of the public API carries where it was declared. + const raw = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": {"/pets": {"get": {"operationId": "g", "responses": {"200": {"description": "ok"}}}}}, + "definitions": {"Bad": {"$ref": "file:///etc/passwd"}} + }` + + doc, err := loads.Analyzed(json.RawMessage(raw), "") + require.NoError(t, err) + + res, _ := NewSpecValidator(doc.Schema(), strfmt.Default).Validate(doc) + require.False(t, res.IsValid()) + + located := res.LocatedErrors() + require.Len(t, located, 1) + assert.StringContainsT(t, located[0].Err.Error(), "invalid ref") + assert.EqualT(t, "/definitions/Bad", located[0].Pointer) +} + +func TestValidateDubiousRefs_HostSpreadHasNoSingleLocation(t *testing.T) { + t.Parallel() + + // the warning is about the set of hosts, not about one reference + doc := `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": {}, + "definitions": { + "A": {"$ref": "http://host-one.example/a.json"}, + "B": {"$ref": "https://host-two.example/b.json"} + } + }` + + res := dubiousValidatorFromJSON(t, doc).validateDubiousRefs() + located := res.LocatedWarnings() + require.Len(t, located, 1) + assert.Empty(t, located[0].Pointer) +} diff --git a/result.go b/result.go index ede9455..fc6f8e7 100644 --- a/result.go +++ b/result.go @@ -14,6 +14,25 @@ import ( var emptyResult = &Result{MatchCount: 1} +// Located pairs a validation error with the location of the value that caused it. +type Located struct { + // Err is the reported error or warning. + Err error + + // Pointer locates the offending value as an RFC 6901 JSON pointer, + // relative to the validated document. + // + // It is empty when the document as a whole is the answer: either because + // the finding is about the document rather than a value in it, such as a + // duplicate operation id, or because the value in question is the root. + // An empty pointer is a valid one, addressing the whole document. + // + // A finding about something a document does not contain, a missing + // required property say, is located on the value that should contain it: + // what is absent has no node to point at. + Pointer string +} + // Result represents a validation result set, composed of // errors and warnings. // @@ -25,12 +44,17 @@ var emptyResult = &Result{MatchCount: 1} // schema validation. Results from the validation branch // with most matches get eventually selected. // -// Proposal for enhancement: keep path of key originating the error. +// Use [Result.LocatedErrors] to know where each error happened. type Result struct { Errors []error Warnings []error MatchCount int + // errorLocations[i] locates Errors[i], and likewise for warnings. Kept + // aligned by the add methods; see [Result.LocatedErrors]. + errorLocations []string + warningLocations []string + // the object data data any @@ -173,8 +197,8 @@ func (r *Result) MergeAsErrors(others ...*Result) *Result { for _, other := range others { if other != nil { r.resetCaches() - r.AddErrors(other.Errors...) - r.AddErrors(other.Warnings...) + r.carryErrors(other.Errors, other.errorLocations) + r.carryErrors(other.Warnings, other.warningLocations) r.MatchCount += other.MatchCount if other.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(other) @@ -191,8 +215,8 @@ func (r *Result) MergeAsWarnings(others ...*Result) *Result { for _, other := range others { if other != nil { r.resetCaches() - r.AddWarnings(other.Errors...) - r.AddWarnings(other.Warnings...) + r.carryWarnings(other.Errors, other.errorLocations) + r.carryWarnings(other.Warnings, other.warningLocations) r.MatchCount += other.MatchCount if other.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(other) @@ -207,39 +231,79 @@ func (r *Result) MergeAsWarnings(others ...*Result) *Result { // Since the same check may be passed several times while exploring the // spec structure (via $ref, ...) reported messages are kept // unique. +// +// Errors added this way carry no location. Validators use [Result.addErrorsAt] +// so that [Result.LocatedErrors] can tell where the failure happened. func (r *Result) AddErrors(errors ...error) { - for _, e := range errors { - found := false - if e != nil { - for _, isReported := range r.Errors { - if e.Error() == isReported.Error() { - found = true - break - } - } - if !found { - r.Errors = append(r.Errors, e) - } - } - } + r.addLocatedErrors("", errors...) } // AddWarnings adds warnings to this validation result (if not already reported). func (r *Result) AddWarnings(warnings ...error) { - for _, e := range warnings { - found := false - if e != nil { - for _, isReported := range r.Warnings { - if e.Error() == isReported.Error() { - found = true - break - } - } - if !found { - r.Warnings = append(r.Warnings, e) - } + r.addLocatedWarnings("", warnings...) +} + +// isReportedError tells if the same message is already part of a collection. +func isReportedError(reported []error, e error) bool { + msg := e.Error() + for _, isReported := range reported { + if msg == isReported.Error() { + return true } } + + return false +} + +// locationAt reads a location out of a slice that may be shorter than the +// errors it describes. +func locationAt(locations []string, i int) string { + if i < len(locations) { + return locations[i] + } + + return "" +} + +// appendLocation records the location of the error that has just been appended, +// keeping the location slice aligned with the error slice it describes. +// +// Errors may reach a Result without going through the methods here (a caller +// assigning Errors directly, say), so the slice is padded rather than assumed +// to be in step. +func appendLocation(locations []string, upTo int, pointer string) []string { + for len(locations) < upTo-1 { + locations = append(locations, "") + } + + return append(locations, pointer) +} + +// LocatedErrors returns the reported errors, each paired with the JSON pointer +// of the value that caused it. +// +// The pointer is empty whenever the location is unknown, so callers should +// treat it as a hint and keep using the error message as the primary report. +func (r *Result) LocatedErrors() []Located { + return locate(r.Errors, r.errorLocations) +} + +// LocatedWarnings returns the reported warnings, each paired with the JSON +// pointer of the value that caused it. +func (r *Result) LocatedWarnings() []Located { + return locate(r.Warnings, r.warningLocations) +} + +func locate(errs []error, locations []string) []Located { + located := make([]Located, len(errs)) + for i, err := range errs { + located[i] = Located{Err: err} + if i < len(locations) { + located[i].Pointer = locations[i] + } + } + + return located } // IsValid returns true when this result is valid. @@ -298,6 +362,61 @@ func (r *Result) AsError() error { return errors.CompositeValidationError(r.Errors...) } +// addErrorsAt adds errors located at the given path. +func (r *Result) addErrorsAt(at pathSegments, errors ...error) { + r.addLocatedErrors(at.pointer(), errors...) +} + +// addWarningsAt adds warnings located at the given path. +func (r *Result) addWarningsAt(at pathSegments, warnings ...error) { + r.addLocatedWarnings(at.pointer(), warnings...) +} + +func (r *Result) addLocatedErrors(pointer string, errors ...error) { + for _, e := range errors { + if e == nil { + continue + } + + if isReportedError(r.Errors, e) { + continue + } + + r.Errors = append(r.Errors, e) + r.errorLocations = appendLocation(r.errorLocations, len(r.Errors), pointer) + } +} + +func (r *Result) addLocatedWarnings(pointer string, warnings ...error) { + for _, e := range warnings { + if e == nil { + continue + } + + if isReportedError(r.Warnings, e) { + continue + } + + r.Warnings = append(r.Warnings, e) + r.warningLocations = appendLocation(r.warningLocations, len(r.Warnings), pointer) + } +} + +// carryErrors adds errors from another result as errors, one by one, so that +// each keeps the location that result recorded for it. +func (r *Result) carryErrors(errs []error, locations []string) { + for i, e := range errs { + r.addLocatedErrors(locationAt(locations, i), e) + } +} + +// carryWarnings adds errors from another result as warnings, keeping locations. +func (r *Result) carryWarnings(errs []error, locations []string) { + for i, e := range errs { + r.addLocatedWarnings(locationAt(locations, i), e) + } +} + func (r *Result) resetCaches() { r.cachedFieldSchemata = nil r.cachedItemSchemata = nil @@ -391,8 +510,8 @@ func (r *Result) addSliceSchemata(slice reflect.Value, i int, schema *spec.Schem // mergeWithoutRootSchemata merges other into r, ignoring the rootObject schemata. func (r *Result) mergeWithoutRootSchemata(other *Result) { r.resetCaches() - r.AddErrors(other.Errors...) - r.AddWarnings(other.Warnings...) + r.carryErrors(other.Errors, other.errorLocations) + r.carryWarnings(other.Warnings, other.warningLocations) r.MatchCount += other.MatchCount if other.fieldSchemata != nil { @@ -438,15 +557,19 @@ func (r *Result) keepRelevantErrors() *Result { // codes would require to change a lot here. So, for the moment, let's go with // placeholders. strippedErrors := []error{} - for _, e := range r.Errors { + strippedErrorLocations := []string{} + for i, e := range r.Errors { if isImportant(e) { strippedErrors = append(strippedErrors, stripImportantTag(e)) + strippedErrorLocations = append(strippedErrorLocations, locationAt(r.errorLocations, i)) } } strippedWarnings := []error{} - for _, e := range r.Warnings { + strippedWarningLocations := []string{} + for i, e := range r.Warnings { if isImportant(e) { strippedWarnings = append(strippedWarnings, stripImportantTag(e)) + strippedWarningLocations = append(strippedWarningLocations, locationAt(r.warningLocations, i)) } } var strippedResult *Result @@ -456,14 +579,18 @@ func (r *Result) keepRelevantErrors() *Result { strippedResult = new(Result) } strippedResult.Errors = strippedErrors + strippedResult.errorLocations = strippedErrorLocations strippedResult.Warnings = strippedWarnings + strippedResult.warningLocations = strippedWarningLocations return strippedResult } func (r *Result) cleared() *Result { // clear the Result to be reusable. Keep allocated capacity. r.Errors = r.Errors[:0] + r.errorLocations = r.errorLocations[:0] r.Warnings = r.Warnings[:0] + r.warningLocations = r.warningLocations[:0] r.MatchCount = 0 r.data = nil r.rootObjectSchemata.one = nil diff --git a/result_location_test.go b/result_location_test.go new file mode 100644 index 0000000..11d209b --- /dev/null +++ b/result_location_test.go @@ -0,0 +1,331 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/go-openapi/loads" + "github.com/go-openapi/spec" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +// Locations are kept in a slice parallel to the errors. The tests below guard +// that invariant across the operations that reshuffle a Result, and check the +// pointers actually resolve into the validated document. + +func TestResultLocations_AlignedAcrossMerges(t *testing.T) { + t.Parallel() + + first := new(Result) + first.addErrorsAt(newPathSegments("a", "0"), errOne) + first.AddErrors(errAnother) // no location + + second := new(Result) + second.addErrorsAt(newPathSegments("b"), errNew) + second.addWarningsAt(newPathSegments("c"), errOneWarning) + + first.Merge(second) + + located := first.LocatedErrors() + require.Len(t, located, 3) + assert.EqualT(t, "/a/0", located[0].Pointer) + assert.EqualT(t, errOne, located[0].Err) + assert.Empty(t, located[1].Pointer, "expected AddErrors to leave the location unknown") + assert.EqualT(t, errAnother, located[1].Err) + assert.EqualT(t, "/b", located[2].Pointer) + assert.EqualT(t, errNew, located[2].Err) + + warnings := first.LocatedWarnings() + require.Len(t, warnings, 1) + assert.EqualT(t, "/c", warnings[0].Pointer) +} + +func TestResultLocations_SurviveMergeAsWarnings(t *testing.T) { + t.Parallel() + + source := new(Result) + source.addErrorsAt(newPathSegments("a"), errOne) + source.addWarningsAt(newPathSegments("b"), errOneWarning) + + target := new(Result) + target.MergeAsWarnings(source) + + assert.Empty(t, target.Errors) + located := target.LocatedWarnings() + require.Len(t, located, 2) + assert.EqualT(t, "/a", located[0].Pointer) + assert.EqualT(t, "/b", located[1].Pointer) +} + +func TestResultLocations_SurviveMergeAsErrors(t *testing.T) { + t.Parallel() + + source := new(Result) + source.addErrorsAt(newPathSegments("a"), errOne) + source.addWarningsAt(newPathSegments("b"), errOneWarning) + + target := new(Result) + target.MergeAsErrors(source) + + located := target.LocatedErrors() + require.Len(t, located, 2) + assert.EqualT(t, "/a", located[0].Pointer) + assert.EqualT(t, "/b", located[1].Pointer) +} + +func TestResultLocations_DedupeKeepsTheFirstLocation(t *testing.T) { + t.Parallel() + + // AddErrors drops a message that is already reported; the location slice + // must not gain an entry for the error that was dropped. + res := new(Result) + res.addErrorsAt(newPathSegments("a"), errOne) + res.addErrorsAt(newPathSegments("b"), errOne) + + require.Len(t, res.Errors, 1) + located := res.LocatedErrors() + require.Len(t, located, 1) + assert.EqualT(t, "/a", located[0].Pointer) +} + +func TestResultLocations_ClearedOnRecycle(t *testing.T) { + t.Parallel() + + res := pools.poolOfResults.BorrowResult() + res.addErrorsAt(newPathSegments("a"), errOne) + require.Len(t, res.LocatedErrors(), 1) + + res = res.cleared() + assert.Empty(t, res.Errors) + assert.Empty(t, res.LocatedErrors(), "expected a recycled result to leak no location") +} + +func TestResultLocations_KeepRelevantErrors(t *testing.T) { + t.Parallel() + + res := new(Result) + res.addErrorsAt(newPathSegments("dropped"), errOne) + res.addErrorsAt(newPathSegments("kept"), errImportant) + + stripped := res.keepRelevantErrors() + located := stripped.LocatedErrors() + require.Len(t, located, 1) + assert.EqualT(t, "/kept", located[0].Pointer) +} + +func TestResultLocations_UnknownWhenNeverRecorded(t *testing.T) { + t.Parallel() + + res := new(Result) + res.AddErrors(errOne, errAnother) + + for _, located := range res.LocatedErrors() { + assert.Empty(t, located.Pointer) + } +} + +func TestResultLocations_SchemaValidation(t *testing.T) { + t.Parallel() + + schema := new(spec.Schema) + require.NoError(t, json.Unmarshal([]byte(`{ + "type": "object", + "properties": { + "friends": { + "type": "array", + "items": {"type": "object", "properties": {"name": {"type": "string"}}, "required": ["age"]} + }, + "n~x/y": {"type": "string", "maxLength": 2} + } + }`), schema)) + + data := map[string]any{ + "friends": []any{map[string]any{nameProp: 42}}, + "n~x/y": "far too long", + } + + res := NewSchemaValidator(schema, nil, "", strfmt.Default).Validate(data) + require.False(t, res.IsValid()) + + pointers := make([]string, 0, len(res.Errors)) + for _, located := range res.LocatedErrors() { + pointers = append(pointers, located.Pointer) + } + + assert.SliceContainsT(t, pointers, "/friends/0/name") + // a missing property has no node of its own: the object lacking it is + // what has to be amended, and what a document can address + assert.SliceContainsT(t, pointers, "/friends/0") + assert.SliceContainsT(t, pointers, "/n~0x~1y", "expected the token to be escaped") +} + +func TestResultLocations_SpecValidation(t *testing.T) { + t.Parallel() + + const raw = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": { + "/pets/{id}": { + "get": { + "operationId": "getPet", + "parameters": [{"name": "id", "in": "path", "required": true, "type": "string"}], + "responses": { + "200": { + "description": "ok", + "schema": {"$ref": "#/definitions/Pet"}, + "examples": {"application/json": {"friends": [{"name": 7}]}} + } + } + } + } + }, + "definitions": { + "Pet": { + "type": "object", + "properties": { + "name": {"type": "string", "default": 42}, + "friends": {"type": "array", "items": {"$ref": "#/definitions/Pet"}} + } + }, + "Unused": {"type": "object"} + } + }` + + doc, err := loads.Analyzed(json.RawMessage(raw), "") + require.NoError(t, err) + + res, warns := NewSpecValidator(doc.Schema(), strfmt.Default).Validate(doc) + require.False(t, res.IsValid()) + + t.Run("a default is located in the definition that declares it", func(t *testing.T) { + assert.SliceContainsT(t, pointersOf(res.LocatedErrors()), "/definitions/Pet/properties/name/default") + }) + + t.Run("an example is located under its response", func(t *testing.T) { + assert.SliceContainsT(t, pointersOf(warns.LocatedErrors()), + "/paths/~1pets~1{id}/get/responses/200/examples/application~1json/friends/0/name") + }) + + t.Run("an unused definition is located", func(t *testing.T) { + assert.SliceContainsT(t, pointersOf(warns.LocatedErrors()), "/definitions/Unused") + }) + + t.Run("every reported location is a valid JSON pointer", func(t *testing.T) { + for _, located := range append(res.LocatedErrors(), warns.LocatedErrors()...) { + if located.Pointer == "" { + continue + } + + assert.EqualT(t, uint8('/'), located.Pointer[0], + "expected a pointer to start with a separator, got %q", located.Pointer) + } + }) +} + +func TestResultLocations_MissingValueIsLocatedOnItsHolder(t *testing.T) { + t.Parallel() + + // what is absent has no node to point at, so the value that should hold + // it is reported: that is what a reader has to open, and the only thing a + // document can address + schema := new(spec.Schema) + require.NoError(t, json.Unmarshal([]byte(`{ + "type": "object", + "required": ["atRoot"], + "properties": { + "nested": {"type": "object", "required": ["deep"]} + } + }`), schema)) + + res := NewSchemaValidator(schema, nil, "", strfmt.Default). + Validate(map[string]any{"nested": map[string]any{}}) + require.False(t, res.IsValid()) + + located := res.LocatedErrors() + byMessage := make(map[string]string, len(located)) + for _, l := range located { + byMessage[l.Err.Error()] = l.Pointer + } + + assert.EqualT(t, "/nested", byMessage["nested.deep in body is required"], + "expected the object lacking the property") + assert.EqualT(t, "", byMessage["atRoot in body is required"], + "expected the document root, which an empty pointer addresses") +} + +func TestResultLocations_RequiredEntryIsLocated(t *testing.T) { + t.Parallel() + + // the TUI anchors on where the offending text sits, so a required entry + // that names no property points at the entry, not at the definition + const raw = `{ + "swagger": "2.0", + "info": {"title": "t", "version": "1"}, + "paths": {"/pets": {"get": {"operationId": "g", "responses": {"200": {"description": "ok"}}}}}, + "definitions": { + "Pet": { + "type": "object", + "required": ["name", "notDeclared"], + "properties": { + "name": {"type": "string"}, + "readOnlyOne": {"type": "string", "readOnly": true} + } + }, + "Owner": { + "type": "object", + "required": ["readOnlyOne"], + "properties": {"readOnlyOne": {"type": "string", "readOnly": true}} + } + } + }` + + doc, err := loads.Analyzed(json.RawMessage(raw), "") + require.NoError(t, err) + + validator := NewSpecValidator(doc.Schema(), strfmt.Default) + validator.SetContinueOnErrors(true) + res, warns := validator.Validate(doc) + + t.Run("an undefined required property points at its entry", func(t *testing.T) { + var found bool + for _, located := range res.LocatedErrors() { + if !strings.Contains(located.Err.Error(), "notDeclared") { + continue + } + + found = true + // index 1 of Pet's required array, not "/definitions/Pet" + assert.EqualT(t, "/definitions/Pet/required/1", located.Pointer) + } + require.True(t, found, "expected the undefined required property to be reported") + }) + + t.Run("a required and readOnly property points at its entry", func(t *testing.T) { + var found bool + for _, located := range warns.LocatedErrors() { + if !strings.Contains(located.Err.Error(), "readOnly") { + continue + } + + found = true + assert.EqualT(t, "/definitions/Owner/required/0", located.Pointer) + } + require.True(t, found, "expected the readOnly-and-required warning to be reported") + }) +} + +func pointersOf(located []Located) []string { + pointers := make([]string, 0, len(located)) + for _, l := range located { + pointers = append(pointers, l.Pointer) + } + + return pointers +} diff --git a/schema.go b/schema.go index 706b7f5..2dfa39a 100644 --- a/schema.go +++ b/schema.go @@ -15,7 +15,17 @@ import ( // SchemaValidator validates data against a JSON schema. type SchemaValidator struct { - Path string + // Path is the location of the validated value, in the legacy dot-separated + // notation. It is what surfaces as the name of a validation error. + // + // Deprecated: a dotted path is ambiguous whenever a property name contains + // a dot. Prefer the JSON pointer rendering of the same location. + Path string + + // path is the same location, kept as JSON pointer reference tokens so that + // children may be derived from it unambiguously. + path pathSegments + in string Schema *spec.Schema validators [8]valueValidator @@ -51,10 +61,23 @@ func NewSchemaValidator(schema *spec.Schema, rootSchema any, root string, format o(opts) } - return newSchemaValidator(schema, rootSchema, root, formats, opts) + return newSchemaValidator(schema, rootSchema, rootPathFromString(root), formats, opts) +} + +// rootPathFromString interprets the root path of the exported constructors. +// +// The caller hands over an opaque string, so there is no telling which of its +// dots are separators and which belong to a name: it is taken as a single +// reference token. +func rootPathFromString(root string) pathSegments { + if root == "" { + return rootPath() + } + + return newPathSegments(root) } -func newSchemaValidator(schema *spec.Schema, rootSchema any, root string, formats strfmt.Registry, opts *SchemaValidatorOptions) *SchemaValidator { +func newSchemaValidator(schema *spec.Schema, rootSchema any, root pathSegments, formats strfmt.Registry, opts *SchemaValidatorOptions) *SchemaValidator { if schema == nil { return nil } @@ -82,7 +105,8 @@ func newSchemaValidator(schema *spec.Schema, rootSchema any, root string, format s = new(SchemaValidator) } - s.Path = root + s.path = root + s.Path = root.dotted() s.in = "body" s.Schema = schema s.Root = rootSchema @@ -104,8 +128,11 @@ func newSchemaValidator(schema *spec.Schema, rootSchema any, root string, format } // SetPath sets the path for this schema validator. +// +// Note that the sub-validators are built when the validator is created, so +// this only affects errors reported by this validator, not by its children. func (s *SchemaValidator) SetPath(path string) { - s.Path = path + s.setPath(rootPathFromString(path)) } // Applies returns true when this schema validator applies. @@ -169,7 +196,7 @@ func (s *SchemaValidator) Validate(data any) *Result { // to map[string]interface{}. var dd any if err := jsonutils.FromDynamicJSON(data, &dd); err != nil { - result.AddErrors(err) + result.addErrorsAt(s.path, err) result.Inc() return result @@ -185,7 +212,7 @@ func (s *SchemaValidator) Validate(data any) *Result { if s.Schema.Type.Contains(integerType) { // avoid lossy conversion in, erri := num.Int64() if erri != nil { - result.AddErrors(invalidTypeConversionMsg(s.Path, erri)) + result.addErrorsAt(s.path, invalidTypeConversionMsg(s.Path, erri)) result.Inc() return result @@ -194,7 +221,7 @@ func (s *SchemaValidator) Validate(data any) *Result { } else { nf, errf := num.Float64() if errf != nil { - result.AddErrors(invalidTypeConversionMsg(s.Path, errf)) + result.addErrorsAt(s.path, invalidTypeConversionMsg(s.Path, errf)) result.Inc() return result @@ -235,7 +262,7 @@ func (s *SchemaValidator) Validate(data any) *Result { func (s *SchemaValidator) typeValidator() valueValidator { return newTypeValidator( - s.Path, + s.path, s.in, s.Schema.Type, s.Schema.Nullable, @@ -246,7 +273,7 @@ func (s *SchemaValidator) typeValidator() valueValidator { func (s *SchemaValidator) commonValidator() valueValidator { return newBasicCommonValidator( - s.Path, + s.path, s.in, s.Schema.Default, s.Schema.Enum, @@ -256,7 +283,7 @@ func (s *SchemaValidator) commonValidator() valueValidator { func (s *SchemaValidator) sliceValidator() valueValidator { return newSliceValidator( - s.Path, + s.path, s.in, s.Schema.MaxItems, s.Schema.MinItems, @@ -271,7 +298,7 @@ func (s *SchemaValidator) sliceValidator() valueValidator { func (s *SchemaValidator) numberValidator() valueValidator { return newNumberValidator( - s.Path, + s.path, s.in, s.Schema.Default, s.Schema.MultipleOf, @@ -287,7 +314,7 @@ func (s *SchemaValidator) numberValidator() valueValidator { func (s *SchemaValidator) stringValidator() valueValidator { return newStringValidator( - s.Path, + s.path, s.in, nil, false, @@ -301,7 +328,7 @@ func (s *SchemaValidator) stringValidator() valueValidator { func (s *SchemaValidator) formatValidator() valueValidator { return newFormatValidator( - s.Path, + s.path, s.in, s.Schema.Format, s.KnownFormats, @@ -312,14 +339,14 @@ func (s *SchemaValidator) formatValidator() valueValidator { func (s *SchemaValidator) schemaPropsValidator() valueValidator { sch := s.Schema return newSchemaPropsValidator( - s.Path, s.in, sch.AllOf, sch.OneOf, sch.AnyOf, sch.Not, sch.Dependencies, s.Root, s.KnownFormats, + s.path, s.in, sch.AllOf, sch.OneOf, sch.AnyOf, sch.Not, sch.Dependencies, s.Root, s.KnownFormats, s.Options, ) } func (s *SchemaValidator) objectValidator() valueValidator { return newObjectValidator( - s.Path, + s.path, s.in, s.Schema.MaxProperties, s.Schema.MinProperties, @@ -333,6 +360,11 @@ func (s *SchemaValidator) objectValidator() valueValidator { ) } +func (s *SchemaValidator) setPath(path pathSegments) { + s.path = path + s.Path = path.dotted() +} + func (s *SchemaValidator) redeem() { pools.poolOfSchemaValidators.RedeemValidator(s) } diff --git a/schema_props.go b/schema_props.go index 2c4354d..08a2030 100644 --- a/schema_props.go +++ b/schema_props.go @@ -12,7 +12,7 @@ import ( ) type schemaPropsValidator struct { - Path string + Path pathSegments In string AllOf []spec.Schema OneOf []spec.Schema @@ -28,12 +28,8 @@ type schemaPropsValidator struct { Options *SchemaValidatorOptions } -func (s *schemaPropsValidator) SetPath(path string) { - s.Path = path -} - func newSchemaPropsValidator( - path string, in string, allOf, oneOf, anyOf []spec.Schema, not *spec.Schema, deps spec.Dependencies, root any, formats strfmt.Registry, + path pathSegments, in string, allOf, oneOf, anyOf []spec.Schema, not *spec.Schema, deps spec.Dependencies, root any, formats strfmt.Registry, opts *SchemaValidatorOptions, ) *schemaPropsValidator { if opts == nil { @@ -178,7 +174,7 @@ func (s *schemaPropsValidator) validateAnyOf(data any, mainResult, keepResultAny } } - mainResult.AddErrors(mustValidateAtLeastOneSchemaMsg(s.Path)) + mainResult.addErrorsAt(s.Path, mustValidateAtLeastOneSchemaMsg(s.Path.dotted())) mainResult.Merge(bestFailures) } @@ -224,7 +220,7 @@ func (s *schemaPropsValidator) validateOneOf(data any, mainResult, keepResultOne switch validated { case 0: - mainResult.AddErrors(mustValidateOnlyOneSchemaMsg(s.Path, "Found none valid")) + mainResult.addErrorsAt(s.Path, mustValidateOnlyOneSchemaMsg(s.Path.dotted(), "Found none valid")) mainResult.Merge(bestFailures) // firstSucess necessarily nil case 1: @@ -233,7 +229,7 @@ func (s *schemaPropsValidator) validateOneOf(data any, mainResult, keepResultOne pools.poolOfResults.RedeemResult(bestFailures) } default: - mainResult.AddErrors(mustValidateOnlyOneSchemaMsg(s.Path, fmt.Sprintf("Found %d valid alternatives", validated))) + mainResult.addErrorsAt(s.Path, mustValidateOnlyOneSchemaMsg(s.Path.dotted(), fmt.Sprintf("Found %d valid alternatives", validated))) mainResult.Merge(bestFailures) if firstSuccess != nil && firstSuccess.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(firstSuccess) @@ -260,10 +256,10 @@ func (s *schemaPropsValidator) validateAllOf(data any, mainResult, keepResultAll switch validated { case 0: - mainResult.AddErrors(mustValidateAllSchemasMsg(s.Path, ". None validated")) + mainResult.addErrorsAt(s.Path, mustValidateAllSchemasMsg(s.Path.dotted(), ". None validated")) case len(s.allOfValidators): default: - mainResult.AddErrors(mustValidateAllSchemasMsg(s.Path, "")) + mainResult.addErrorsAt(s.Path, mustValidateAllSchemasMsg(s.Path.dotted(), "")) } } @@ -274,7 +270,7 @@ func (s *schemaPropsValidator) validateNot(data any, mainResult *Result) { } // We keep inner IMPORTANT! errors no matter what MatchCount tells us if result.IsValid() { - mainResult.AddErrors(mustNotValidatechemaMsg(s.Path)) + mainResult.addErrorsAt(s.Path, mustNotValidatechemaMsg(s.Path.dotted())) } if result.wantsRedeemOnMerge { pools.poolOfResults.RedeemResult(result) // this result is ditched @@ -291,7 +287,7 @@ func (s *schemaPropsValidator) validateDependencies(data any, mainResult *Result if dep.Schema != nil { mainResult.Merge( - newSchemaValidator(dep.Schema, s.Root, s.Path+"."+key, s.KnownFormats, s.Options).Validate(data), + newSchemaValidator(dep.Schema, s.Root, s.Path.child(key), s.KnownFormats, s.Options).Validate(data), ) continue } @@ -299,13 +295,17 @@ func (s *schemaPropsValidator) validateDependencies(data any, mainResult *Result if len(dep.Property) > 0 { for _, depKey := range dep.Property { if _, ok := val[depKey]; !ok { - mainResult.AddErrors(hasADependencyMsg(s.Path, depKey)) + mainResult.addErrorsAt(s.Path, hasADependencyMsg(s.Path.dotted(), depKey)) } } } } } +func (s *schemaPropsValidator) setPath(path pathSegments) { + s.Path = path +} + func (s *schemaPropsValidator) redeem() { pools.poolOfSchemaPropsValidators.RedeemValidator(s) } diff --git a/schema_props_test.go b/schema_props_test.go index 3a08add..bd8a203 100644 --- a/schema_props_test.go +++ b/schema_props_test.go @@ -18,15 +18,15 @@ import ( func TestSchemaPropsValidator_EdgeCases(t *testing.T) { t.Run("should validate props against empty validator", func(t *testing.T) { s := newSchemaPropsValidator( - "", "", nil, nil, nil, nil, nil, nil, strfmt.Default, nil) - s.SetPath("path") - assert.EqualT(t, "path", s.Path) + nil, "", nil, nil, nil, nil, nil, nil, strfmt.Default, nil) + s.setPath(newPathSegments("path")) + assert.EqualT(t, "path", s.Path.dotted()) }) t.Run("with allOf", func(t *testing.T) { makeValidator := func() EntityValidator { return newSchemaPropsValidator( - "path", "body", + newPathSegments("path"), "body", []spec.Schema{ *spec.StringProperty(), *spec.StrFmtProperty("date"), @@ -64,7 +64,7 @@ func TestSchemaPropsValidator_EdgeCases(t *testing.T) { t.Run("with oneOf", func(t *testing.T) { makeValidator := func() EntityValidator { return newSchemaPropsValidator( - "path", "body", + newPathSegments("path"), "body", nil, []spec.Schema{ *spec.Int64Property(), @@ -94,7 +94,7 @@ func TestSchemaPropsValidator_EdgeCases(t *testing.T) { t.Run("with anyOf", func(t *testing.T) { makeValidator := func() EntityValidator { return newSchemaPropsValidator( - "path", "body", + newPathSegments("path"), "body", nil, nil, []spec.Schema{ @@ -125,7 +125,7 @@ func TestSchemaPropsValidator_EdgeCases(t *testing.T) { t.Run("with not", func(t *testing.T) { makeValidator := func() EntityValidator { return newSchemaPropsValidator( - "path", "body", + newPathSegments("path"), "body", nil, nil, nil, @@ -182,7 +182,7 @@ func TestSchemaPropsValidator_EdgeCases(t *testing.T) { }, }, nil, - "root", + newPathSegments("root"), strfmt.Default, &SchemaValidatorOptions{recycleValidators: true}) } diff --git a/slice_validator.go b/slice_validator.go index 8f49d13..3445dad 100644 --- a/slice_validator.go +++ b/slice_validator.go @@ -4,7 +4,6 @@ package validate import ( - "fmt" "reflect" "github.com/go-openapi/spec" @@ -12,7 +11,7 @@ import ( ) type schemaSliceValidator struct { - Path string + Path pathSegments In string MaxItems *int64 MinItems *int64 @@ -24,7 +23,7 @@ type schemaSliceValidator struct { Options *SchemaValidatorOptions } -func newSliceValidator(path, in string, +func newSliceValidator(path pathSegments, in string, maxItems, minItems *int64, uniqueItems bool, additionalItems *spec.SchemaOrBool, items *spec.SchemaOrArray, root any, formats strfmt.Registry, opts *SchemaValidatorOptions, @@ -54,10 +53,6 @@ func newSliceValidator(path, in string, return v } -func (s *schemaSliceValidator) SetPath(path string) { - s.Path = path -} - func (s *schemaSliceValidator) Applies(source any, kind reflect.Kind) bool { _, ok := source.(*spec.Schema) r := ok && kind == reflect.Slice @@ -85,8 +80,10 @@ func (s *schemaSliceValidator) Validate(data any) *Result { if s.Items != nil && s.Items.Schema != nil { for i := range size { - validator := newSchemaValidator(s.Items.Schema, s.Root, s.Path, s.KnownFormats, s.Options) - validator.SetPath(fmt.Sprintf("%s.%d", s.Path, i)) + // the index has to reach the constructor: the sub-validators that + // report the error are built there, and setting the path afterwards + // would leave them located on the array rather than on the item. + validator := newSchemaValidator(s.Items.Schema, s.Root, s.Path.item(i), s.KnownFormats, s.Options) value := val.Index(i) result.mergeForSlice(val, i, validator.Validate(value.Interface())) } @@ -100,41 +97,45 @@ func (s *schemaSliceValidator) Validate(data any) *Result { break } - validator := newSchemaValidator(&s.Items.Schemas[i], s.Root, fmt.Sprintf("%s.%d", s.Path, i), s.KnownFormats, s.Options) + validator := newSchemaValidator(&s.Items.Schemas[i], s.Root, s.Path.item(i), s.KnownFormats, s.Options) result.mergeForSlice(val, i, validator.Validate(val.Index(i).Interface())) } } if s.AdditionalItems != nil && itemsSize < size { if s.Items != nil && len(s.Items.Schemas) > 0 && !s.AdditionalItems.Allows { - result.AddErrors(arrayDoesNotAllowAdditionalItemsMsg()) + result.addErrorsAt(s.Path, arrayDoesNotAllowAdditionalItemsMsg()) } if s.AdditionalItems.Schema != nil { for i := itemsSize; i < size-itemsSize+1; i++ { - validator := newSchemaValidator(s.AdditionalItems.Schema, s.Root, fmt.Sprintf("%s.%d", s.Path, i), s.KnownFormats, s.Options) + validator := newSchemaValidator(s.AdditionalItems.Schema, s.Root, s.Path.item(i), s.KnownFormats, s.Options) result.mergeForSlice(val, i, validator.Validate(val.Index(i).Interface())) } } } if s.MinItems != nil { - if err := MinItems(s.Path, s.In, int64(size), *s.MinItems); err != nil { - result.AddErrors(err) + if err := MinItems(s.Path.dotted(), s.In, int64(size), *s.MinItems); err != nil { + result.addErrorsAt(s.Path, err) } } if s.MaxItems != nil { - if err := MaxItems(s.Path, s.In, int64(size), *s.MaxItems); err != nil { - result.AddErrors(err) + if err := MaxItems(s.Path.dotted(), s.In, int64(size), *s.MaxItems); err != nil { + result.addErrorsAt(s.Path, err) } } if s.UniqueItems { - if err := UniqueItems(s.Path, s.In, val.Interface()); err != nil { - result.AddErrors(err) + if err := UniqueItems(s.Path.dotted(), s.In, val.Interface()); err != nil { + result.addErrorsAt(s.Path, err) } } result.Inc() return result } +func (s *schemaSliceValidator) setPath(path pathSegments) { + s.Path = path +} + func (s *schemaSliceValidator) redeem() { pools.poolOfSliceValidators.RedeemValidator(s) } diff --git a/slice_validator_test.go b/slice_validator_test.go index 7fbedd9..6db2954 100644 --- a/slice_validator_test.go +++ b/slice_validator_test.go @@ -13,9 +13,9 @@ import ( // to simulate with specs // (this one is a trivial, just to check all methods are filled). func TestSliceValidator_EdgeCases(t *testing.T) { - s := newSliceValidator("", "", nil, nil, false, nil, nil, nil, nil, nil) - s.SetPath("path") - assert.EqualT(t, "path", s.Path) + s := newSliceValidator(nil, "", nil, nil, false, nil, nil, nil, nil, nil) + s.setPath(newPathSegments("path")) + assert.EqualT(t, "path", s.Path.dotted()) r := s.Validate(nil) assert.NotNil(t, r) diff --git a/spec.go b/spec.go index d6a61ea..5568d38 100644 --- a/spec.go +++ b/spec.go @@ -10,6 +10,7 @@ import ( "fmt" "slices" "sort" + "strconv" "strings" "github.com/go-openapi/analysis" @@ -48,13 +49,15 @@ func Spec(doc *loads.Document, formats strfmt.Registry, options ...Option) error // SpecValidator validates a swagger 2.0 spec. type SpecValidator struct { - schema *spec.Schema // swagger 2.0 schema - spec *loads.Document - analyzer *analysis.Spec - expanded *loads.Document - KnownFormats strfmt.Registry - Options Opts // validation options - schemaOptions *SchemaValidatorOptions + schema *spec.Schema // swagger 2.0 schema + spec *loads.Document + analyzer *analysis.Spec + expanded *loads.Document + refLocations refLocations + paramLocations paramLocations + KnownFormats strfmt.Registry + Options Opts // validation options + schemaOptions *SchemaValidatorOptions } // NewSpecValidator creates a new swagger spec validator instance. @@ -98,6 +101,12 @@ func (s *SpecValidator) Validate(data any) (*Result, *Result) { } s.spec = sd s.analyzer = analysis.New(sd.Spec()) + // where each $ref sits, as authored: refs are reported against the + // unexpanded document, before expansion flattens them away + s.refLocations = newRefLocations(s.analyzer) + // where each operation declares its parameters: the document addresses + // them by index, and expansion loses that + s.paramLocations = newParamLocations(sd.Spec()) // Raw spec unmarshalling errors var obj any @@ -111,11 +120,13 @@ func (s *SpecValidator) Validate(data any) (*Result, *Result) { // errs holds all errors and warnings, // warnings only warnings errs.MergeAsWarnings(warnings) - warnings.AddErrors(errs.Warnings...) + // reported as errors of the warnings-only result, but keeping the + // location each was recorded with + warnings.carryErrors(errs.Warnings, errs.warningLocations) }() // Swagger schema validator - schv := newSchemaValidator(s.schema, nil, "", s.KnownFormats, s.schemaOptions) + schv := newSchemaValidator(s.schema, nil, rootPath(), s.KnownFormats, s.schemaOptions) errs.Merge(schv.Validate(obj)) // error - // There may be a point in continuing to try and determine more accurate errors if !s.Options.ContinueOnErrors && errs.HasErrors() { @@ -170,21 +181,21 @@ func (s *SpecValidator) validateNonEmptyPathParamNames() *Result { res := pools.poolOfResults.BorrowResult() if s.spec.Spec().Paths == nil { // There is no Paths object: error - res.AddErrors(noValidPathMsg()) + res.addErrorsAt(newPathSegments(swaggerPaths), noValidPathMsg()) return res } if s.spec.Spec().Paths.Paths == nil { // Paths may be empty: warning - res.AddWarnings(noValidPathMsg()) + res.addWarningsAt(newPathSegments(swaggerPaths), noValidPathMsg()) return res } for k := range s.spec.Spec().Paths.Paths { if strings.Contains(k, "{}") { - res.AddErrors(emptyPathParameterMsg(k)) + res.addErrorsAt(newPathSegments(swaggerPaths, k), emptyPathParameterMsg(k)) } } @@ -238,7 +249,7 @@ func (s *SpecValidator) validateDuplicatePropertyNames() *Result { res.Merge(rec) } if len(ancs) > 0 { - res.AddErrors(circularAncestryDefinitionMsg(k, ancs)) + res.addErrorsAt(newPathSegments(swaggerDefinitions, k), circularAncestryDefinitionMsg(k, ancs)) return res } @@ -252,7 +263,7 @@ func (s *SpecValidator) validateDuplicatePropertyNames() *Result { for _, v := range dups { pns = append(pns, v.Definition+"."+v.Name) } - res.AddErrors(duplicatePropertiesMsg(k, pns)) + res.addErrorsAt(newPathSegments(swaggerDefinitions, k), duplicatePropertiesMsg(k, pns)) } } @@ -367,7 +378,7 @@ func (s *SpecValidator) validateItems() *Result { for _, param := range paramHelp.safeExpandedParamsFor(path, method, op.ID, res, s) { if param.TypeName() == arrayType && param.ItemsTypeName() == "" { - res.AddErrors(arrayInParamRequiresItemsMsg(param.Name, op.ID)) + res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), arrayInParamRequiresItemsMsg(param.Name, op.ID)) continue } if param.In != swaggerBody { @@ -375,7 +386,7 @@ func (s *SpecValidator) validateItems() *Result { items := param.Items for items.TypeName() == arrayType { if items.ItemsTypeName() == "" { - res.AddErrors(arrayInParamRequiresItemsMsg(param.Name, op.ID)) + res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), arrayInParamRequiresItemsMsg(param.Name, op.ID)) break } items = items.Items @@ -384,32 +395,38 @@ func (s *SpecValidator) validateItems() *Result { } else { // In: body if param.Schema != nil { - res.Merge(s.validateSchemaItems(*param.Schema, fmt.Sprintf("body param %q", param.Name), op.ID)) + res.Merge(s.validateSchemaItems(*param.Schema, s.parameterPath(path, method, param.In, param.Name).child(jsonSchema), + fmt.Sprintf("body param %q", param.Name), op.ID)) } } } - var responses []spec.Response + type codedResponse struct { + code string + resp spec.Response + } + var responses []codedResponse if op.Responses != nil { if op.Responses.Default != nil { - responses = append(responses, *op.Responses.Default) + responses = append(responses, codedResponse{code: jsonDefault, resp: *op.Responses.Default}) } if op.Responses.StatusCodeResponses != nil { - for _, v := range op.Responses.StatusCodeResponses { - responses = append(responses, v) + for code, v := range op.Responses.StatusCodeResponses { + responses = append(responses, codedResponse{code: strconv.Itoa(code), resp: v}) } } } for _, resp := range responses { + at := responsePath(path, method, resp.code) // Response headers with array - for hn, hv := range resp.Headers { + for hn, hv := range resp.resp.Headers { if hv.TypeName() == arrayType && hv.ItemsTypeName() == "" { - res.AddErrors(arrayInHeaderRequiresItemsMsg(hn, op.ID)) + res.addErrorsAt(at.children(swaggerHeaders, hn), arrayInHeaderRequiresItemsMsg(hn, op.ID)) } } - if resp.Schema != nil { - res.Merge(s.validateSchemaItems(*resp.Schema, "response body", op.ID)) + if resp.resp.Schema != nil { + res.Merge(s.validateSchemaItems(*resp.resp.Schema, at.child(jsonSchema), "response body", op.ID)) } } } @@ -418,24 +435,24 @@ func (s *SpecValidator) validateItems() *Result { } // Verifies constraints on array type. -func (s *SpecValidator) validateSchemaItems(schema spec.Schema, prefix, opID string) *Result { +func (s *SpecValidator) validateSchemaItems(schema spec.Schema, at pathSegments, prefix, opID string) *Result { res := pools.poolOfResults.BorrowResult() if !schema.Type.Contains(arrayType) { return res } if schema.Items == nil || schema.Items.Len() == 0 { - res.AddErrors(arrayRequiresItemsMsg(prefix, opID)) + res.addErrorsAt(at, arrayRequiresItemsMsg(prefix, opID)) return res } if schema.Items.Schema != nil { schema = *schema.Items.Schema if _, err := compileRegexp(schema.Pattern); err != nil { - res.AddErrors(invalidItemsPatternMsg(prefix, opID, schema.Pattern)) + res.addErrorsAt(at, invalidItemsPatternMsg(prefix, opID, schema.Pattern)) } - res.Merge(s.validateSchemaItems(schema, prefix, opID)) + res.Merge(s.validateSchemaItems(schema, at.child(jsonItems), prefix, opID)) } return res } @@ -453,7 +470,7 @@ func (s *SpecValidator) validatePathParamPresence(path string, fromPath, fromOpe } } if !matched { - res.AddErrors(noParameterInPathMsg(l)) + res.addErrorsAt(newPathSegments(swaggerPaths, l), noParameterInPathMsg(l)) } } @@ -463,7 +480,7 @@ func (s *SpecValidator) validatePathParamPresence(path string, fromPath, fromOpe matched = true } if !matched { - res.AddErrors(pathParamNotInPathMsg(path, p)) + res.addErrorsAt(newPathSegments(swaggerPaths, path), pathParamNotInPathMsg(path, p)) } } @@ -498,7 +515,7 @@ func (s *SpecValidator) validateReferencedParameters() *Result { } result := pools.poolOfResults.BorrowResult() for k := range expected { - result.AddWarnings(unusedParamMsg(k)) + result.addWarningsAt(localRefPath(k), unusedParamMsg(k)) } return result } @@ -523,7 +540,7 @@ func (s *SpecValidator) validateReferencedResponses() *Result { } result := pools.poolOfResults.BorrowResult() for k := range expected { - result.AddWarnings(unusedResponseMsg(k)) + result.addWarningsAt(localRefPath(k), unusedResponseMsg(k)) } return result } @@ -549,7 +566,7 @@ func (s *SpecValidator) validateReferencedDefinitions() *Result { result := new(Result) for k := range expected { - result.AddWarnings(unusedDefinitionMsg(k)) + result.addWarningsAt(localRefPath(k), unusedDefinitionMsg(k)) } return result } @@ -561,8 +578,12 @@ func (s *SpecValidator) validateRequiredDefinitions() *Result { DEFINITIONS: for d, schema := range s.spec.Spec().Definitions { if schema.Required != nil { // Safeguard - for _, pn := range schema.Required { - red := s.validateRequiredProperties(pn, d, &schema) //#nosec + definitionAt := newPathSegments(swaggerDefinitions, d) + for i, pn := range schema.Required { + // the offending entry of the required array, not the definition + // holding it: that is what a reader has to go and amend + requiredAt := definitionAt.child(jsonRequired).item(i) + red := s.validateRequiredProperties(pn, d, definitionAt, requiredAt, &schema) //#nosec // NOTE: capture validity before merging: Merge may redeem `red` to the // pool (wantsRedeemOnMerge), after which reading it races with a concurrent // BorrowResult().cleared() in another goroutine sharing the global pool. @@ -577,7 +598,14 @@ DEFINITIONS: return res } -func (s *SpecValidator) validateRequiredProperties(path, in string, v *spec.Schema) *Result { +// validateRequiredProperties checks one entry of a definition's required array. +// +// schemaAt locates the schema being searched for the property, which moves as +// the search descends into additionalProperties. requiredAt locates the entry +// of the required array that started it, and stays put. +func (s *SpecValidator) validateRequiredProperties( + path, in string, schemaAt, requiredAt pathSegments, v *spec.Schema, +) *Result { // Takes care of recursive property definitions, which may be nested in additionalProperties schemas res := pools.poolOfResults.BorrowResult() propertyMatch := false @@ -596,7 +624,7 @@ func (s *SpecValidator) validateRequiredProperties(path, in string, v *spec.Sche for pp, pv := range v.PatternProperties { re, err := compileRegexp(pp) if err != nil { - res.AddErrors(invalidPatternMsg(pp, in)) + res.addErrorsAt(schemaAt, invalidPatternMsg(pp, in)) } else if re.MatchString(path) { patternMatch = true if !propertyMatch { @@ -613,7 +641,7 @@ func (s *SpecValidator) validateRequiredProperties(path, in string, v *spec.Sche // additionalProperties as schema are upported in swagger // recursively validates additionalProperties schema // Proposal for enhancement: anyOf, allOf, oneOf like in schemaPropsValidator - red := s.validateRequiredProperties(path, in, v.AdditionalProperties.Schema) + red := s.validateRequiredProperties(path, in, schemaAt.child(jsonAdditionalProperties), requiredAt, v.AdditionalProperties.Schema) if red.IsValid() { additionalPropertiesMatch = true if !propertyMatch && !patternMatch { @@ -626,11 +654,11 @@ func (s *SpecValidator) validateRequiredProperties(path, in string, v *spec.Sche } if !propertyMatch && !patternMatch && !additionalPropertiesMatch { - res.AddErrors(requiredButNotDefinedMsg(path, in)) + res.addErrorsAt(requiredAt, requiredButNotDefinedMsg(path, in)) } if isReadOnly { - res.AddWarnings(readOnlyAndRequiredMsg(in, path)) + res.addWarningsAt(requiredAt, readOnlyAndRequiredMsg(in, path)) } return res } @@ -656,16 +684,16 @@ func (s *SpecValidator) validateParameters() *Result { // Warn on garbled path afer param stripping if rexGarbledPathSegment.MatchString(pathToAdd) { - res.AddWarnings(pathStrippedParamGarbledMsg(pathToAdd)) + res.addWarningsAt(newPathSegments(swaggerPaths, path), pathStrippedParamGarbledMsg(pathToAdd)) } // Check uniqueness of stripped paths if _, found := methodPaths[method][pathToAdd]; found { // Sort names for stable, testable output if strings.Compare(path, methodPaths[method][pathToAdd]) < 0 { - res.AddErrors(pathOverlapMsg(path, methodPaths[method][pathToAdd])) + res.addErrorsAt(newPathSegments(swaggerPaths, path), pathOverlapMsg(path, methodPaths[method][pathToAdd])) } else { - res.AddErrors(pathOverlapMsg(methodPaths[method][pathToAdd], path)) + res.addErrorsAt(newPathSegments(swaggerPaths, path), pathOverlapMsg(methodPaths[method][pathToAdd], path)) } } else { if _, found := methodPaths[method]; !found { @@ -697,10 +725,10 @@ func (s *SpecValidator) validateParameters() *Result { for _, pr := range paramHelp.safeExpandedParamsFor(path, method, op.ID, res, s) { // An expanded parameter must validate the Parameter schema (an unexpanded $ref always passes high-level schema validation) - schv := newSchemaValidator(¶mSchema, s.schema, fmt.Sprintf("%s.%s.parameters.%s", path, method, pr.Name), s.KnownFormats, s.schemaOptions) + schv := newSchemaValidator(¶mSchema, s.schema, s.parameterPath(path, method, pr.In, pr.Name), s.KnownFormats, s.schemaOptions) var obj any if err := jsonutils.FromDynamicJSON(pr, &obj); err != nil { - res.AddErrors(err) + res.addErrorsAt(s.parameterPath(path, method, pr.In, pr.Name), err) return res } @@ -709,7 +737,7 @@ func (s *SpecValidator) validateParameters() *Result { // Validate pattern regexp for parameters with a Pattern property if _, err := compileRegexp(pr.Pattern); err != nil { - res.AddErrors(invalidPatternInParamMsg(op.ID, pr.Name, pr.Pattern)) + res.addErrorsAt(s.parameterPath(path, method, pr.In, pr.Name), invalidPatternInParamMsg(op.ID, pr.Name, pr.Pattern)) } // There must be at most one parameter in body: list them all @@ -722,7 +750,7 @@ func (s *SpecValidator) validateParameters() *Result { paramNames = append(paramNames, pr.Name) // Path declared in path must have the required: true property if !pr.Required { - res.AddErrors(pathParamRequiredMsg(op.ID, pr.Name)) + res.addErrorsAt(s.parameterPath(path, method, pr.In, pr.Name), pathParamRequiredMsg(op.ID, pr.Name)) } } @@ -733,31 +761,31 @@ func (s *SpecValidator) validateParameters() *Result { if pr.Type != numberType && pr.Type != integerType && (pr.Maximum != nil || pr.Minimum != nil || pr.MultipleOf != nil) { // A non-numeric parameter has validation keywords for numeric instances (number and integer) - res.AddWarnings(parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) + res.addWarningsAt(s.parameterPath(path, method, pr.In, pr.Name), parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) } if pr.Type != stringType && // A non-string parameter has validation keywords for strings (pr.MaxLength != nil || pr.MinLength != nil || pr.Pattern != "") { - res.AddWarnings(parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) + res.addWarningsAt(s.parameterPath(path, method, pr.In, pr.Name), parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) } if pr.Type != arrayType && // A non-array parameter has validation keywords for arrays (pr.MaxItems != nil || pr.MinItems != nil || pr.UniqueItems) { - res.AddWarnings(parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) + res.addWarningsAt(s.parameterPath(path, method, pr.In, pr.Name), parameterValidationTypeMismatchMsg(pr.Name, path, pr.Type)) } } // In:formData and In:body are mutually exclusive if hasBody && hasForm { - res.AddErrors(bothFormDataAndBodyMsg(op.ID)) + res.addErrorsAt(operationPath(path, method), bothFormDataAndBodyMsg(op.ID)) } // There must be at most one body param // Accurately report situations when more than 1 body param is declared (possibly unnamed) if len(bodyParams) > 1 { sort.Strings(bodyParams) - res.AddErrors(multipleBodyParamMsg(op.ID, bodyParams)) + res.addErrorsAt(operationPath(path, method), multipleBodyParamMsg(op.ID, bodyParams)) } // Check uniqueness of parameters in path @@ -765,7 +793,7 @@ func (s *SpecValidator) validateParameters() *Result { for i, p := range paramsInPath { for j, q := range paramsInPath { if p == q && i > j { - res.AddErrors(pathParamNotUniqueMsg(path, p, q)) + res.addErrorsAt(newPathSegments(swaggerPaths, path), pathParamNotUniqueMsg(path, p, q)) break } } @@ -775,7 +803,7 @@ func (s *SpecValidator) validateParameters() *Result { rexGarbledParam := mustCompileRegexp(`{.*[{}\s]+.*}`) for _, p := range paramsInPath { if rexGarbledParam.MatchString(p) { - res.AddWarnings(pathParamGarbledMsg(path, p)) + res.addWarningsAt(newPathSegments(swaggerPaths, path), pathParamGarbledMsg(path, p)) } } @@ -791,7 +819,7 @@ func (s *SpecValidator) validateReferencesValid() *Result { res := pools.poolOfResults.BorrowResult() for _, r := range s.analyzer.AllRefs() { if !r.IsValidURI(s.spec.SpecFilePath()) { // Safeguard - spec should always yield a valid URI - res.AddErrors(invalidRefMsg(r.String())) + res.addErrorsAt(s.refLocations.at(r.String()), invalidRefMsg(r.String())) } } if !res.HasErrors() { @@ -830,7 +858,7 @@ func (s *SpecValidator) checkUniqueParams(path, method string, op *spec.Operatio key := fmt.Sprintf("%s#%s", pr.In, pr.Name) if _, ok = pnames[key]; ok { - res.AddErrors(duplicateParamNameMsg(pr.In, pr.Name, op.ID)) + res.addErrorsAt(s.parameterPath(path, method, pr.In, pr.Name), duplicateParamNameMsg(pr.In, pr.Name, op.ID)) } pnames[key] = struct{}{} } diff --git a/spec_ref_warnings.go b/spec_ref_warnings.go index 49c7231..a499a64 100644 --- a/spec_ref_warnings.go +++ b/spec_ref_warnings.go @@ -48,7 +48,7 @@ func (s *SpecValidator) validateDubiousRefs() *Result { // Rule 1: absolute local reference escaping the base path. if refPath, isLocalAbs := absoluteLocalRefPath(r, u); isLocalAbs { if !hasBase || !isBeneathBase(refPath, baseDir) { - res.AddWarnings(dubiousAbsoluteRefMsg(r.String())) + res.addWarningsAt(s.refLocations.at(r.String()), dubiousAbsoluteRefMsg(r.String())) } continue } diff --git a/spec_ref_warnings_test.go b/spec_ref_warnings_test.go index f013c49..3700430 100644 --- a/spec_ref_warnings_test.go +++ b/spec_ref_warnings_test.go @@ -148,6 +148,10 @@ func dubiousValidatorFromJSON(t *testing.T, doc string) *SpecValidator { s := NewSpecValidator(d.Schema(), strfmt.Default) s.spec = d s.analyzer = analysis.New(d.Spec()) + + // as Validate does, so that warnings can say where a $ref sits + s.refLocations = newRefLocations(s.analyzer) + return s } diff --git a/spec_test.go b/spec_test.go index 43c20f6..0b8f6eb 100644 --- a/spec_test.go +++ b/spec_test.go @@ -107,7 +107,7 @@ func TestSpec_Issue52(t *testing.T) { res := schemaValidator.Validate(&sch) assert.FalseT(t, res.IsValid()) require.NotEmpty(t, res.Errors) - require.EqualError(t, res.Errors[0], ".paths in body is required") + require.EqualError(t, res.Errors[0], "paths in body is required") // as swagger spec: path is set to nil // Here, validation stops as paths is initialized to empty @@ -116,7 +116,7 @@ func TestSpec_Issue52(t *testing.T) { verifiedErrors := verifiedTestErrors(res) assert.Len(t, verifiedErrors, 2, "Unexpected number of error messages returned") - assert.SliceContainsT(t, verifiedErrors, ".paths in body is required") + assert.SliceContainsT(t, verifiedErrors, "paths in body is required") assert.SliceContainsT(t, verifiedErrors, "spec has no valid path defined") } @@ -132,13 +132,13 @@ func TestSpec_Issue53(t *testing.T) { res := schemaValidator.Validate(&sch) assert.FalseT(t, res.IsValid()) require.NotEmpty(t, res.Errors) - require.EqualError(t, res.Errors[0], ".swagger in body is required") + require.EqualError(t, res.Errors[0], "swagger in body is required") // as swagger despec res, _ = loadAndValidate(t, fp, false) require.FalseT(t, res.IsValid()) require.NotEmpty(t, res.Errors) - require.EqualError(t, res.Errors[0], ".swagger in body is required") + require.EqualError(t, res.Errors[0], "swagger in body is required") } func TestSpec_Issue62(t *testing.T) { @@ -215,7 +215,8 @@ func TestSpec_Issue18(t *testing.T) { verifiedErrors := verifiedTestErrors(res) switch { case strings.Contains(path, "headerItems.json"): - assert.SliceContainsT(t, verifiedErrors, "X-Foo in header has invalid pattern: \")<-- bad pattern\"") + assert.SliceContainsT(t, verifiedErrors, + "paths./foo.get.responses.default.headers.X-Foo in header has invalid pattern: \")<-- bad pattern\"") case strings.Contains(path, "headers.json"): const badPatternSuffix = ` has invalid pattern ")<-- bad pattern": error parsing regexp: unexpected ): ` + "`)<-- bad pattern`" @@ -226,14 +227,14 @@ func TestSpec_Issue18(t *testing.T) { case strings.Contains(path, "paramItems.json"): assert.SliceContainsT(t, verifiedErrors, "body param \"user\" for \"\" has invalid items pattern: \")<-- bad pattern\"") - // Updated message: from "user.items in body has invalid pattern: \")<-- bad pattern\"" to: assert.SliceContainsT(t, verifiedErrors, "default value for user in body does not validate its schema") - assert.SliceContainsT(t, verifiedErrors, "user.items.default in body has invalid pattern: \")<-- bad pattern\"") + assert.SliceContainsT(t, verifiedErrors, + "paths./foo.parameters.user.items in body has invalid pattern: \")<-- bad pattern\"") case strings.Contains(path, "parameters.json"): assert.SliceContainsT(t, verifiedErrors, "operation \"\" has invalid pattern in param \"userId\": \")<-- bad pattern\"") case strings.Contains(path, "schema.json"): - // NOTE: strange that the text does not say response "200"... - assert.SliceContainsT(t, verifiedErrors, "200 in response has invalid pattern: \")<-- bad pattern\"") + assert.SliceContainsT(t, verifiedErrors, + "paths./foo.get.responses.200 in response has invalid pattern: \")<-- bad pattern\"") default: t.Logf("Returned error messages: %v", verifiedErrors) t.Fatal("fixture not tested. Please add assertions for messages") @@ -404,6 +405,7 @@ func TestSpec_ValidateParameters(t *testing.T) { validator := NewSpecValidator(spec.MustLoadSwagger20Schema(), strfmt.Default) validator.spec = doc validator.analyzer = analysis.New(doc.Spec()) + validator.paramLocations = newParamLocations(doc.Spec()) return validator } @@ -460,10 +462,10 @@ func TestSpec_ValidateParameters(t *testing.T) { res := validator.validateParameters() require.Len(t, res.Errors, 2) assert.StringContainsT(t, res.Errors[0].Error(), - `"/pets.POST.parameters.pet" must validate one and only one schema (oneOf). Found none valid`, + `"paths./pets.post.parameters.pet" must validate one and only one schema (oneOf). Found none valid`, ) assert.StringContainsT(t, res.Errors[1].Error(), - `/pets.POST.parameters.pet.schema.anyOf in body is a forbidden property`, + `paths./pets.post.parameters.pet.schema.anyOf in body is a forbidden property`, ) }) t.Run("with loads.Spec", func(t *testing.T) { @@ -561,16 +563,16 @@ func TestSpec_ValidateParameters(t *testing.T) { err = Spec(doc, strfmt.Default) require.Error(t, err) require.ErrorContains(t, err, - `/deposits.GET.parameters..enum in body is a forbidden property`, + `paths./deposits.get.parameters..enum in body is a forbidden property`, ) require.ErrorContains(t, err, - `deposits.GET.parameters..type in body is a forbidden property`, + `paths./deposits.get.parameters..type in body is a forbidden property`, ) require.ErrorContains(t, err, - `/deposits.GET.parameters..name in body is required`, + `paths./deposits.get.parameters..name in body is required`, ) require.ErrorContains(t, err, - `/deposits.GET.parameters..in in body is required`, + `paths./deposits.get.parameters..in in body is required`, ) }) @@ -606,18 +608,24 @@ func TestSpec_ValidateParameters(t *testing.T) { require.NoError(t, err) errs, warns := NewSpecValidator(doc.Schema(), strfmt.Default).Validate(doc) - require.Len(t, errs.Errors, 3) + + // the fixture declares "type: [zilk, zork]": both entries are invalid and + // each is reported on its own index. + require.Len(t, errs.Errors, 4) require.Empty(t, warns.Errors) - var found1, found2, found3 int + const oneOfTypes = ` in body should be one of [array boolean integer null number object string]` + var found1, found2, found3, found4 int for _, err := range errs.Errors { switch { case strings.Contains(err.Error(), `definitions.WrongSchema.descriptions in body is a forbidden property`): found1++ case strings.Contains(err.Error(), `"definitions.WrongSchema.type" must validate at least one schema (anyOf)`): found2++ - case strings.Contains(err.Error(), `definitions.WrongSchema.type in body should be one of [array boolean integer null number object string]`): + case strings.Contains(err.Error(), `definitions.WrongSchema.type.0`+oneOfTypes): found3++ + case strings.Contains(err.Error(), `definitions.WrongSchema.type.1`+oneOfTypes): + found4++ } } @@ -625,6 +633,7 @@ func TestSpec_ValidateParameters(t *testing.T) { require.EqualT(t, 1, found1) require.EqualT(t, 1, found2) require.EqualT(t, 1, found3) + require.EqualT(t, 1, found4) }) }) }) diff --git a/type.go b/type.go index d29574c..9baa3d6 100644 --- a/type.go +++ b/type.go @@ -15,7 +15,7 @@ import ( ) type typeValidator struct { - Path string + Path pathSegments In string Type spec.StringOrArray Nullable bool @@ -23,7 +23,7 @@ type typeValidator struct { Options *SchemaValidatorOptions } -func newTypeValidator(path, in string, typ spec.StringOrArray, nullable bool, format string, opts *SchemaValidatorOptions) *typeValidator { +func newTypeValidator(path pathSegments, in string, typ spec.StringOrArray, nullable bool, format string, opts *SchemaValidatorOptions) *typeValidator { if opts == nil { opts = new(SchemaValidatorOptions) } @@ -45,10 +45,6 @@ func newTypeValidator(path, in string, typ spec.StringOrArray, nullable bool, fo return t } -func (t *typeValidator) SetPath(path string) { - t.Path = path -} - func (t *typeValidator) Applies(source any, _ reflect.Kind) bool { // typeValidator applies to Schema, Parameter and Header objects switch source.(type) { @@ -72,7 +68,7 @@ func (t *typeValidator) Validate(data any) *Result { if data == nil { // nil or zero value for the passed structure require Type: null if len(t.Type) > 0 && !t.Type.Contains(nullType) && !t.Nullable { // NOTE: if a property is not required it also passes this - return errorHelp.sErr(errors.InvalidType(t.Path, t.In, strings.Join(t.Type, ","), nullType), t.Options.recycleResult) + return errorHelp.sErrAt(t.Path, errors.InvalidType(t.Path.dotted(), t.In, strings.Join(t.Type, ","), nullType), t.Options.recycleResult) } return emptyResult @@ -98,7 +94,7 @@ func (t *typeValidator) Validate(data any) *Result { !isFloatInt && !isIntFloat && !isLowerInt && !isLowerFloat if formatMismatch { // NOTE: test case - return errorHelp.sErr(errors.InvalidType(t.Path, t.In, t.Format, format), t.Options.recycleResult) + return errorHelp.sErrAt(t.Path, errors.InvalidType(t.Path.dotted(), t.In, t.Format, format), t.Options.recycleResult) } if !t.Type.Contains(numberType) && !t.Type.Contains(integerType) && t.Format != "" && (kind == reflect.String || kind == reflect.Slice) { @@ -106,7 +102,7 @@ func (t *typeValidator) Validate(data any) *Result { } if !t.Type.Contains(schType) && !isFloatInt && !isIntFloat { - return errorHelp.sErr(errors.InvalidType(t.Path, t.In, strings.Join(t.Type, ","), schType), t.Options.recycleResult) + return errorHelp.sErrAt(t.Path, errors.InvalidType(t.Path.dotted(), t.In, strings.Join(t.Type, ","), schType), t.Options.recycleResult) } return emptyResult @@ -201,6 +197,10 @@ func (t *typeValidator) schemaInfoForType(data any) (string, string) { return "", "" } +func (t *typeValidator) setPath(path pathSegments) { + t.Path = path +} + func (t *typeValidator) redeem() { pools.poolOfTypeValidators.RedeemValidator(t) } diff --git a/type_test.go b/type_test.go index 6f77813..84f4876 100644 --- a/type_test.go +++ b/type_test.go @@ -256,7 +256,7 @@ func TestType_schemaInfoForType(t *testing.T) { } v := newTypeValidator( - "", "", nil, false, "", nil, + nil, "", nil, false, "", nil, ) t.Run("should not apply", func(t *testing.T) { diff --git a/validator.go b/validator.go index e7aebc5..46a32ca 100644 --- a/validator.go +++ b/validator.go @@ -4,7 +4,6 @@ package validate import ( - "fmt" "reflect" "github.com/go-openapi/errors" @@ -18,7 +17,7 @@ type EntityValidator interface { } type valueValidator interface { - SetPath(path string) + setPath(path pathSegments) Applies(source any, kind reflect.Kind) bool Validate(data any) *Result } @@ -26,14 +25,14 @@ type valueValidator interface { type itemsValidator struct { items *spec.Items root any - path string + path pathSegments in string validators [6]valueValidator KnownFormats strfmt.Registry Options *SchemaValidatorOptions } -func newItemsValidator(path, in string, items *spec.Items, root any, formats strfmt.Registry, opts *SchemaValidatorOptions) *itemsValidator { +func newItemsValidator(path pathSegments, in string, items *spec.Items, root any, formats strfmt.Registry, opts *SchemaValidatorOptions) *itemsValidator { if opts == nil { opts = new(SchemaValidatorOptions) } @@ -79,7 +78,7 @@ func (i *itemsValidator) Validate(index int, data any) *Result { result = new(Result) } - path := fmt.Sprintf("%s.%d", i.path, index) + path := i.path.item(index) for idx, validator := range i.validators { if !validator.Applies(i.root, kind) { @@ -97,7 +96,7 @@ func (i *itemsValidator) Validate(index int, data any) *Result { continue } - validator.SetPath(path) + validator.setPath(path) err := validator.Validate(data) if i.Options.recycleValidators { i.validators[idx] = nil // prevents further (unsafe) usage @@ -130,7 +129,7 @@ func (i *itemsValidator) typeValidator() valueValidator { func (i *itemsValidator) commonValidator() valueValidator { return newBasicCommonValidator( - "", + nil, // located by the item index, set on each Validate call i.in, i.items.Default, i.items.Enum, @@ -140,7 +139,7 @@ func (i *itemsValidator) commonValidator() valueValidator { func (i *itemsValidator) sliceValidator() valueValidator { return newBasicSliceValidator( - "", + nil, // located by the item index, set on each Validate call i.in, i.items.Default, i.items.MaxItems, @@ -155,7 +154,7 @@ func (i *itemsValidator) sliceValidator() valueValidator { func (i *itemsValidator) numberValidator() valueValidator { return newNumberValidator( - "", + nil, // located by the item index, set on each Validate call i.in, i.items.Default, i.items.MultipleOf, @@ -171,7 +170,7 @@ func (i *itemsValidator) numberValidator() valueValidator { func (i *itemsValidator) stringValidator() valueValidator { return newStringValidator( - "", + nil, // located by the item index, set on each Validate call i.in, i.items.Default, false, // Required @@ -185,7 +184,7 @@ func (i *itemsValidator) stringValidator() valueValidator { func (i *itemsValidator) formatValidator() valueValidator { return newFormatValidator( - "", + nil, // located by the item index, set on each Validate call i.in, i.items.Format, i.KnownFormats, @@ -213,14 +212,14 @@ func (i *itemsValidator) redeemChildren() { } type basicCommonValidator struct { - Path string + Path pathSegments In string Default any Enum []any Options *SchemaValidatorOptions } -func newBasicCommonValidator(path, in string, def any, enum []any, opts *SchemaValidatorOptions) *basicCommonValidator { +func newBasicCommonValidator(path pathSegments, in string, def any, enum []any, opts *SchemaValidatorOptions) *basicCommonValidator { if opts == nil { opts = new(SchemaValidatorOptions) } @@ -241,10 +240,6 @@ func newBasicCommonValidator(path, in string, def any, enum []any, opts *SchemaV return b } -func (b *basicCommonValidator) SetPath(path string) { - b.Path = path -} - func (b *basicCommonValidator) Applies(source any, _ reflect.Kind) bool { switch source.(type) { case *spec.Parameter, *spec.Schema, *spec.Header: @@ -279,7 +274,11 @@ func (b *basicCommonValidator) Validate(data any) (res *Result) { } } - return errorHelp.sErr(errors.EnumFail(b.Path, b.In, data, b.Enum), b.Options.recycleResult) + return errorHelp.sErrAt(b.Path, errors.EnumFail(b.Path.dotted(), b.In, data, b.Enum), b.Options.recycleResult) +} + +func (b *basicCommonValidator) setPath(path pathSegments) { + b.Path = path } func (b *basicCommonValidator) redeem() { @@ -323,7 +322,7 @@ func newHeaderValidator(name string, header *spec.Header, formats strfmt.Registr p.Options = opts p.validators = [6]valueValidator{ newTypeValidator( - name, + newPathSegments(name), "header", spec.StringOrArray([]string{header.Type}), header.Nullable, @@ -397,7 +396,7 @@ func (p *HeaderValidator) Validate(data any) *Result { func (p *HeaderValidator) commonValidator() valueValidator { return newBasicCommonValidator( - p.name, + newPathSegments(p.name), "response", p.header.Default, p.header.Enum, @@ -407,7 +406,7 @@ func (p *HeaderValidator) commonValidator() valueValidator { func (p *HeaderValidator) sliceValidator() valueValidator { return newBasicSliceValidator( - p.name, + newPathSegments(p.name), "response", p.header.Default, p.header.MaxItems, @@ -422,7 +421,7 @@ func (p *HeaderValidator) sliceValidator() valueValidator { func (p *HeaderValidator) numberValidator() valueValidator { return newNumberValidator( - p.name, + newPathSegments(p.name), "response", p.header.Default, p.header.MultipleOf, @@ -438,7 +437,7 @@ func (p *HeaderValidator) numberValidator() valueValidator { func (p *HeaderValidator) stringValidator() valueValidator { return newStringValidator( - p.name, + newPathSegments(p.name), "response", p.header.Default, true, @@ -452,7 +451,7 @@ func (p *HeaderValidator) stringValidator() valueValidator { func (p *HeaderValidator) formatValidator() valueValidator { return newFormatValidator( - p.name, + newPathSegments(p.name), "response", p.header.Format, p.KnownFormats, @@ -514,7 +513,7 @@ func newParamValidator(param *spec.Parameter, formats strfmt.Registry, opts *Sch p.Options = opts p.validators = [6]valueValidator{ newTypeValidator( - param.Name, + newPathSegments(param.Name), param.In, spec.StringOrArray([]string{param.Type}), param.Nullable, @@ -589,7 +588,7 @@ func (p *ParamValidator) Validate(data any) *Result { func (p *ParamValidator) commonValidator() valueValidator { return newBasicCommonValidator( - p.param.Name, + newPathSegments(p.param.Name), p.param.In, p.param.Default, p.param.Enum, @@ -599,7 +598,7 @@ func (p *ParamValidator) commonValidator() valueValidator { func (p *ParamValidator) sliceValidator() valueValidator { return newBasicSliceValidator( - p.param.Name, + newPathSegments(p.param.Name), p.param.In, p.param.Default, p.param.MaxItems, @@ -614,7 +613,7 @@ func (p *ParamValidator) sliceValidator() valueValidator { func (p *ParamValidator) numberValidator() valueValidator { return newNumberValidator( - p.param.Name, + newPathSegments(p.param.Name), p.param.In, p.param.Default, p.param.MultipleOf, @@ -630,7 +629,7 @@ func (p *ParamValidator) numberValidator() valueValidator { func (p *ParamValidator) stringValidator() valueValidator { return newStringValidator( - p.param.Name, + newPathSegments(p.param.Name), p.param.In, p.param.Default, p.param.Required, @@ -644,7 +643,7 @@ func (p *ParamValidator) stringValidator() valueValidator { func (p *ParamValidator) formatValidator() valueValidator { return newFormatValidator( - p.param.Name, + newPathSegments(p.param.Name), p.param.In, p.param.Format, p.KnownFormats, @@ -672,7 +671,7 @@ func (p *ParamValidator) redeemChildren() { } type basicSliceValidator struct { - Path string + Path pathSegments In string Default any MaxItems *int64 @@ -685,7 +684,7 @@ type basicSliceValidator struct { } func newBasicSliceValidator( - path, in string, + path pathSegments, in string, def any, maxItems, minItems *int64, uniqueItems bool, items *spec.Items, source any, formats strfmt.Registry, opts *SchemaValidatorOptions, @@ -715,10 +714,6 @@ func newBasicSliceValidator( return s } -func (s *basicSliceValidator) SetPath(path string) { - s.Path = path -} - func (s *basicSliceValidator) Applies(source any, kind reflect.Kind) bool { switch source.(type) { case *spec.Parameter, *spec.Items, *spec.Header: @@ -738,20 +733,20 @@ func (s *basicSliceValidator) Validate(data any) *Result { size := int64(val.Len()) if s.MinItems != nil { - if err := MinItems(s.Path, s.In, size, *s.MinItems); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := MinItems(s.Path.dotted(), s.In, size, *s.MinItems); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } if s.MaxItems != nil { - if err := MaxItems(s.Path, s.In, size, *s.MaxItems); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := MaxItems(s.Path.dotted(), s.In, size, *s.MaxItems); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } if s.UniqueItems { - if err := UniqueItems(s.Path, s.In, data); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := UniqueItems(s.Path.dotted(), s.In, data); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } @@ -775,12 +770,16 @@ func (s *basicSliceValidator) Validate(data any) *Result { return nil } +func (s *basicSliceValidator) setPath(path pathSegments) { + s.Path = path +} + func (s *basicSliceValidator) redeem() { pools.poolOfBasicSliceValidators.RedeemValidator(s) } type numberValidator struct { - Path string + Path pathSegments In string Default any MultipleOf *float64 @@ -795,7 +794,7 @@ type numberValidator struct { } func newNumberValidator( - path, in string, def any, + path pathSegments, in string, def any, multipleOf, maximum *float64, exclusiveMaximum bool, minimum *float64, exclusiveMinimum bool, typ, format string, opts *SchemaValidatorOptions, @@ -826,10 +825,6 @@ func newNumberValidator( return n } -func (n *numberValidator) SetPath(path string) { - n.Path = path -} - func (n *numberValidator) Applies(source any, kind reflect.Kind) bool { switch source.(type) { case *spec.Parameter, *spec.Schema, *spec.Items, *spec.Header: @@ -881,22 +876,22 @@ func (n *numberValidator) Validate(val any) *Result { data := valueHelp.asFloat64(val) // Is the provided value within the range of the specified numeric type and format? - res.AddErrors(IsValueValidAgainstRange(val, n.Type, n.Format, "Checked", n.Path)) + res.addErrorsAt(n.Path, IsValueValidAgainstRange(val, n.Type, n.Format, "Checked", n.Path.dotted())) if n.MultipleOf != nil { resMultiple = pools.poolOfResults.BorrowResult() // Is the constraint specifier within the range of the specific numeric type and format? - resMultiple.AddErrors(IsValueValidAgainstRange(*n.MultipleOf, n.Type, n.Format, "MultipleOf", n.Path)) + resMultiple.addErrorsAt(n.Path, IsValueValidAgainstRange(*n.MultipleOf, n.Type, n.Format, "MultipleOf", n.Path.dotted())) if resMultiple.IsValid() { // Constraint validated with compatible types - if err := MultipleOfNativeType(n.Path, n.In, val, *n.MultipleOf); err != nil { - resMultiple.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := MultipleOfNativeType(n.Path.dotted(), n.In, val, *n.MultipleOf); err != nil { + resMultiple.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } else { // Constraint nevertheless validated, converted as general number - if err := MultipleOf(n.Path, n.In, data, *n.MultipleOf); err != nil { - resMultiple.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := MultipleOf(n.Path.dotted(), n.In, data, *n.MultipleOf); err != nil { + resMultiple.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } } @@ -905,16 +900,16 @@ func (n *numberValidator) Validate(val any) *Result { resMaximum = pools.poolOfResults.BorrowResult() // Is the constraint specifier within the range of the specific numeric type and format? - resMaximum.AddErrors(IsValueValidAgainstRange(*n.Maximum, n.Type, n.Format, "Maximum boundary", n.Path)) + resMaximum.addErrorsAt(n.Path, IsValueValidAgainstRange(*n.Maximum, n.Type, n.Format, "Maximum boundary", n.Path.dotted())) if resMaximum.IsValid() { // Constraint validated with compatible types - if err := MaximumNativeType(n.Path, n.In, val, *n.Maximum, n.ExclusiveMaximum); err != nil { - resMaximum.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := MaximumNativeType(n.Path.dotted(), n.In, val, *n.Maximum, n.ExclusiveMaximum); err != nil { + resMaximum.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } else { // Constraint nevertheless validated, converted as general number - if err := Maximum(n.Path, n.In, data, *n.Maximum, n.ExclusiveMaximum); err != nil { - resMaximum.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := Maximum(n.Path.dotted(), n.In, data, *n.Maximum, n.ExclusiveMaximum); err != nil { + resMaximum.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } } @@ -923,16 +918,16 @@ func (n *numberValidator) Validate(val any) *Result { resMinimum = pools.poolOfResults.BorrowResult() // Is the constraint specifier within the range of the specific numeric type and format? - resMinimum.AddErrors(IsValueValidAgainstRange(*n.Minimum, n.Type, n.Format, "Minimum boundary", n.Path)) + resMinimum.addErrorsAt(n.Path, IsValueValidAgainstRange(*n.Minimum, n.Type, n.Format, "Minimum boundary", n.Path.dotted())) if resMinimum.IsValid() { // Constraint validated with compatible types - if err := MinimumNativeType(n.Path, n.In, val, *n.Minimum, n.ExclusiveMinimum); err != nil { - resMinimum.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := MinimumNativeType(n.Path.dotted(), n.In, val, *n.Minimum, n.ExclusiveMinimum); err != nil { + resMinimum.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } else { // Constraint nevertheless validated, converted as general number - if err := Minimum(n.Path, n.In, data, *n.Minimum, n.ExclusiveMinimum); err != nil { - resMinimum.Merge(errorHelp.sErr(err, n.Options.recycleResult)) + if err := Minimum(n.Path.dotted(), n.In, data, *n.Minimum, n.ExclusiveMinimum); err != nil { + resMinimum.Merge(errorHelp.sErrAt(n.Path, err, n.Options.recycleResult)) } } } @@ -942,12 +937,16 @@ func (n *numberValidator) Validate(val any) *Result { return res } +func (n *numberValidator) setPath(path pathSegments) { + n.Path = path +} + func (n *numberValidator) redeem() { pools.poolOfNumberValidators.RedeemValidator(n) } type stringValidator struct { - Path string + Path pathSegments In string Default any Required bool @@ -959,7 +958,7 @@ type stringValidator struct { } func newStringValidator( - path, in string, + path pathSegments, in string, def any, required, allowEmpty bool, maxLength, minLength *int64, pattern string, opts *SchemaValidatorOptions, ) *stringValidator { @@ -987,10 +986,6 @@ func newStringValidator( return s } -func (s *stringValidator) SetPath(path string) { - s.Path = path -} - func (s *stringValidator) Applies(source any, kind reflect.Kind) bool { switch source.(type) { case *spec.Parameter, *spec.Schema, *spec.Items, *spec.Header: @@ -1009,35 +1004,39 @@ func (s *stringValidator) Validate(val any) *Result { data, ok := val.(string) if !ok { - return errorHelp.sErr(errors.InvalidType(s.Path, s.In, stringType, val), s.Options.recycleResult) + return errorHelp.sErrAt(s.Path, errors.InvalidType(s.Path.dotted(), s.In, stringType, val), s.Options.recycleResult) } if s.Required && !s.AllowEmptyValue && (s.Default == nil || s.Default == "") { - if err := RequiredString(s.Path, s.In, data); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := RequiredString(s.Path.dotted(), s.In, data); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } if s.MaxLength != nil { - if err := MaxLength(s.Path, s.In, data, *s.MaxLength); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := MaxLength(s.Path.dotted(), s.In, data, *s.MaxLength); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } if s.MinLength != nil { - if err := MinLength(s.Path, s.In, data, *s.MinLength); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := MinLength(s.Path.dotted(), s.In, data, *s.MinLength); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } if s.Pattern != "" { - if err := Pattern(s.Path, s.In, data, s.Pattern); err != nil { - return errorHelp.sErr(err, s.Options.recycleResult) + if err := Pattern(s.Path.dotted(), s.In, data, s.Pattern); err != nil { + return errorHelp.sErrAt(s.Path, err, s.Options.recycleResult) } } return nil } +func (s *stringValidator) setPath(path pathSegments) { + s.Path = path +} + func (s *stringValidator) redeem() { pools.poolOfStringValidators.RedeemValidator(s) } diff --git a/validator_test.go b/validator_test.go index ff2738a..1d85741 100644 --- a/validator_test.go +++ b/validator_test.go @@ -66,7 +66,7 @@ func TestNumberValidator_EdgeCases(t *testing.T) { maximum := float64(math.MaxInt32 + 1) v := newNumberValidator( - "path", + newPathSegments("path"), "in", nil, nil, @@ -129,7 +129,7 @@ func TestStringValidator_EdgeCases(t *testing.T) { // Apply v := newStringValidator( - "", "", nil, false, false, nil, nil, "", nil, + nil, "", nil, false, false, nil, nil, "", nil, ) // stringValidator applies to: Parameter,Schema,Items,Header @@ -160,7 +160,7 @@ func TestBasicCommonValidator_EdgeCases(t *testing.T) { // Apply v := newBasicCommonValidator( - "", "", + nil, "", nil, []any{"a", nil, 3}, nil, ) @@ -190,7 +190,7 @@ func TestBasicCommonValidator_EdgeCases(t *testing.T) { t.Run("shoud validate empty Enum", func(t *testing.T) { ev := newBasicCommonValidator( - "", "", + nil, "", nil, nil, nil, ) res := ev.Validate("a") @@ -213,7 +213,7 @@ func testCommonApply(t *testing.T, v *basicCommonValidator, sources []any) { func TestBasicSliceValidator_EdgeCases(t *testing.T) { t.Run("should Apply", func(t *testing.T) { v := newBasicSliceValidator( - "", "", + nil, "", nil, nil, nil, false, nil, nil, strfmt.Default, nil, ) @@ -233,7 +233,7 @@ func TestBasicSliceValidator_EdgeCases(t *testing.T) { t.Run("with recycling", func(t *testing.T) { v := newBasicSliceValidator( - "", "", + nil, "", nil, nil, nil, false, nil, nil, strfmt.Default, &SchemaValidatorOptions{recycleValidators: true}, )