minLength/maxLength string tests assume the property is required

Issue #322 resolved
Former user created an issue

I believe JSON without an optional string that has length requirements defined in the schema should pass validation.

It appears the tests for minLength and maxLength of a string assume that the string is required:

    public static Parameter stringParam(final Integer minLength, final Integer maxLength) {
        final StringSchema schema = new StringSchema();
        schema.setMinLength(minLength);
        schema.setMaxLength(maxLength);
        return param(schema, true);
    }

I.e. I believe a new [method overload | optional parameter] (I don't know Java) should allow a third parameter (final boolean isRequired) and that should be used when creating the param:

    public static Parameter stringParam(final Integer minLength, final Integer maxLength, final boolean isRequired) {
        final StringSchema schema = new StringSchema();
        schema.setMinLength(minLength);
        schema.setMaxLength(maxLength);
        return param(schema, isRequired);
    }

Comments (16)

  1. James Navin

    Can I just confirm if this is a problem you are encountering? Or is it a concern that the test coverage is missing for this scenario?

    If it is a problem you are encountering it would be useful to see an example spec + request/response that triggers the behavior.

    Cheers.

  2. Joanne Wright

    This is an error in the validator because a minLength should not imply that the property is required; it indicates the minimum length of the value, should the property be included in the message payload.

    When tested In another online tool for JSON schema validation this is working as expected.

  3. James Navin

    This is an error in the validator because a minLength should not imply that the property is required; it indicates the minimum length of the value, should the property be included in the message payload.

    I agree. The issue I have is that the tests I added to uncover the problem aren’t failing (https://bitbucket.org/atlassian/swagger-request-validator/branch/322-length-validation-assumes-required#diff) so I must not be testing the problem you are reporting.

    If you could include an example spec and request to recreate the problem it would be very helpful.

  4. Joanne Wright

    My recollection is this error is evident in a patch message, line 6242 .

    "/registrations/{registrationId}": {

    "patch": {

    "parameters": [

    Shipper[xxx] fields are non mandatory the spec suggests only mpxn and data fields are required, and not those within the data block

    "required": [

    "mpxn",

    "data"

    ],

    Data block is defined as :

    "data": {

    "type": "object",

    "additionalProperties": false,

    "properties": {

    "domesticPremisesInd": {

    "type": "boolean",

    "description": "Indicates whether the premises are Domestic or not"

    },

    "shipperMpid": {

    "type": "string",

    "minLength": 1,

    "maxLength": 4,

    "description": "Market Participant Id",

    "example": "ABCD"

    },

    "shipperRole": {

    "type": "string",

    "description": "The Role of the Market Participant",

    "example": "M",

    "minLength": 1

    },

    "shipperFromDate": {

    "type": "string",

    "format": "date-time",

    "description": "ISO8061 format \"date-time\"",

    "example": "2019-08-07T22:57:49.000Z"

    }

    }

    Please let me know if you need more information.

  5. James Navin

    Thanks for the additional information. Unfortunately Im still struggling to reproduce the problem.

    I added additional tests based on your example (objects with non-required properties that have a minLength) and they are passing fine.

    https://bitbucket.org/atlassian/swagger-request-validator/branch/322-length-validation-assumes-required#Lswagger-request-validator-core/src/test/java/com/atlassian/oai/validator/OpenAPIV3RequestValidationTest.javaT782

    Can I confirm you are using the most recent version of the validator?

  6. Joanne Wright

    We are using the latest version of the validator - 2.12.1

    <dependency>

    <groupId>com.atlassian.oai</groupId>

    <artifactId>swagger-request-validator-core</artifactId>

    <version>2.12.1</version>

    </dependency>

    JSON example.

    ZHV|12|S2000001|X|DXCZ|Z|CSSP|20210101000000|1.0|PATCH||TR01|

    [

    {

    "registrationId": "f31e97af-1a94-4ef4-b99b-b73f49e18c1b",

    "mpxn": "2199990866750",

    "data": {

    "shipperMpid": "",

    "shipperRole": "",

    "shipperFromDate": "",

    "domesticPremisesInd": true

    }

    },

    {

    "registrationId": "f31e97af-1a94-4ef4-b99b-b73f49e18c1b",

    "mpxn": "2199990866751",

    "data": {

    "shipperMpid": "",

    "shipperRole": "",

    "shipperFromDate": "",

    "domesticPremisesInd": false

    }

    }

    ]

  7. Joanne Wright

    Please can you try the following

    try the below payload against URL /registration/{registrationId} as per attached open API spec (supplier_v1.0.8.json)

    {

    "mpxn": "2199990866750",

    "data": {

    "shipperMpid": "",

    "shipperRole": "",

    "shipperFromDate": "",

    "domesticPremisesInd": true

    }

    }

  8. James Navin

    Ah - I see the problem. Empty strings are different to missing properties according to the OpenAPI spec. In this case the validator is behaving as expected - you have provided a string of length 0 to a property with a minLength: 1 constraint. The required constraint is about the existence of the property in the object, not the value it holds.

    Options here would be to add the nullable constraint and supply null rather than empty string, or change your JSON serialization to ignore empty values (e.g. in Jackson this would be done via https://fasterxml.github.io/jackson-annotations/javadoc/2.7/com/fasterxml/jackson/annotation/JsonInclude.Include.html#NON_EMPTY)

  9. Joanne Wright

    Thank you for your help with this issue. We have now implemented a fix and happy to close this issue.

  10. Log in to comment