pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
466 stars 134 forks source link

$id properties not working. #96

Closed Markus-St closed 4 years ago

Markus-St commented 4 years ago

Hello, I wanted to use this validator to validate my json against a schema which works on multiple online validator, but not with yours. I investigated the problem and it all boils down to the validator not correctly handling the $id properties. I reduced my problem to a very small schema.

So this schema works:

{
    "$id": "http://xxx.local/schemas/mySchema.json",
    "$schema": "http://json-schema.org/draft-07/schema#",
    "definitions": {
        "topDef": {
            "type": "object",
            "properties": {
                "value": {
                    "type": "integer",
                    "maximum": 100
                }
            }
        }
    },

    "type": "object",
    "properties": {
        "top": {
            "$ref": "#/definitions/topDef"
        }
    }
}

But as soon as i don't want to reference topDefby its path, but by an $id property:

{
    "$id": "http://xxx.local/schemas/mySchema.json",
    "$schema": "http://json-schema.org/draft-07/schema#",
    "definitions": {
        "topDef": {
            "$id": "#topDef_ref",
            "type": "object",
            "properties": {
                "value": {
                    "type": "integer",
                    "maximum": 100
                }
            }
        }
    },

    "type": "object",
    "properties": {
        "top": {
            "$ref": "#topDef_ref"
        }
    }
}

I get the error set_root_schema failed: schema with http://xxx.local/schemas/mySchema.json # topDef_ref already inserted.

I am very sure, that I use the $id property correctly, because I do it just as described here: Using id with ref and at least 4 online validators having no problem with it. By the way, the schema at this example does also not work with your validator.

pboettch commented 4 years ago

Thanks, nice find. Interesting problem.

What do you mean by "By the way, the schema at this example does also not work with your validator." Is this a separate issue?

pboettch commented 4 years ago

I tested your working schema with the following instance:

{"top": {"value": 101}}

It fails with 'maximum exceeded'.

pboettch commented 4 years ago

I understood the problem. It is a known issue which I didn't know how to solve when I added the support for plain name references (Your #topDef_ref is a plain name (no beginning '/' in JSON-pointer))

pboettch commented 4 years ago

How should it work for sub-schemas identified with a plain name identifier?

In your case, would you expect to be able to reference a sub-schema to value?

"$ref": "#topDef_ref/properties/value"
Markus-St commented 4 years ago

What do you mean by "By the way, the schema at this example does also not work with your validator." Is this a separate issue?

No, I just meant the example schema, which is in the example Using id with ref is also not working. I mean this schema:

{
  "$schema": "http://json-schema.org/draft-07/schema#",

  "definitions": {
    "address": {
      "$id": "#address",
      "type": "object",
      "properties": {
        "street_address": { "type": "string" },
        "city":           { "type": "string" },
        "state":          { "type": "string" }
      },
      "required": ["street_address", "city", "state"]
    }
  },

  "type": "object",

  "properties": {
    "billing_address": { "$ref": "#address" },
    "shipping_address": { "$ref": "#address" }
  }
}

I just wanted to point out, that not only my schema has a problem, since it also happens with a schema, written by the creators of json-schema.

I understood the problem. It is a known issue which I didn't know how to solve when I added the support for plain name references (Your #topDef_ref is a plain name (no beginning '/' in JSON-pointer))

Ok, so it's a known issue. Do you somewhere have a list of known issues? Because in your README it states "Version 2 supports JSON schema draft 7". And apparetnly it doesn't. At least not completely. Because these kind of references are draft 7 compliant. They even work on other validators with draft 5 and upwards. I have to say, trying a library and the first thing you try doesn't work because of an error in the library just doesn't make me very confident in using this library in future.

How should it work for sub-schemas identified with a plain name identifier?

It should work like the draft defines it. I actually don't know if a plain name identifier + path is allowed. I just know a normal one is.

pboettch commented 4 years ago

Could you check the branch fix-96? I just pushed a fix.

With known issue I meant that I was asking myself the question at the time of developing it but I don't remember my interpretation.

How should it work for sub-schemas identified with a plain name identifier?

