Optional header provided as empty passes validation despite not conforming to schema

[Ivor Potter]

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:

1. NumberParameterValidationTest#validate_withEmptyValue_shouldPass_whenNotRequired
2. 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.

‌

[James Navin]

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

[Ivor Potter]

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

[James Navin]

Available in v2.21.0

[Ivor Potter]

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

[Gediminas Rimša]

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