Skip to content

Consolidate and clarify language around duplicate IRIs #1272

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 6, 2022
Merged

Consolidate and clarify language around duplicate IRIs #1272

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29f4a12
Disallow "#" as an $id.
handrews Aug 13, 2022
bdbf918
Consolidate and clarify duplicate IRI behavior
handrews Aug 13, 2022
170ba3b
Note that empty fragments produce duplicates
handrews Aug 13, 2022
853c82b
Don't forbid cases already noted as errors
handrews Aug 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions jsonschema-core.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1353,8 +1353,8 @@
</t>
<t>
If present, the value for this keyword MUST be a string, and MUST represent a
valid <xref target="RFC3987">IRI-reference</xref>. This IRI-reference
SHOULD be normalized, and MUST resolve to an
valid <xref target="RFC3987">IRI-reference</xref> with a non-empty non-fragment
component. This IRI-reference SHOULD be normalized, and MUST resolve to an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change is necessary. The two valid options are presented as an absolute-IRI or an IRI with an empty fragment. Both absolute-IRI and an IRI are by definition non-relative, so we don't need to say "non-empty". It also says it "MUST resolve to", which also indicates a non-relative result.

Copy link
Contributor

@notEthan notEthan Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers: This doesn't sound right. $id can be a relative IRI. If that relative IRI is empty (apart from fragment), it resolves to the same absolute IRI as the parent schema (or the same as its retrieval IRI if it's at the root, but that part isn't a problem). Then a subschema has the same id as its parent. Forbidding the empty non-fragment portion disallows this id collision.

@handrews: The wording is awkward, "with a non-empty non-fragment component". Maybe a new sentence would be better:

If present, the value for this keyword MUST be a string, and MUST represent a valid IRI-reference. [unchanged]
This IRI-reference MUST NOT be empty or consist only of an empty fragment.
[or]
This IRI-reference MUST NOT have an empty non-fragment component.

This IRI-reference SHOULD be normalized ...

Copy link
Contributor Author

@handrews handrews Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdesrosiers I'm not sure I understand. "$id": "foo/bar" is valid, both before and after this change. It is a path-only relative IRI-reference that, resolved against a base IRI (say, https://example.com/schemas/base) produces an absolute-IRI (https://example.com/schemas/foo/bar).

@notEthan Thanks, I think it like "This IRI-reference MUST NOT be empty or consist only of an empty fragment." I checked RFC 3986 and it uses the term "empty" to describe relative references in §4.4 Same-Document Reference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't saying that $id can't be relative. I was talking about the result of resolving the $id against the base IRI. I thought this change was trying to protect against case where you don't have a baseURI and therefore the IRI can't be fully resolved. I was pointing out that the current wording already protects against that case.

If that relative IRI is empty (apart from fragment), it resolves to the same absolute IRI as the parent schema

Oh, ok, now I understand where this change is coming from. But, I still don't think it's necessary because we talk about not allowing duplicate identifiers more generally elsewhere. IMO, it's already covered and doesn't need to be repeated here.

Copy link
Contributor Author

@handrews handrews Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this change was trying to protect against case where you don't have a baseURI and therefore the IRI can't be fully resolved. I was pointing out that the current wording already protects against that case.

Ah, I see. Yeah, that is already handled and not improved (or made worse) by this change.

we talk about not allowing duplicate identifiers more generally elsewhere. IMO, it's already covered and doesn't need to be repeated here.

Cool- yeah, that's the main question (for one of the commits in this PR- the rest of it remains valid as it clarifies that "already covered" part that, as currently written, kinda contradicts itself in a confusing way).

I'd like to leave this open for probably another week and see if we get any more opinions from other @json-schema-org/spec-team folks (or anyone else, I just can't tag the whole universe). I don't feel strongly on this point, so if no one advocates for the change I'll strip it out and we can go with just the clean-up of the duplicate IRI stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option might be to note that, while syntactically allowed, "" and "#" effectively produce duplicate IRIs which are considered errors per §whatever:

Note that while the empty IRI and the empty fragment-only IRI are legal values, they produce duplicate IRIs when used in an embedded resource, which is considered an error per 8.2.3.

That's probably enough to discourage the values without requiring people to detect and block them. I'm currently leaning towards this option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be a little odd to include a MUST NOT with the intent of heading off a route to duplicate IRIs, when the language on duplicate IRIs itself isn't a MUST NOT ("implementations SHOULD raise an error condition").

<xref target="RFC3987">absolute-IRI</xref> (without a fragment),
or to a IRI with an empty fragment.
</t>
Expand Down Expand Up @@ -1456,11 +1456,21 @@
fragment "#foo" when used in a IRI. See below for full examples.
</cref>
</t>
</section>

<section title="Duplicate schema identifiers">
<t>
A schema MAY (and likely will) have multiple IRIs, but there is no way
for an IRI to identify more than one schema. When multiple schemas
attempt to identify as the same IRI through the use of "$id", "$anchor",
"$dynamicAnchor", or any other mechanism, implementations SHOULD raise
an error condition. Otherwise the result is undefined, and even if
documented will not be interoperable.
</t>
<t>
The effect of specifying the same fragment name multiple times within
the same resource, using any combination of "$anchor" and/or
"$dynamicAnchor", is undefined. Implementations MAY
raise an error if such usage is detected.
Note that due to the semantics of JSON Pointer fragments, schema IRIs
that differ only by the presence or absence of an empty fragment MUST
be considered duplicates.
</t>
</section>

Expand Down Expand Up @@ -1671,11 +1681,6 @@
be noted within a schema document as it is processed, producing associations
as shown in appendix <xref target="idExamples" format="counter"></xref>.
</t>
<t>
A schema MAY (and likely will) have multiple IRIs, but there is no way for a
IRI to identify more than one schema. When multiple schemas try to identify
as the same IRI, validators SHOULD raise an error condition.
</t>
</section>

<section title="Detecting a Meta-Schema">
Expand Down