It should work like the draft defines it. I actually don't know if a plain name identifier + path is allowed. I just know a normal one is.

Oh, OK I thought you were familiar with plain name fragments, as you were using them. I have to admit finding this information in the different RFC referenced by the draft is not easy.

pboettch commented 4 years ago

I have to say, trying a library and the first thing you try doesn't work because of an error in the library just doesn't make me very confident in using this library in future.

At least it was not a segmentation fault. ;-)

This library is open-source and was written by volunteering contributors. Your bug report is an important contribution as its fix improved the code for everyone. Thanks for having taken the time to create it.

Markus-St commented 4 years ago

Oh, OK I thought you were familiar with plain name fragments, as you were using them. I have to admit finding this information in the different RFC referenced by the draft is not easy.

I am still learning, I am no expert. I just started using json schema. I know, what i know from the tutorial at Understanding json schema and trying them with online validators.

This library is open-source and was written by volunteering contributors. Your bug report is an important contribution as its fix improved the code for everyone. Thanks for having taken the time to create it.

I understand and appreciate it. That's also why I'm giving feedback. I'm just saying, that a validator is usually a thing you want to trust, since it's validating your stuff. And it's hard to trust something when you encounter errors as soon as you start using it.

Could you check the branch fix-96? I just pushed a fix.

Thank you for the quick action. And for the problem stated above it is a fix. But unfortunately i came across a different problem, that could even be caused by your fix as far as i understand it, since you seem to remove the subschemas with id from the main schema. I think the subschemas are then not checked.

This is a correct schema according to other validators and is also "accepted" by your validator, although I'm not sure it is used correctly:

{
    "$id": "http://xxx.local/schemas/mySchema.json",
    "$schema": "http://json-schema.org/draft-07/schema#",
    "definitions": {
        "childDef": {
            "$id": "#childDef_ref",
            "type": "object",
            "properties": {
                "value": {
                    "type": "integer",
                    "maximum": 100
                }
            }
        },
        "topDef": {
            "$id": "#topDef_ref",
            "type": "object",
            "properties": {
                "child": {
                    "$ref": "#childDef_ref"
                }
            }
        }
    },

    "type": "object",
    "properties": {
        "top": {
            "$ref": "#topDef_ref"
        }
    }
}

Then I introduce a typo in the reference (it's now only "childDef_re"):

{
    "$id": "http://xxx.local/schemas/mySchema.json",
    "$schema": "http://json-schema.org/draft-07/schema#",
    "definitions": {
        "childDef": {
            "$id": "#childDef_ref",
            "type": "object",
            "properties": {
                "value": {
                    "type": "integer",
                    "maximum": 100
                }
            }
        },
        "topDef": {
            "$id": "#topDef_ref",
            "type": "object",
            "properties": {
                "child": {
                    "$ref": "#childDef_re"
                }
            }
        }
    },

    "type": "object",
    "properties": {
        "top": {
            "$ref": "#topDef_ref"
        }
    }
}

This schema is also accepted by your validator, although #childDef_re is not defined. I even get a successfull validation with it. https://www.jsonschemavalidator.net/ gives me Could not resolve schema reference '#childDef_re'. Path 'definitions.topDef.properties.child', line 19, position 26. which is correct.

pboettch commented 4 years ago

Interesting. I will check it out.

pboettch commented 4 years ago

You'll get the error when validating:

71: ERROR: '"/top"' - '{"value":101}': unresolved schema-reference http://xxx.local/schemas/mySchema.json # childDef_re
pboettch commented 4 years ago

Could you make another bug-report, please? This is independent of this issue and its fix.

pboettch commented 4 years ago

I made one: https://github.com/pboettch/json-schema-validator/issues/97

Markus-St commented 4 years ago

Thank you. My concern was exactly what you mentioned in the new issue. Not having an error on call of set_root_schema.

pboettch commented 4 years ago

Can this issue be closed?

Markus-St commented 4 years ago

Yes, this was fixed with your fix-96 branch.

Markus-St commented 4 years ago

You did not yet merge the fix-96 branch into master. Is there a specific reason for that?

pboettch commented 4 years ago

Nope.