- changed status to open
Optional header provided as empty passes validation despite not conforming to schema
Hi
I am validating as per a spec which contains the following parameter definition:
- name: OriginatingSystem
in: header
description: Originating system
schema:
pattern: ^[A-Z0-9-_]{1,10}$
type: string
This correctly allows requests which do not provide the header and rejects requests which provide a non-empty value that does conform to the schema (pattern in my case).
However, it allows requests which provide an empty value for the header despite such a value not conforming to the provided pattern.
I would expect any parameter to be rejected if its value does not conform to its schema, including empty values, with the sole exception of query parameters which have allowEmptyValues explicitly set.
Modifying ParameterValidator.java:64 thus:
if (value == null /* || value.trim().isEmpty() */ ) {
return ValidationReport.empty();
}
… results in 2 test failures:
- NumberParameterValidationTest#validate_withEmptyValue_shouldPass_whenNotRequired
- IntegerParameterValidationTest#validate_withEmptyValue_shouldPass_whenNotRequired
… both of which seem justifiable to me, given it would be impossible to convert the value to the appropriate number.
Additional tests could then be added to cover various StringSchemas where an empty value does and does not conform, eg:
Scenario | PASS / FAIL for empty value |
---|---|
No schema definition beyond specifying type | PASS |
pattern: ^.*$ | PASS |
pattern: ^.+$ | FAIL |
minLength: 0 | PASS |
minLength: 1 | FAIL |
enum: [ “one”, “two” ] | FAIL |
I am happy to submit a pull request but would like to hear your thoughts first given this would be a breaking change.
Comments (6)
-
reporter -
Hi Ivor,
Thanks for raising such a detailed issue - much appreciated.
This issue seems related to #61 but you are correct - for non-query params we should be failing on empty values if that does not match the schema.
I think your suggestion makes sense - happy to review a PR and release the change as a minor version bump.
Cheers,
James
-
reporter Thanks James
I have now submitted pull request #267 - please let me know if you have any feedback or want anything changed.
I have signed the CLA.
Cheers
Ivor
-
- changed status to resolved
Available in v2.21.0
-
reporter Hi @James Navin
How long does it normally take for a release to appear on Maven Central - 2.21.0 is still not available but 2.20.0 looks like it appeared the same day?
Thanks
Ivor
-
Not sure if it warrants a separate issue or not, but I noticed the error message is slightly misleading in case an empty value is provided.
A simplified example - an endpoint has an optional
manufacturerId
parameter that must be a UUID:paths: /things/index: get: summary: Search for things. operationId: searchThingsUsingGet parameters: - name: voltage in: query required: true schema: type: integer format: int32 - name: manufacturerId in: query required: false schema: type: string format: uuid
If
GET https://{domain}/things/index?voltage=240&manufacturerId=
request is sent, the response is HTTP 400 (expected), but the message is:{ "messages" : [ { "key" : "validation.request.parameter.missing", "level" : "ERROR", "message" : "Parameter 'manufacturerId' is required but is missing.", "context" : { "requestPath" : "/things/index", "parameter" : { "name" : "manufacturerId", "in" : "query", "required" : false, "style" : "FORM", "explode" : true, "schema" : { "type" : "string", "format" : "uuid", "exampleSetFlag" : false } }, "location" : "REQUEST", "requestMethod" : "GET" } } ] }
This is slightly misleading, as the parameter is actually present, just that its value is not a UUID.
FYI @Ivor Potter @James Navin
- Log in to comment