Inconsistent Parameter Validation Behavior in libopenapi-validator v0.4.0

[wsqyouth]

Description:

I've encountered an inconsistency in parameter validation when using libopenapi-validator version v0.4.0. Specifically, there are differences in how validation errors are reported for different types of illegal parameter scenarios:

• Enum Value Mismatch: When providing parameter values that do not match the expected enum values, the validation error response is generated, but the ValidationError.SchemaValidationErrors field is empty.

• Exceeding Maximum Range: Conversely, when a parameter exceeds its maximum allowable value, the validation error response includes details within the ValidationError.SchemaValidationErrors field.

Upon reviewing the source code, I noticed that within the validateSimpleParam method, there is preliminary validation only for illegal values, whereas other validation types utilize the ValidateSingleParameterSchema function. This discrepancy results in the ValidationError.SchemaValidationErrors field not being populated consistently. github.com/pb33f/[email protected]/parameters/query_parameters.go:232

Suggestion:

I believe that this inconsistent behavior can make it challenging for users to handle validation errors uniformly. It would be beneficial to use ValidateSingleParameterSchema for all types of parameter validations, ensuring that the response behavior is consistent across different scenarios.

Your feedback and thoughts on this matter would be greatly appreciated.

[its-hammer-time]

https://github.com/pb33f/libopenapi-validator/issues/130

Likely related to my issue above. Noticed that the max limit of a query param was being ignored.

[daveshanley]

@emilien-puget this is from one of your contributions, any ideas before I dig in?

[emilien-puget]

What about modifying the errors.incorrectqueryparamenum to populate that field? If we remove the code about the enum, we will stop validating enum

[daveshanley]

I think this is a larger problem, rather than your code. These methods are custom business logic validating properties. The SchemaValidationErrors come from abstracting schema errors that come from the underlying jsonschema library.

The custom business logic results contain no SchemaValidationErrors because they have not been run through the jsonschema lb, we have manually checked things - so there isn't really anything to return, except a duplicate of the data inValidationError

So, this really becomes a design choice, sometimes the SchemaValidationErrors is populated, sometimes it isn't, depending on which part of the validator ran.

should we document this? or make it consistent (which means adding a SchemaValidationError to the ValidationError for all custom logic.)

[emilien-puget]

it's technically not a schema validation error because there are no schemas for those parameters, maybe putting all errors in a ValidationError array is the way to go ?

[its-hammer-time]

https://github.com/pb33f/libopenapi-validator/issues/168#issuecomment-3293943597

This ties into the issue I'm experiencing over here as well as the questions I raised.

---

it's technically not a schema validation error because there are no schemas for those parameters, maybe putting all errors in a ValidationError array is the way to go ?

I don't think this is necessarily true. To my knowledge there are 2 types of schema issues:

1. Doesn't match OpenAPI specific schema
2. Doesn't match JSONSchema schema (specifically for 3.1.0, this is JSONSchema Draft 2020-12)

In my opinion, for either case, these should be reported as the same type of error as it's breaching the schema defined in OpenAPI. For example, things like maximum, enum, etc. are all handled by JSONSchema, but there are a couple of parameter errors which are unique to the OpenAPI specification like required (for parameters), allowEmptyValues, style, etc. It's a better user experience for all of these to be located in the same "spot" and have similar information like:

1. The path + name of the field that failed the schema validation
2. The reason why it failed
3. The location where it failed in the OpenAPI spec

Then there's another type of error which is we couldn't even perform validation due to being unable to reach the schema. For example, malformed request body, invalid path, invalid mimetype, etc. These should be reported as a separate type of error because it's not a "schema" error.

---

Now how all this gets raised is more of a design decision. IMO, I think it makes sense to have something along the lines of:

1. ValidationError - Conveys errors with schema "finding" (e.g. invalid mimetype) as well as a wrapper for "schema error"
   • SchemaValidationError - Conveys a unique error that came from schema validation (e.g. enum, required, etc.)
     • OriginalError - JSONSchema | OpenAPI - The underlying error we caught before it was translated to a SchemaValidationError. This can either be something custom that's part of this library or from the JSONSchema library.