Invalid $ref and $schema version usage, leading to checks being noops across a number of schemas #3102
Labels
enhancement:build-server
Propose something new for the SchemaStore web site or build server. (auto-generated by issue forms)
issue:bug
Report errors or unexpected behavior (auto-generated by issue forms)
Comments
ChALkeR
added
the
issue:bug
This was referenced Jul 27, 2023
Merged
ChALkeR
changed the title
|
Thanks for the report - yeah I was only aware that combining To fix this sooner rather than later, I would like to add a Grunt task with some code they checks this stuff. Later, it might be worth creating (or finding) a larger tool to handle issues like this more generally |
|
@hyperupcall not specifying This was changed only in The correct fix is to either specify As for the test, you can use this: const { validator } = require('@exodus/schemasafe')
const fs = require('fs')
const path = require('path')
const dir = 'schemas/json'
const files = fs.readdirSync(dir).sort().map(x => path.join(dir, x))
const schemas = files.map(x => [x, JSON.parse(fs.readFileSync(x, 'utf-8'))])
for (const [name, schema] of schemas) {
try {
// validator(schema, { requireValidation: true })
validator(schema)
} catch (e) {
// if (e.message.startsWith('failed to resolve $ref:')) continue
console.log(name, ': ', e.message)
}
}
|
|
Strong mode schema checks find a number of different mistakes across this database btw. Non-anchored regexes that should likely be anchored: Empty/ineffective regexes: Regexes that should be Not sure, but |
|
I'm impressed by 1) how easy it is to find mistakes with this tool; and 2) how many mistakes are out there for so much time without even being noticed until now. |
|
@madskristensen maybe it's a good idea to have a "schemasafe" check for pull requests on this repo, github actions could be used for that. |
|
That would be super helpful |
|
I added a separate const { lint } = require('@exodus/schemasafe')
const fs = require('fs')
const path = require('path')
const dir = 'schemas/json'
const files = fs.readdirSync(dir).sort().map(x => path.join(dir, x))
const schemas = files.map(x => [x, JSON.parse(fs.readFileSync(x, 'utf-8'))])
for (const [name, schema] of schemas) {
const errors = lint(schema) // lint(schema, { mode: 'strong' })
for (const e of errors) {
console.log(`${name}: ${e.message}`)
}
}Exact messages and their format are experimental, but now it doesn't stop on the first error per schema and instead collects all of them. |
|
Thanks for mentioning I made a PR that adds integration of schemasfe, but it's behind a Indeed, a integrating this into the GitHub Actions would be very useful. In fact I'm looking at the Actions, and I think I can improve it more generally, with exposing the various Grunt tasks and making it clear what is being checked. But before we can add this to the GitHub actions checks, I think it would be good to either fix or hide the current lint errors |
|
So as I'm working on ripping out the tv4 validator (separate issue), I find through this issue that we have previously implemented SchemaSafe checking. That was 2021, so it might possible that the repository was not as well maintained and had fewer checks. Or the then-quality of the code made it more difficult to add individual validators. In any case, thought it might be useful to mention. Very basic functionality of using SchemaSafe has already been merged, but for now it's undocumented because I don't want to encourage people using it until the reasons I mentioned earlier are resolved. #3105 and #3106 both contribute to making checking the schemas easier, and there is still more work to do. I will close this PR once all schemasafe issues have been resolved or hidden. |
|
@hyperupcall I wonder what were the exact issues you faced! |
hyperupcall
added
the
enhancement:build-server
|
Partial report from the lint mode: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Area with issue?
JSON Schema
✔️ Expected Behavior
See ExodusMovement/schemasafe#158 (comment)
In
draft-07and below (below2019-09),$refmakes all sibling keywords ignored: https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-8.3Upstream testsuite: https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/8cdfac41e37527795879e480a483997cbd6188f3/tests/draft7/ref.json#L145-L177
❌ Actual Behavior
A number of schemas here don't check what they are supposed to, e.g. the following checks are noops/ignored in all JSON Schema implementations that pass on upstream testsuite:
github-workflow.json:schemastore/src/schemas/json/github-workflow.json
Line 3 in ac57222
schemastore/src/schemas/json/github-workflow.json
Lines 1017 to 1031 in ac57222
propertiesis a noop/ignored, with all nested checks.prettierrc.json:schemastore/src/schemas/json/prettierrc.json
Line 2 in ac57222
schemastore/src/schemas/json/prettierrc.json
Lines 378 to 382 in ac57222
typeis a noop/ignored.stale.json:schemastore/src/schemas/json/stale.json
Lines 3 to 4 in ac57222
schemastore/src/schemas/json/stale.json
Lines 91 to 103 in ac57222
typeandpropertiesare noops/ignored, with all nested checks.A number of other schemas.
The ones above are just examples, this seems wide-spread.
Passing all schemas through schemasafe should highlight all issues — it refuses to compile such schemas in default configuration (unless schema mistakes are silenced).
YAML or JSON file that does not work.
No response
IDE or code editor.
None
Are you making a PR for this?
No, someone else must create the PR.
The text was updated successfully, but these errors were encountered: