Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Validation error on nullable parameters with a format #34

Closed
Dimitris-Christodoulou opened this issue Jan 13, 2020 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@Dimitris-Christodoulou
Copy link

Dimitris-Christodoulou commented Jan 13, 2020

Describe the bug
A validation error is issued by SchemaValidator when a nullable number property with a format (e.g. double) is null.

To Reproduce

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.openapi4j.core.model.v3.OAI3;
import org.openapi4j.core.validation.ValidationResults;
import org.openapi4j.parser.OpenApi3Parser;
import org.openapi4j.parser.model.v3.OpenApi3;
import org.openapi4j.parser.model.v3.Schema;

import java.nio.file.Files;
import java.nio.file.Path;
import org.openapi4j.schema.validator.ValidationContext;
import org.openapi4j.schema.validator.v3.SchemaValidator;

public class NullableFormatBug {

    public static void main(String[] args) throws Exception {
    String openApi =
              "paths:\n"
            + "  /example:\n"
            + "    post:\n"
            + "      requestBody:\n"
            + "        content:\n"
            + "          'application/json':\n"
            + "            schema:\n"
            + "              $ref: '#/components/schemas/NullableFormat'\n"
            + "      responses:\n"
            + "        '200':\n"
            + "          description: Updated\n"
            + "components:\n"
            + "  schemas:\n"
            + "    NullableFormat:\n"
            + "      type: array\n"
            + "      items:\n"
            + "        type: object\n"
            + "        additionalProperties: false\n"
            + "        properties:\n"
            + "          foo:\n"
            + "            type: number\n"
            + "            format: double\n"
            + "            nullable: true\n";

      Path nullableFormatBug = Files.createTempFile("NullableFormatBug", "");
      Files.write(nullableFormatBug, openApi.getBytes());

      OpenApi3 api = new OpenApi3Parser().parse(nullableFormatBug.toFile(), false);

      Schema schema = api.getPath("/example").getOperation("post")
          .getRequestBody().getContentMediaType("application/json").getSchema();

      JsonNode jsonNode = TreeUtil.toJsonNode(schema);

      ValidationContext<OAI3> validationContext = new ValidationContext<>(api.getContext());
      SchemaValidator schemaValidator = new SchemaValidator(validationContext, "", jsonNode);

      String testData1 =
                "["
                 + "{\n"
                 + "  \"foo\": null\n"
                 + "}"
              + "]";

      ObjectMapper mapper = new ObjectMapper();

      ValidationResults results = new ValidationResults();
      schemaValidator.validate(mapper.readTree(testData1), results);
      if (!results.isValid()) {
        System.out.println(results);
      }
    }
}

The following error is displayed when the above code is run:

Validation error(s) :
.#/components/schemas/NullableFormat.items.foo : Value 'null' does not match format 'double'.

Expected behavior
Expected no validation errors from the above code.

@Dimitris-Christodoulou Dimitris-Christodoulou added the bug Something isn't working label Jan 13, 2020
@llfbandit
Copy link
Contributor

Welcome and thank you for the report.
The issue is about the format validator which did not detect the null value.

Can you confirm the fix in the latest snapshot? It should be already available.

@Dimitris-Christodoulou
Copy link
Author

Dimitris-Christodoulou commented Jan 13, 2020

@llfbandit Thank you for the quick response.
Where is 0.6-SNAPSHOT being published? I can't seem to find it in the maven repo.

Also, is there an estimate on when 0.6 will be released?

@llfbandit
Copy link
Contributor

You have to add https://oss.sonatype.org/content/repositories/snapshots before accessing snapshot versions.

I'm waiting for #28 confirmation before bumping to 0.7 so a couple of days I would say.

@Dimitris-Christodoulou
Copy link
Author

You have to add https://oss.sonatype.org/content/repositories/snapshots before accessing snapshot versions.

I'm waiting for #28 confirmation before bumping to 0.7 so a couple of days I would say.

The above test throws a NullPointerException with 0.6-SNAPSHOT. It seems to be coming from ReferenceValidator. More specifically, schemaParentNode.get(ABS_REF_FIELD) is null. The schema parent node contains a normal $ref instead of an absolute ref. Aren't both valid in that context?

@llfbandit
Copy link
Contributor

llfbandit commented Jan 13, 2020

This is due to the usage of toNode which serialize the content with constraints to reflect the definition of OAI. The field ABS_REF_FIELD is not exported in this case. This why you get the RuntimeException.

If you really use the toolset like with the reproducer you'll need to use TreeUtil.toJsonNode to keep the info.

To let me know, what kind of usage do you have? For this kind of validation process, the operation validator is dedicated to make the glue code between parser and schema validation modules.

@Dimitris-Christodoulou
Copy link
Author

I updated the code to JsonNode jsonNode = TreeUtil.toJsonNode(schema); but I am still getting the same NullPointerException. I updated the test case in the original post accordingly so it can be reproduced.

@Dimitris-Christodoulou
Copy link
Author

To let me know, what kind of usage do you have? For this kind of validation process, the operation validator is dedicated to make the glue code between parser and schema validation modules.

We are using the schema validator to validate input and output payloads against the schemas defined in the OpenAPI definition of the REST API.

@llfbandit
Copy link
Contributor

0.6 introduced this bug. Sorry for that.
This invalidates 0.6 as a release for now. You can follow up this subsequent fix to the dedicated issue #28.

@Dimitris-Christodoulou
Copy link
Author

0.6 introduced this bug. Sorry for that.
This invalidates 0.6 as a release for now. You can follow up this subsequent fix to the dedicated issue #28.

Ok thank you I will keep an eye on #28

@llfbandit
Copy link
Contributor

Can you validate this issue ? We're pretty close to conclude on the other side. Thank you.

@Dimitris-Christodoulou
Copy link
Author

Validated. Seems to be fixed. Thank you very much for the quick turnaround.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants