json-schema-org / JSON-Schema-Test-Suite

A language agnostic test suite for the JSON Schema specifications
MIT License
615 stars 205 forks source link

add `$dynamicAnchor` to meta-schemas #715

Closed notEthan closed 8 months ago

notEthan commented 8 months ago

These meta-schemas need $dynamicAnchor. The meta/core and meta/applicator vocabulary schemas use $dynamicRef: "#meta" and without $dynamicAnchor in these meta-schemas, those resolve to the vocabulary schema's $dynamicAnchor instead of resolving to these meta-schemas. For a schema described by one of these meta-schemas, its subschemas are then described by the vocabulary schema, but not the meta-schema. This affects subschemas such as the properties subschemas in https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/b41167c7/tests/draft2020-12/vocabulary.json#L8-L11

That example is the only place it's actually an issue in practice, because no other test schemas using these meta-schemas have any subschemas.

I did this for 2020-12 and draft-next - I don't have an implementation of 2019-09 but I think its custom meta-schemas should have $recursiveAnchor: true? Somebody with an implementation or better knowledge, please weigh in.

karenetheridge commented 8 months ago

Nice find! I wonder if we should add some tests that would have failed before this change was made? Otherwise, it might be possible that some implementations assume the presence of $dynamicAnchor even when not present (i.e. we want to make sure this change actually changes behaviour).

jdesrosiers commented 8 months ago

I wonder if we should add some tests that would have failed before this change was made?

This is a great thought! But, I don't think there's anything we can do in this case. The missing dynamic anchor will only effect how the schema is validated, not what keywords are part of the dialect, which is what is being tested. The test suite currently doesn't have a way to express tests for schema validation, only instance validation. There's an argument to be made for removing the references and anchors altogether because they don't have any effect on what's being tested.

notEthan commented 8 months ago

Thanks Jason, updated.

karenetheridge commented 8 months ago

There must be something we can do, otherwise

This affects subschemas such as the properties subschemas...

..wouldn't be true. I don't have the time to sit down and stare at this in detail right now, but surely there is some change in the way the schemas are evaluated by adding the keyword that we can leverage in a test case?

giorgioghelli commented 8 months ago

We can write a test. Consider the following schema:

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "$id": "http://localhost:1234/draft2020-12/metaschema-no-validation.json",
    "$vocabulary": {
        "https://json-schema.org/draft/2020-12/vocab/applicator": true,
        "https://json-schema.org/draft/2020-12/vocab/core": true
    },
    "$dynamicAnchor": "meta",
    "allOf": [
        { "$ref": "https://json-schema.org/draft/2020-12/meta/applicator" },
        { "$ref": "https://json-schema.org/draft/2020-12/meta/core" }
    ]
}

Consider the following instance:

{ "items" : {"$comment" : 1 }}

Validation fails because, inside the "applicator" scope, when you find "$dynamicRef": "#meta" you come back to the root. Here, you validate { "$comment" : 1 } with both "applicator" and "core", and it fails, because of "core".

giorgioghelli commented 8 months ago

Now, consider the following schema where we forgot to redefine "meta" at the top level:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "http://localhost:1234/draft2020-12/metaschema-no-validation.json",
  "$vocabulary": {
    "https://json-schema.org/draft/2020-12/vocab/applicator": true,
    "https://json-schema.org/draft/2020-12/vocab/core": true
  },
  "allOf": [
    { "$ref": "https://json-schema.org/draft/2020-12/meta/applicator" },
    { "$ref": "https://json-schema.org/draft/2020-12/meta/core" }
  ]
}

Now, the following instance

{ "items" : {"$comment" : 1 } }

is validated, because, inside the "applicator" scope, when you find "$dynamicRef": "#meta" you recur inside the "applicator" schema, and, in this schema, there is no constraint defined for "$comment", so that an integer value is ok.

giorgioghelli commented 8 months ago

I tried this example over a couple of validators. https://jschon.dev/schema and https://validationproofs.oa.r.appspot.com/ yield the correct results, while https://json-everything.net/json-schema/ fails even in the second case, as if it interpreted $dynamicRef: #meta as a reference to the root schema even in the second case when "$dynamicAnchor" : "meta" is not defined at the root level

giorgioghelli commented 8 months ago

The schema I am using here is that of https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/a01aeee6510a0cf7f1982fd18753d2456fde45cd/remotes/draft2020-12/metaschema-no-validation.json

giorgioghelli commented 8 months ago

PS: of course the same example works for 2019-09 with $recursiveRef, and for "next".

giorgioghelli commented 8 months ago

...it is funny because this is essentially the example that we used in the paper that I presented at POPL two weeks ago in order to explain why dynamic references are useful
image

https://dl.acm.org/doi/pdf/10.1145/3632891

jdesrosiers commented 8 months ago

@giorgioghelli The schemas in question here are fixtures that are used for the tests here. They're used as values for $schema. Using your example, a schema under test would look like this,

{
  "$schema": "http://localhost:1234/draft2020-12/metaschema-no-validation.json",
  "items": { "$comment": 1 }
}

We expect that to be an invalid schema, but we have no way in our test framework to represent that we expect a schema to be invalid. We can only assert that an instance is valid or invalid against a valid schema. Therefore, your example isn't useful for this case.

What could work for this case is a schema that would be an invalid schema if the dynamic reference was missing and valid if it were present. I don't think there's a way to do that in this case, but I could be wrong. I'm not even sure it's worth trying to find a case because that has nothing to do with what is being tested.

However, you may have found a dynamic reference case we don't already cover if JSON Everything is not behaving as expected. We should definitely dig into that and come up with a new dynamic reference test case based on your example.

jdesrosiers commented 8 months ago

I tried this example over a couple of validators. https://jschon.dev/schema and https://validationproofs.oa.r.appspot.com/ yield the correct results, while https://json-everything.net/json-schema/ fails even in the second case, as if it interpreted $dynamicRef: #meta as a reference to the root schema even in the second case when "$dynamicAnchor" : "meta" is not defined at the root level

I just tried the schema on https://json-everything.net/json-schema/ and got the correct/expected result. It seems to be working fine.

giorgioghelli commented 8 months ago

@giorgioghelli The schemas in question here are fixtures that are used for the tests here. They're used as values for $schema. Using your example, a schema under test would look like this,

{
  "$schema": "http://localhost:1234/draft2020-12/metaschema-no-validation.json",
  "items": { "$comment": 1 }
}

We expect that to be an invalid schema, but we have no way in our test framework to represent that we expect a schema to be invalid. We can only assert that an instance is valid or invalid against a valid schema. Therefore, your example isn't useful for this case.

What could work for this case is a schema that would be an invalid schema if the dynamic reference was missing and valid if it were present. I don't think there's a way to do that in this case, but I could be wrong. I'm not even sure it's worth trying to find a case because that has nothing to do with what is being tested.

However, you may have found a dynamic reference case we don't already cover if JSON Everything is not behaving as expected. We should definitely dig into that and come up with a new dynamic reference test case based on your example.

I see, We do not want to test here how the presence/absence of "$dynamicAnchor" : "meta" changes the validation behaviour of "metaschema-no-validation.json", but we would like to test how the presence/absence of "$dynamicAnchor" : "meta" changes the validation behaviour of a schema that has "metaschema-no-validation.json" as its meta-schema. Of course the presence/absence of that line changes which schemas are "valid" with respect to the metaschema (as showed by my example), but that is not what is tested here - the only aspect of the metaschema that affects the validation behavior of the schema is the "$vocabulary" keyword, everything else is essentially irrelevant. I hope that now I understood.

giorgioghelli commented 8 months ago

I tried this example over a couple of validators. https://jschon.dev/schema and https://validationproofs.oa.r.appspot.com/ yield the correct results, while https://json-everything.net/json-schema/ fails even in the second case, as if it interpreted $dynamicRef: #meta as a reference to the root schema even in the second case when "$dynamicAnchor" : "meta" is not defined at the root level

I just tried the schema on https://json-everything.net/json-schema/ and got the correct/expected result. It seems to be working fine.

I confirm, I tried again with https://json-everything.net/json-schema/ and now I get the expected results. I use the web interface, and apparently I need to reload the page when I change the schema - I have not done that during my previous test, sorry about this.