Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

null is not valid against nullable array with typeLoose false #194

Closed
chuwy opened this issue Oct 3, 2019 · 9 comments
Closed

null is not valid against nullable array with typeLoose false #194

chuwy opened this issue Oct 3, 2019 · 9 comments

Comments

@chuwy
Copy link
Contributor

chuwy commented Oct 3, 2019

I have this test case:

    {
        "description": "multiple types can be specified in an array with null",
        "schema": {"type": ["array", "null"], "items": {"type": "string"}},
        "tests": [
            {
                "description": "a null is valid",
                "isTypeLoose": true,
                "data": null,
                "valid": true
            }
        ]
    }

I assume this should pass validation, but it fail with following error:

[$[0]: null found, string expected]

It passes however with "isTypeLoose": false or no items. I thought this should have been fixed in #183, but I observe this behavior in 1.0.20 as well. It seems this behavior was introduced in 1.0.8 (presumably here)

I have several questions about it:

  1. isTypeLoose is related to OpenAPI specification and goes against official v4 specification
  2. isTypeLoose should make validation less strict, e.g. make "3" (string) pass against integer type
  3. In this case however, seems isTypeLoose does something different and it should be considered a bug
  4. Regardless of whether this is a bug or not, do you think isTypeLoose should default to false? At least I'd assume a JSON Schema validator library should be compliant with official specs
@chuwy
Copy link
Contributor Author

chuwy commented Oct 4, 2019

I fixed this by changing:

if (!node.isArray() && !config.isTypeLoose()) {

to

if (!node.isArray()) {

in ItemsValidator.java. But now several other tests fail, e.g.

    {
        "description": "an typeLoose array of schemas for items",
        "schema": {
            "items": [
                {"type": "integer"}
            ]
        },
        "tests": [
            {
                "description": "wrong types",
                "data": "foo",
                "isTypeLoose": true,
                "valid": false
            }
        ]
    }

Funny thing is that IMO this instance is valid against above schema, because items simply irrelevant to string foo. Probably I just still don't understand a meaning of "looseType" config, it looks very confusing to me.

@stevehu
Copy link
Contributor

stevehu commented Oct 4, 2019

@chuwy TypeLoose is to handle the OpenAPI specification as it is different than the JSON scheme validation at the moment. The specific flag is to allow the library to treat query parameters, headers and path parameters without worry about the types. In the HTTP context, everything is a string but the schema might indicate that pageNum is an integer. So the library will tolerate the input "20" as an integer although it is a string. That is what TypeLoose works. If you are using pure schema validation, set the TypeLoose to false.

@chuwy
Copy link
Contributor Author

chuwy commented Oct 4, 2019

Thanks @stevehu! So this confirms my understanding and I will change its use in our lib. Then it also means that typeLoose is relevant only to strings and probably my fix is actually correct.

@stevehu
Copy link
Contributor

stevehu commented Oct 4, 2019

Yes. The change looks good. Just to confirm that we don't need to do anything except to set the TypeLosse to false to handle this use case. If yes, can we close this issue? Or we still need to do something?

@chuwy
Copy link
Contributor Author

chuwy commented Oct 4, 2019

Sorry, by "my fix" I meant one mentioned here: #194 (comment). I'm just not sure what we need to do with other tests that fail because of that fix, IMO those test cases are invalid (also regardless of typeLoose).

I still think this is a bug and the fact that it works with typeLoose: false is just a lucky coincedence.

@stevehu
Copy link
Contributor

stevehu commented Oct 4, 2019

Let's keep this open as a bug so that we can investigate further. Thanks.

@koxauvin
Copy link

I ended up reverting back to 1.0.7 to get my case or work. Confirming > "It seems this behavior was introduced in 1.0.8" from above.

@MarinaRappoport
Copy link

isTypeLoose also converts string/integers/objects to array, that seems not to be correct according to documentation. I opened another issue for that:
#415

@stevehu
Copy link
Contributor

stevehu commented May 30, 2024

This shouldn't be an issue for the latest version 1.4.x

@stevehu stevehu closed this as completed May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants