-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Disallow fragments in uri-references that point elsewhere in the OpenAPI description #4423
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
Disallow fragments in uri-references that point elsewhere in the OpenAPI description #4423
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The definition is fine, but AFAICT the only place where it would be used would be the new $self
field. I thought there were more but I can't think of any now.
schemas/v3.1/schema.yaml
Outdated
@@ -621,8 +619,7 @@ $defs: | |||
type: object | |||
properties: | |||
operationRef: | |||
type: string | |||
format: uri-reference | |||
$ref: '#/$defs/uri-reference-no-fragment' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
operationRef
is allowed to (and in fact almost always does) have a fragment.
schemas/v3.1/schema.yaml
Outdated
@@ -214,8 +213,7 @@ $defs: | |||
type: object | |||
properties: | |||
$ref: | |||
type: string | |||
format: uri-reference | |||
$ref: '#/$defs/uri-reference-no-fragment' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the $ref
fields are allowed to have a fragment.
schemas/v3.1/schema.yaml
Outdated
@@ -716,8 +713,7 @@ $defs: | |||
type: object | |||
properties: | |||
$ref: | |||
type: string | |||
format: uri-reference | |||
$ref: '#/$defs/uri-reference-no-fragment' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the $ref
fields are allowed to have a fragment.
@@ -11,8 +11,7 @@ properties: | |||
info: | |||
$ref: '#/$defs/info' | |||
jsonSchemaDialect: | |||
type: string | |||
format: uri-reference | |||
$ref: '#/$defs/uri-reference-no-fragment' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with $schema
, a fragment is allowed. It is not particularly good practice, and if matching a $id
there are limits on what fragment, but $schema
(and therefore jsonSchemaDialect
) is not restricted to only targeting $id
values.
finally got all caught up: proposal-->pr 4389. @karenetheridge , @rahulsom are we missing anything? |
Looking again.. I think I was on autopilot when I wrote this ;) |
9cf8ec8
to
14753e6
Compare
Fixed. This only applies (for now) to jsonSchemaDialect, as a metaschema cannot be embedded inside another document e.g. as a definition. |
Can you provide a citation for this? §8.1.1 The "$schema" keyword states:
It goes on to forbid using
(honestly I'm a bit surprised, as I also thought it was forbidden, but I can't find any requirement forbidding it?) |
14753e6
to
307e9bc
Compare
This should also apply to $self, when it is added.
307e9bc
to
ecd0e3e
Compare
Ok, a However, Is it legal to refer to a metaschema by a non-canonical identifier (using a json pointer fragment from a higher location)? I created issue # 1590 in the json-schema-spec repo to get some clarity (and we should really tighten this spec language). |
@karenetheridge what you're talking about here is where |
@karenetheridge I guess part of this is that we typically assume that any metaschema will itself have a |
Where it's used has relevance as to what its address is (whether canonical or non-canonical). If it can only be used at a document root, then it would never have a json pointer fragment. I did find a place in the spec that says that |
@karenetheridge I'm still confused. Yes, $schema: "#/$defs/metaschema"
$defs:
$ref: "https://json-schema/draft/2020-12"
unevaluatedProperties: false Now, this is not that useful since |
@handrews (leaving aside the missing name for the definition..) I don't think that last example can ever work because the metaschema is contained inside the schema -- and in order to parse the entire document we need to know what the metaschema is first, which hasn't been parsed yet. |
@karenetheridge but what is the requirement that says so? I just don't think we can forbid a syntax without a requirement that forbids it. There are a lot of things in JSON Schema (and the OAS) that don't "work" but aren't forbidden. |
@karenetheridge Actually that is not entirely true. The example only relies on the core vocabulary, which MUST be assumed in order to bootstrap anything. If you take out the |
I'm just saying that using an example that cannot mathematically work is not a useful example :) |
@karenetheridge OK in that case |
There's nothing to do here; the definition can be added to the |
@handrews to pick up the relevant pieces from this and follow up with another PR. |
As referenced in #4389
Tick one of the following options: