Skip to content
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

Note undefined behavior with unknown $ref targets #713

Merged
merged 2 commits into from
Mar 20, 2019

Conversation

handrews
Copy link
Contributor

Fixes #687 (in the sense of "it's clear what you are and are not required to implement", not in the sense of "solves the underlying applicator keyword recognition issue", which will be part of the draft after this as it is quite involved").

The most common situation here, which are basically alternate
names for "definitions" or "$defs", will generally work, but
this makes clear that there is no obligation to attempt to
guarantee the proper behavior in such situations.

We might want to narrow this behavior down in the future,
but for now this at least clearly removes any possible burdensome
and totally impractical inferencing requirements.

@handrews
Copy link
Contributor Author

Note that the CI build is failing because of segmentation faults in xml2rfc, which has to be some sort of environmental problem. They build fine locally, and FFS how does XML provoke a segfault? Is there a buggy version of xml2rfc?

The most common situation here, which are basically alternate
names for "definitions" or "$defs", will generally work, but
this makes clear that there is no obligation to attempt to
guarantee the proper behavior in such situations.

We might want to narrow this behavior down in the future,
but for now this at least clearly removes any possible burdensome
and totally impractical inferencing requirements.
Whether the target is definitely not a schema or simply
unknown and therefore not necessarily a schema, the result
of $ref-ing it is undefined.  Only references to targets
that the implementation can determine with certainty to be
schemas have well-defined behavior.
Copy link
Member

@Relequestual Relequestual left a comment

Choose a reason for hiding this comment

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

Super!
Do we want to add a note in the changelog with this commit, also noting that as such, [specifc test in the referenced issue. I can't find which right now.] has been removed?

@handrews
Copy link
Contributor Author

@Relequestual I'll do the change log as a batch near the end- lots of things are missing, but the issues are all tagged with the milestone so it won't be hard to go through and write it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Items that need to be clarified in the specification core Priority: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants