Skip to content

Add tests for reference-to-unknown to earlier drafts #348

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

Closed
ssilverman opened this issue Apr 25, 2020 · 5 comments
Closed

Add tests for reference-to-unknown to earlier drafts #348

ssilverman opened this issue Apr 25, 2020 · 5 comments
Labels
missing test A request to add a test to the suite that is currently not covered elsewhere.

Comments

@ssilverman
Copy link
Member

ssilverman commented Apr 25, 2020

Only Draft 2019-09 has these tests, in optional/refOfUnknownKeyword.json. This issue tracks adding them to the earlier draft tests, all the way back to draft-03.

@Julian
Copy link
Member

Julian commented Apr 25, 2020

I have to look back at history, but my quick recollection is this is actually new in 2019-09. I think previous drafts actually required this behavior (and that there are tests already ensuring it's followed)

But I'm not 100% positive, I remember some recent discussion about it -- maybe hunting down the commit that added this to 2019-09 is a good first step?

@ssilverman
Copy link
Member Author

Turns out I don't fully understand this yet. I found this: json-schema-org/json-schema-spec#713, the bug it's fixing (json-schema-org/json-schema-spec#687), and other things that refer to it. [duplicate comment to one I added to #349]

@Julian
Copy link
Member

Julian commented Apr 26, 2020

Those definitely look like the relevant discussions yep.

@Julian Julian added the missing test A request to add a test to the suite that is currently not covered elsewhere. label May 30, 2020
@handrews
Copy link
Contributor

Yes, this was added in 2019-09. The pathological case is this (where singleSubschemaDefinition is like definitions or $defs but with one subschema, and the implementation is not aware of this):

$id:  https://example.com/schemas/foo
properties:
   thing: {$ref: "#/singleSubschemaDefinition/singleSubschemaDefinition"}
singleSubschemaDefinition:
    $id: bar/baz                # resolves to https://example.com/bar/baz
    singleSubschemaDefinition:
        $id: stuff                 # resolves to https://example.com/bar/stuff
        $ref: zippy              # resolves to https://example.com/bar/zippy

The problem here is that the JSON Pointer-based $ref gets us to the "stuff" schema, but without understanding that keyword, the implementation wouldn't notice the intervening $id: bar/baz. So it would see the $id in the "stuff" schema, and resolve it to https://example.com/stuff (note the missing /bar), and based on that, resolve the $ref in that schema to https://example.com/zippy (again, missing the /bar).

So in 2019-09 and later, we allow implementations to refuse to resolve $ref: "#/singleSubschemaDefinition/singleSubschemaDefinition" because doing so fails cleanly rather than having $ref: zippy resolve to the wrong place.

However, in draft-07 and earlier, there is nothing that allows refusing to resolve a JSON Pointer-based URI where you understand the non-fragment part. Here is the relevant langauge:

Schemas can be identified by any URI that has been given to them,
including a JSON Pointer or their URI given directly by "$id". In
all cases, dereferencing a "$ref" reference involves first resolving
its value as a URI reference against the current base URI per RFC
3986 [RFC3986].

If the resulting URI identifies a schema within the current document,
or within another schema document that has been made available to the
implementation, then that schema SHOULD be used automatically.

There is nothing that discusses how an implementation can tell if a given URI identifies a schema, so the reasonable assumption is that if the location exists and is an object or boolean, it is treated as a schema. Because there's nothing that automatically disqualifies an object from being a schema.

So I agree with @Julian here, this issue can be closed as this test should not be backported earlier than 2019-09.

@Julian
Copy link
Member

Julian commented Aug 12, 2022

Gonna go with that, closing, thanks!

@Julian Julian closed this as completed Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing test A request to add a test to the suite that is currently not covered elsewhere.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants