networknt / json-schema-validator

A fast Java JSON schema validator that supports draft V4, V6, V7, V2019-09 and V2020-12
Apache License 2.0
807 stars 320 forks source link

bug: null test missing for missing discriminator property. #985

Closed GregoryEssertel closed 4 months ago

GregoryEssertel commented 4 months ago

Where: https://github.com/networknt/json-schema-validator/blob/eea61d6b630ee1277371bf7ffc0e171ac4cbff54/src/main/java/com/networknt/schema/OneOfValidator.java#L113

To trigger: schema:

    {
          "type": "object",
          "discriminator": { "propertyName": "name" },
          "oneOf": [
            {
              "$ref": "#/defs/Foo"
            },
            {
              "$ref": "#/defs/Bar"
            }
          ],
          "defs": {
            "Foo": {
              "type": "object",
              "properties": {
                "name": {
                  "const": "Foo"
                }
              },
              "required": [ "name" ],
              "additionalProperties": false
            },
            "Bar": {
              "type": "object",
              "properties": {
                "name": {
                  "const": "Bar"
                }
              },
              "required": [ "name" ],
              "additionalProperties": false
            }
          }
        }

value:

{}

I want to try to make a PR, what should be the expected behavior in this case. Ideas: 1 - returning an error similar to the missing required property, but specifying that it is the discriminator. 2 - ignoring discriminator and return all errors

justin-tay commented 4 months ago

Thanks for raising the issue.

I think the simple fix is to have it fall back on

https://github.com/networknt/json-schema-validator/blob/eea61d6b630ee1277371bf7ffc0e171ac4cbff54/src/main/java/com/networknt/schema/OneOfValidator.java#L141-L149

I don't really have an objection to a specific error message for the missing property, since the spec states If the discriminator value does not match an implicit or explicit mapping, no schema can be determined and validation SHOULD fail.

But if you want to do so then ideally the PR should have a proper fix, meaning that

justin-tay commented 4 months ago

After looking at this closer it looks like due to the following

To make the behavior consistent with the rest of the usages the logic needs to treat the discriminator as matching if the property is not present in the data and rely on the required validator to catch the missing discriminator.

I shall probably create the PR to resolve this.