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

jsonschema resolving $ref values as JSON Pointers #758

Closed
wheelerlaw opened this issue Nov 20, 2020 · 8 comments
Closed

jsonschema resolving $ref values as JSON Pointers #758

wheelerlaw opened this issue Nov 20, 2020 · 8 comments
Labels
Bug Something doesn't work the way it should.

Comments

@wheelerlaw
Copy link

wheelerlaw commented Nov 20, 2020

Given the following malformed reference:

"$ref": "common.json#properties/version"

jsonschema is resolving it as if it were a JSON Pointer. However, it is not, since according to section 3 of RFC 6901, JSON Pointers always begin with a leading /:

A JSON Pointer is a Unicode string (see [RFC4627], Section 3)
containing a sequence of zero or more reference tokens, each prefixed
by a '/' (%x2F) character.

However, it can't be resolved as a subschema identifier either because subschema identifiers use plain-named fragments and don't allow / in the fragment.

See section 8.2.3 of the draft 7 spec of JSON Schema:

Using JSON Pointer fragments requires knowledge of the structure of
the schema. When writing schema documents with the intention to
provide re-usable schemas, it may be preferable to use a plain name
fragment that is not tied to any particular structural location.
This allows a subschema to be relocated without requiring JSON
Pointer references to be updated.

To specify such a subschema identifier, the "$id" keyword is set to a
URI reference with a plain name fragment (not a JSON Pointer
fragment). This value MUST begin with the number sign that specifies
a fragment ("#"), then a letter ([A-Za-z]), followed by any number of
letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons
(":"), or periods (".").

As well as section 5:

Per the W3C's best practices for fragment identifiers
[W3C.WD-fragid-best-practices-20121025], plain name fragment
identifiers in "application/schema+json" are reserved for referencing
locally named schemas. All fragment identifiers that do not match
the JSON Pointer syntax MUST be interpreted as plain name fragment
identifiers.

The syntactically correct form of the first example would be:

"$ref": "common.json#/properties/version"

(notice the / after the #)

or

"$ref": "common.json#properties"

where some subschema has "$id": "#properties".

If I understand the code correctly, some changes are needed in the ref function in _validators.py, so to parse the value and determine if it s a JSON Pointer, a subschema identifier, or a URI reference.

Section 8.2.4 of the JSON Schema spec has some good examples.

@Julian
Copy link
Member

Julian commented Nov 20, 2020

Hi, thanks.

I believe this likely will be a duplicate of #371 (i.e. for location-independent identifier support, which is a known open issue) -- though GitHub closed #371 when I moved the master branch, even though it wasn't finished, so happy to keep this open.

But yeah I suspect it's the same issue (and is covered perhaps by the same tests which we currently skip until that's fixed).

@wheelerlaw
Copy link
Author

Ah, yes. It does look like it is a duplicate of that, sorry for not checking to see if it had already been brought up. I mainly just opened this since it's in relation to json-schema-org/JSON-Schema-Test-Suite#449.

@Julian
Copy link
Member

Julian commented Nov 20, 2020

No worries! All good -- I do assume though the upstream test suite issue may be closeable, because yeah I think this behavior is likely already covered by the tests that are being skipped. We can have a look though.

@wheelerlaw
Copy link
Author

I'm not sure that the tests exists, I did look through the existing tests quickly and I didn't find that negative test case of a $ref in the format of <uri>#incorrect/json/pointer. But if you know that one exists, I would be happy to close that issue.

@Julian Julian added the Bug Something doesn't work the way it should. label Dec 5, 2020
@YKdvd
Copy link

YKdvd commented May 9, 2021

I'm not quite sure, but I believe this (and the closed but not fixed) #371 covers the simple case of {"$ref": "#some_local_id"} using and $id in a $ref? That seems to be part of the draft-07 spec, and doesn't seem to work with a test I did with jsonschema 3.2.0 from PyPI or the master branch here? Is this an exception to the "Full support for Draft 7"?

@Julian
Copy link
Member

Julian commented May 9, 2021

#371 was closed accidentally when renaming the master branch and GH doesn't allow reopening it. But yes location independent identifiers are a known bug (one of a few -- see the test suite for the small number of tests that are skipped).

@Julian
Copy link
Member

Julian commented Feb 23, 2023

Hello there!

This, along with many many other $ref-related issues, is now finally being handled in #1049 with the introduction of a new referencing library which is fully compliant and has APIs which I hope are a lot easier to understand and customize.

The next release of jsonschema (v4.18.0) will contain a merged version of that PR, and should be released shortly in beta, and followed quickly by a regular release, assuming no critical issues are reported.

The new APIs seem to behave better here, for something like your example:

from referencing import Registry, Resource
from referencing.jsonschema import DRAFT7
import jsonschema
common = DRAFT7.create_resource({"properties": {"version": {}}})
registry = Registry().with_resource("common.json", common)
jsonschema.validate(
    schema={"$ref": "common.json#properties/version"},
    instance=12,
    registry=registry,
)

it indeed will try to resolve that as a plain-name fragment, not a pointer. (As you say, technically this isn't valid there either as a schema, but it'd seem the metaschema does not enforce that invalidity).

If you still care to, I'd love it if you tried out the beta once it is released, or certainly it'd be hugely helpful to immediately install the branch containing this work (https://github.com/python-jsonschema/jsonschema/tree/referencing) and confirm. You can in the interim find documentation for the change in a preview page here.

I'm going to close this given it indeed seems like it is addressed by #1049, but feel free to follow up with any comments. Sorry for the delay in getting to these, but hopefully this new release will bring lots of benefit!

@Julian Julian closed this as completed Feb 23, 2023
@wheelerlaw
Copy link
Author

Unfortunately I have lost most of the context that I had when I originally opened this ticket and I no longer am in the role I was in, but good to know that this is fixed!

Julian added a commit that referenced this issue Jan 31, 2025
4ba013d5 Merge pull request #747 from santhosh-tekuri/duration
aa500e80 Merge pull request #749 from json-schema-org/gregsdennis/json-everything-update
eb8ce976 Merge pull request #757 from ajevans99/main
dcdae5c0 Merge pull request #758 from sirosen/hostname-format-check-empty-string
db21d21b Merge branch 'main' into hostname-format-check-empty-string
3fd78f04 Merge pull request #1 from ajevans99/swift-json-schema
3cada3a9 Update README.md
82a07749 Merge pull request #753 from json-schema-org/ether/fix-draft-locations
a66d23d4  move draft-specific files to the dedicated dir for its draft
8ef15501 Merge pull request #751 from big-andy-coates/format_tests_under_format
fe1b1392 All format test cases should be under the `format` directory.
b1ee90f6 json-everything moved to an org
c00a3f94 test: duration format must start with P
9fc880bf Merge pull request #740 from notEthan/format-pattern-control-char
cbd48ea5 Simplify test of \a regex character to test directly against `pattern` schema
d6f1010a Merge pull request #746 from json-schema-org/annotations
4aec22c1 Revert the changes to additionalProperties.json.
2dc10671 Move the workflow step title.
d9ce71ac May as well also show quotes in the annotation.
1b719a84 Pick the line after the description when attaching spec annotations.
08105151 Markdown is apparently not (yet?) supported in annotations.
81645773 Tidy up the specification annotator a bit.
38628b79 Make the spec URLs structure a bit easier for internal use.
4ebbeaf4 Merge branch 'Era-cell/main'
e4bd7554 dumbness2 corrected
d8ade402 inside run
57c7c869 changed install location
11f8e511 Added installing command in workflow
f2766616 template library, url loads changes
c2badb12 Merge pull request #734 from OptimumCode/idn-hostname-arabic-indic-mixed
dd9599a5 Merge branch 'main' of github.com:json-schema-org/JSON-Schema-Test-Suite
5b393436 add pr dependencies action
3a509007 Clear existin annotations on same PR
23674123 Cases for rfc and iso written separately
0b780b2c Corected yaml format
2b1ffb74 Best practices followed with optimized code
e88a2da6 Works for all OS
7b40efe4 Base path for neighbouring file?
564e6957 Walking through all leaf files
7b84fb44 Merge branch 'main' of https://github.com/Era-cell/JSON-Schema-Test-Suite
891d0265 First workflow2
1c175195 regex correction
96f7683a Final correction2 - file names beautufied
5f050a07 Final correction1
77527b63 Stupidity corrected
eb8fd760 Branch name specified
540a269b Log2
f29d090a Wrong location sepcification
582e12be logging logs check
df3bdecc path corrected
c6b937ca Reading all jsons and spec urls added
cbdd1755 change day2
79dc92f1 TOKEN
ce52852d Python file location changed
3558c2c6 Fake add to tests
eecc7b7a Merge branch 'main' of https://github.com/Era-cell/JSON-Schema-Test-Suite
810d148a First workflow2
4eac02c7 First workflow
40bcb8b3 Corrected replaced unevaluated with additoinalProperties
4ae14268 Add valid first character to avoid Bidi rule violation
202d5625 test: hostname format check fails on empty string

git-subtree-dir: json
git-subtree-split: 4ba013d58e747ecaf48c8bb7cf248cb0d564afbc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something doesn't work the way it should.
Projects
None yet
Development

No branches or pull requests

3 participants