pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
464 stars 133 forks source link

bug with items array #311

Closed andrejlevkovitch closed 3 months ago

andrejlevkovitch commented 3 months ago

For tuple validation json schema allows usage of items keyword as an array, for example like here:

{
  "type": "array",
  "items": [
    {
      "$comment": "some comment",
      "type": "number"
    },
    {
      "type": "string"
    }
  ]
}

Also, we can use it in $defs too, like here:

{
  "type": "object",
  "properties": {
    "element": {
      "$ref": "#/$defs/element"
    }
  },
  "$defs": {
    "element": {
      "type": "array",
      "items": [
        {
          "type": "number"
        },
        {
          "type": "string"
        }
      ]
    }
  }
}

And that works perfectly with the library. But if I put $comment inside of one of the items, then I have exception:

cannot use operator[] with a string argument with array

So, that schema will throw the exception when you try to set it to validator

{
  "type": "object",
  "properties": {
    "element": {
      "$ref": "#/$defs/element"
    }
  },
  "$defs": {
    "element": {
      "type": "array",
      "items": [
        {
          "$comment": "some comment",
          "type": "number"
        },
        {
          "type": "string"
        }
      ]
    }
  }
}

Looks like only $comment keyword doesn't work properly in that context

andrejlevkovitch commented 3 months ago

I traced the issue to that line

andrejlevkovitch commented 3 months ago

seems that there are a problem with indexing - it tries to use an array as object and uses array index as object key

andrejlevkovitch commented 3 months ago

probably it is related to that comment:

https://github.com/pboettch/json-schema-validator/blob/08d8a52a8a1353b6bc953b63f7c6763e74e6047b/src/json-validator.cpp#L213-L216

but I can not understand what that means. Is there a reason why it doesn't use json pointer directly?

andrejlevkovitch commented 3 months ago

If I understood everything correctly, the that pr should fix the issue

pboettch commented 3 months ago

So, it seems there is a bug with the handling of unknown keywords. Your schema shows that.

But why we have the unknown keyword handling getting involved here? Because $defs is something from the 2020-draft of json-schema not directly supported by this library. For the draft7-version it should be definitions. This is something to be taking into consideration in the analysis of this bug.

andrejlevkovitch commented 3 months ago

Yeah, that is fair. I'm not sure what I should do with that issue then... I mean should I close it or keep it?

pboettch commented 3 months ago

Well, it is still a bug with the unknown keyword handling, I'd need a better explanation as to why this happens before merging your PR.

pboettch commented 3 months ago

The errors wasn't the $comment field but a nested schema in an array of schemas in an unknown-keyword.

Your fix was correct and elegant: using a json-pointer to resolve the direct sub-keyword instead of find. This works for arrays and objects. Thanks for contributing.