-
-
Notifications
You must be signed in to change notification settings - Fork 308
prohibit the use of fragments in $id and $schema #1059
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
Comments
I'd rather go the other direction and make For
For {
"type": "object",
"properties": {
"foo": {
"$id": "/schema/bundle#/$defs/foo",
"$defs": {
"string": { "type": "string" },
"foo": { "$ref": "#/defs/string" }
}
}
}
} In this example, If we change that example slightly, I think it's a little easier to see how the {
"type": "object",
"properties": {
"foo": { "$ref": "/schema/bundle#/$defs/foo" }
},
"$defs": {
"": {
"$id": "/schema/bundle",
"$defs": {
"string": { "type": "string" },
"foo": { "$ref": "#/defs/string" }
}
}
}
} It's certainly more intuitive written like this, but I don't think aesthetics are good enough reason to forbid using fragments in |
allowing pointer fragments in $id was an oversight to begin with. disallowing it resolved a confusing/undefined situation where a schema could identify itself as being at one pointer, while actually being at a different pointer in the resource or document. @jdesrosiers I'm not sure why $id should allow a fragment, even in your proposal - which I understand is not the same as declaring the location of the schema itself as schemas used to sometimes do. what you suggest looks like a confusing alternative to $ref, and I think your example schema has an equivalent using $ref: {
"type": "object",
"properties": {
"foo": {
"$id": "/schema/bundle",
"$ref": "#/$defs/foo",
"$defs": {
"string": { "type": "string" },
"foo": { "$ref": "#/defs/string" }
}
}
}
} I see no upside to putting a fragment in $id, only confusion. whether $id should allow an empty fragment, I think it should be disallowed at some point. it has remained for historical reasons, I think, but every new draft breaks compatibility of more major things than that anyway. as for a $schema with a fragment, I don't know that it matters. if the $schema URI does not point to a metaschema the implementation is able to use, fragment or no, the implementation will presumably error attempting to resolve that. it seems faintly plausible to me that a metaschema could be identified by $schema using a fragment (my implementation does support metaschemas which are not at the root of their document) - but it's hard to imagine the value of such a thing. |
This was exactly the use-case we were discussing over at OAS. We already have a meta-schema for OAS 3.0 (and 1.2 and 2.0) but it contains embedded definitions of the OAS schemaObject (which in these versions is a superset-subset of proper JSON Schema). It would be useful for users who want to continue to use OAS 3.0-compatible schemaObjects to be able to reference the schemaObject definition within the existing metaschema using |
@MikeRalphson that is interesting and worth thinking about in this context, although I think that embedded metaschema will almost certainly have an $id, and so not need to use any fragment to refer to it with $schema. if the document it is embedded in has another id, it might be possible to address that metaschema using that id with a fragment, but it would probably be strongly recommended against (which is why I say above I don't imagine such a construct probably has much value). I'm making a number of suppositions here without knowledge of the openapi work, though. |
@notEthan That's useful - thanks. The use of a separate |
In 2019-09,
I know the situation you're referring to. It is confusing, but it has never been undefined. That has always been an unambiguously invalid use of
Yep, that's another equivalent example. Allowing
It makes for a simpler conceptual model. Fewer concepts with fewer exceptions means a simpler spec and implementations that are easier to write, have better performance, and are less bug prone. That's the main benefit. I agree that that benefit is marginal, but I'd rather not have exceptions to rules without a good reason, and don't see a good reason.
The same URI can be represented by multiple strings. It's the normalized URI that's important. For example,
I agree, but I don't want to do extra work to forbid something that isn't causing any harm just in case someone comes up with a good use for it. |
I want to be clear that I opened this issue to discuss these constructs:
|
First of all, sorry for hijacking this issue a bit, but I thought it was necessary to give an explanation for why I'd prefer to go a different direction. I'm happy to drop this if there's not support, but ...
this statement makes me think I haven't presented the proposal clearly. I'm not proposing bringing anything back. Every problem we have previously fixed will stay fixed. My proposal introduces something that never existed before. Originally, fragments in
I'm not following your point here, but URIs with fragments can be canonical. Example from the spec ...
|
I think modifications to the $schema and $id keywords would be best discussed in a new issue (and separate issues, as they are two very different things).
|
I'll drop it then. I might bring it up again in the future if I come up with a better way to explain it.
What I had in mind doesn't alter the resource root, only the evaluation starting point (just like {
"$id": "https://example.com/my-schema",
"$schema": "https://json-schema.org/draft/future/schema#strict"
} {
"$id": "https://json-schema.org/draft/future/schema",
"$schema": "https://json-schema.org/draft/future/schema",
... standard schema ...
"$defs": {
"strict": {
"$anchor": "strict"
... strict schema ...
}
}
} Nothing has to change about where |
I looked this up and I was wrong. Normalization does not remove an empty fragment.
|
I revised my earlier comment - I mixed up the rules for where a $schema keyword can be, and the content of its URI itself, sorry! I think the spec is silent at the moment about whether a $schema URI can have a fragment.. so perhaps it is already possible today to have a schema containing e.g. |
I would be thrilled to see the empty fragment banned from both @jdesrosiers the reason fragments do not have scheme-based normalization is that the fragment syntax and semantics are entirely defined by the media type. The URI scheme has nothing to do with fragments, nor does the RFC 3986 normalization process. For JSON Schema, an empty fragment and no fragment are the same, because we defined them to be that way. This is why the spec for
I would also really hate to see fragments returned to |
It was definitely possible to do, and it was not clearly defined. I know because I did a very, very detailed implementation of Once you started dealing with not only the behavior of the fragment in |
Ever since draft-05, it's been defined what characters are allowed in |
@jdesrosiers if you mean this section of draft-05, specifically:
the key phrases are Regardless, if you want to argue for reverting the |
@handrews You're right.
I can't imagine where you got the idea that I want to revert the (Let's not drag this out. I have no desire to argue for this any time soon.) |
@jdesrosiers To me, the point of I agree on not dragging this out, but perhaps if we just clarify one small point at a time that will feel like progress rather than a battle. |
@MikeRalphson continuing to think on this, my intuition is that it is somewhat important for a meta-schema to be a schema resource, and not just a schema object. But I'm not 100% and before diving into that topic, I wanted to check: you can solve this right now by just putting a |
@handrews there's no reason it's impossible for us to do that, the only reason not to prefer that approach is that it means one more date to remember to update within the larger document every time we rev the meta-schema... |
@MikeRalphson hmm... changing this to avoid a fairly straightforward (if annoying) mechanical change for a relatively rare occurrence feels a little meh, although I might eventually be convinced. I did remember why I think Discussions in #1098 might let us relax that, although I'm not sure. This is a good example of how apparently-superficial changes actually have a lot of knock-on effects deeper in the system. |
I was about to file a new The new If we drop fragments in @jdesrosiers I just want to acknowledge that I misunderstood your proposal when I commented on it before and thought you were undoing the |
I commented previously mostly on @jdesrosiers' alternative but I will comment here in favor of disallowing empty fragments in $id, for the same reasons @handrews states. (I did say above it should be disallowed at some point - now seems good.) I am opposed to disallowing at least an empty fragment from $schema (it probably does warrant its own issue), since that is used for selection across specification versions, including specs whose metaschemas do have an empty fragment in their id. Using a non-empty fragment in $schema seems mostly like a poorer idea than just giving a metaschema an $id, worth strongly recommending against, but I don't know about forbidding that either. Changing the rules of $schema in any one specification is a bit fraught (as other currently-active discussions illustrate). |
We permit empty fragments in
$id
, and say nothing at all about fragments in$schema
. IMO we should prohibit all kinds of fragments in both keywords' URIs.cross-reference: OAI/OpenAPI-Specification#2433 (comment)
The text was updated successfully, but these errors were encountered: