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

schemas inappropriately declare IDs outside their namespace #4073

Open
rogpeppe opened this issue Sep 13, 2024 · 0 comments · May be fixed by #4074
Open

schemas inappropriately declare IDs outside their namespace #4073

rogpeppe opened this issue Sep 13, 2024 · 0 comments · May be fixed by #4074

Comments

@rogpeppe
Copy link
Contributor

Area with issue?

JSON Schema

✔️ Expected Behavior

When I import a schema from schemastore, it should define exactly one top level URL, with all subschemas available as fragment references (potentially anchors or plain-name fragment IDs) within that. That makes it clear that schemas will not step on one another's toes by defining a schema in another namespace. Although it's possible for schemas to do that, it doesn't seem to be good practice.

I noticed this being an issue when investigating #4063, and wrote a little program to find all such cases. The program also looked for any use (via $ref) of such IDs.

Here's a summary the places where the program found instances of IDs defined outside the schema's namespace, places where IDs were declared that duplicated other schema IDs in schemastore, and places where any of those IDs were actually used.

File position summary
NON FRAGMENT IDs

src/schemas/json/base.json:6:14
src/schemas/json/base.json:10:14
src/schemas/json/base.json:14:14
src/schemas/json/base.json:18:14
src/schemas/json/base.json:22:14
src/schemas/json/base.json:26:14
src/schemas/json/base.json:30:14
src/schemas/json/base.json:35:14
src/schemas/json/base.json:46:14
src/schemas/json/base.json:56:14
src/schemas/json/base.json:67:14
src/schemas/json/base.json:78:14
src/schemas/json/base.json:228:14
src/schemas/json/base.json:817:14
src/schemas/json/catalog-info.json:7:14
src/schemas/json/catalog-info.json:105:14
src/schemas/json/catalog-info.json:206:14
src/schemas/json/catalog-info.json:253:14
src/schemas/json/catalog-info.json:357:14
src/schemas/json/catalog-info.json:425:14
src/schemas/json/catalog-info.json:493:14
src/schemas/json/catalog-info.json:547:14
src/schemas/json/catalog-info.json:731:14
src/schemas/json/catalog-info.json:813:14
src/schemas/json/catalog-info.json:1018:14
src/schemas/json/cryproj.52.schema.json:6:14
src/schemas/json/cryproj.52.schema.json:2457:14
src/schemas/json/cryproj.52.schema.json:2650:14
src/schemas/json/cryproj.52.schema.json:2673:14
src/schemas/json/cryproj.52.schema.json:2748:14
src/schemas/json/cryproj.52.schema.json:2770:14
src/schemas/json/cryproj.52.schema.json:2834:14
src/schemas/json/cryproj.52.schema.json:2841:14
src/schemas/json/cryproj.52.schema.json:2848:14
src/schemas/json/pgap_yaml_input_reader.json:9:14
src/schemas/json/pgap_yaml_input_reader.json:17:14
src/schemas/json/pgap_yaml_input_reader.json:25:14
src/schemas/json/pgap_yaml_input_reader.json:35:14
src/schemas/json/pgap_yaml_input_reader.json:43:14
src/schemas/json/pgap_yaml_input_reader.json:65:14
src/schemas/json/pgap_yaml_input_reader.json:108:14
src/schemas/json/pgap_yaml_input_reader.json:115:14
src/schemas/json/pgap_yaml_input_reader.json:122:14
src/schemas/json/pgap_yaml_input_reader.json:233:14
src/schemas/json/pgap_yaml_input_reader.json:254:14
src/schemas/json/pgap_yaml_input_reader.json:262:14
src/schemas/json/pgap_yaml_input_reader.json:285:14
src/schemas/json/pgap_yaml_input_reader.json:369:14
src/schemas/json/pgap_yaml_input_reader.json:377:14
src/schemas/json/xs-app.json:67:14
src/schemas/json/youtrack-app.json:9:14
src/schemas/json/youtrack-app.json:16:14
src/schemas/json/youtrack-app.json:23:14
src/schemas/json/youtrack-app.json:32:14
src/schemas/json/youtrack-app.json:37:14
src/schemas/json/youtrack-app.json:43:14
src/schemas/json/youtrack-app.json:48:14
src/schemas/json/youtrack-app.json:53:14
src/schemas/json/youtrack-app.json:58:14
src/schemas/json/youtrack-app.json:114:14

DUPLICATE IDs:

src/schemas/json/cryproj.53.schema.json:6:14
src/schemas/json/cryproj.53.schema.json:2465:14
src/schemas/json/cryproj.53.schema.json:2665:14
src/schemas/json/cryproj.53.schema.json:2688:14
src/schemas/json/cryproj.53.schema.json:2763:14
src/schemas/json/cryproj.53.schema.json:2785:14
src/schemas/json/cryproj.53.schema.json:2849:14
src/schemas/json/cryproj.53.schema.json:2856:14
src/schemas/json/cryproj.53.schema.json:2863:14
src/schemas/json/cryproj.54.schema.json:6:14
src/schemas/json/cryproj.54.schema.json:2472:14
src/schemas/json/cryproj.54.schema.json:2671:14
src/schemas/json/cryproj.54.schema.json:2694:14
src/schemas/json/cryproj.54.schema.json:2769:14
src/schemas/json/cryproj.54.schema.json:2791:14
src/schemas/json/cryproj.54.schema.json:2855:14
src/schemas/json/cryproj.54.schema.json:2862:14
src/schemas/json/cryproj.54.schema.json:2869:14
src/schemas/json/cryproj.55.schema.json:6:14
src/schemas/json/cryproj.55.schema.json:2476:14
src/schemas/json/cryproj.55.schema.json:2677:14
src/schemas/json/cryproj.55.schema.json:2700:14
src/schemas/json/cryproj.55.schema.json:2775:14
src/schemas/json/cryproj.55.schema.json:2797:14
src/schemas/json/cryproj.55.schema.json:2861:14
src/schemas/json/cryproj.55.schema.json:2868:14
src/schemas/json/cryproj.55.schema.json:2875:14
src/schemas/json/cryproj.dev.schema.json:6:14
src/schemas/json/cryproj.dev.schema.json:2476:14
src/schemas/json/cryproj.dev.schema.json:2677:14
src/schemas/json/cryproj.dev.schema.json:2700:14
src/schemas/json/cryproj.dev.schema.json:2775:14
src/schemas/json/cryproj.dev.schema.json:2797:14
src/schemas/json/cryproj.dev.schema.json:2861:14
src/schemas/json/cryproj.dev.schema.json:2868:14
src/schemas/json/cryproj.dev.schema.json:2875:14
src/schemas/json/cryproj.json:6:14
src/schemas/json/cryproj.json:2476:14
src/schemas/json/cryproj.json:2677:14
src/schemas/json/cryproj.json:2700:14
src/schemas/json/cryproj.json:2775:14
src/schemas/json/cryproj.json:2797:14
src/schemas/json/cryproj.json:2867:14
src/schemas/json/cryproj.json:2874:14
src/schemas/json/cryproj.json:2881:14
src/schemas/json/dependabot-2.0.json:7:14
src/schemas/json/jekyll.json:7:14
src/schemas/json/jekyll.json:12:14

REFERENCES TO NON FRAGMENT IDs

src/schemas/json/base-04.json:37:19
src/schemas/json/base-04.json:58:19
src/schemas/json/base-04.json:817:19
src/schemas/json/base.json:38:19
src/schemas/json/base.json:59:19
src/schemas/json/base.json:820:19

We can see that in practice nothing in the schema store seems to be relying on these ID definitions (the only exception being the internal references inside base.json and base-04.json.

Although there is the possibility that external users are relying on these IDs, I suspect that they're unlikely to be, and if they are, then they should not be.

I suggest that we remove all such $id definitions and change the $ref references in base*.json to use regular fragment identifiers.

❌ Actual Behavior

Duplicate IDs will lead to inappropriate/undefined behavior if multiple such schemas are combined.

YAML or JSON file that does not work.

N/A

IDE or code editor.

None

Are you making a PR for this?

Yes, I will create a PR.

rogpeppe added a commit to rogpeppe-contrib/schemastore that referenced this issue Sep 13, 2024
This removes `$id` definitions from schemas where
those IDs are defining schemas outside of the schema URI itself.

The problem this is fixing is summarized in SchemaStore#4073.

Fixes SchemaStore#4073.
@rogpeppe rogpeppe linked a pull request Sep 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant