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

addSchema adds schema even if invalid but stops iterating over array #1434

Open
davedodo opened this issue Feb 6, 2021 · 4 comments
Open

Comments

@davedodo
Copy link

davedodo commented Feb 6, 2021

I just upgraded from Ajv 6.12.2 to 7.0.4. I noticed that the behavior of addSchema as changed. When passing an invalid schema it is still added. In version 6, it was not added. Additionally, if you pass an array, the iteration still stops after the invalid schema just like it did in version 6. I'm not sure if this is intended behavior. It seems to me that the policy should be to either add all the schemas regardless of their validity, add only the valid schemas, or stop after the first invalid schema without adding it.

Here is a minimal example that illustrates the issue:

const Ajv = require("ajv").default;
const ajv = new Ajv();

try {
	ajv.addSchema([
		{$id: "foo", type: "number"},
		{$id: "bar", type: "corge"},
		{$id: "baz", type: "string"},
	]);
} catch (error) {
	// Will throw because "bar" is invalid.
	console.log(error.message);
}

// Will be true.
console.log(ajv.getSchema("foo") !== undefined);

try {
	// Will throw because "bar" was added but is invalid.
	console.log(ajv.getSchema("bar"));
} catch (error) {
	console.log(error.message);
}

// Will be false because iteration stopped after adding "bar".
console.log(ajv.getSchema("baz") !== undefined);

On my system (Node.js 10 on Ubuntu 18.04) this outputs:

schema is invalid: data/type should be equal to one of the allowed values, data/type should be array, data/type should match some schema in anyOf
true
type must be JSONType or JSONType[]: corge
false

I also think it should be documented how addSchema handles invalid schemas, regardless of whether this is a bug or intended behavior.

@epoberezkin
Copy link
Member

epoberezkin commented Feb 7, 2021

It was not exactly specified how Ajv should behave in this situation... The reason for adding the schema before validating it was to simplify the code and to still support the scenario when the schema is its own meta schema (as is the case for specification meta-schemas - you need to add it before you can validate it against itself).

Possibly the solution is to remove it in case schema validation throws exception.

In general, the recovery from this situation was not considered - for most users it would indicate programming error that needs to be fixed (rather than something that needs to be handled at run time).

It is different in case the schemas are dynamic of course - I need to think about it.

A possible workaround is to validate schema with validateSchema method before adding. It's worth noting that even if schema passes [syntactic] validation, it can still fail compiling for various reasons that validateSchema would not catch:

  • undefined keywords/formats
  • strict mode violations
  • missing references
  • unbounded reference recursion (in which case it would overflow the stack)

@manchuck
Copy link

I'm not sure if I should open a new issue but what I'm seeing is related. I am using ajv.validateSchema as part of our build process to ensure that a developer does not commit a bad schema before committing. Sometimes a bad schema gets past the PR process and causes all types of havoc. When I use validateSchema with all strict options turned on, a bad schema I know will validate true, however, when that schema is complied, a validator error is thrown.

Here is an example:

const Ajv = require('ajv');
const ajv = new Ajv({
    strict: true,
    strictSchema: true,
    strictNumbers: true,
    strictTypes: true,
    strictTuples: true,
    strictRequired: true,
    validateFormats: true,
});

const badSchema = {
    "$schema": "http://json-schema.org/draft-07/schema#",
    "$id": "#my-schema",
    "required": [
        "foo"
    ],
    "properties": {
        "fizz": {
            "type": "string"
        }
    }
};


console.log('Is Valid?', ajv.validateSchema(badSchema));
(async() => {
    try {
        const compiled = await ajv.compile(badSchema)
    } catch (error) {
        console.error(error);
    }
})()

The output is

Is Valid? true
Error: strict mode: missing type "object" for keyword "required" at "#my-schema#" (strictTypes)

@epoberezkin
Copy link
Member

@manchuck validateSchema only checks “syntax” (it validated schema as JSON against metaschema), it may still be semantically incorrect.

To make sure it compiles, you just need to compile it as part of the build/test, no need to use validateSchema separately in this case (it will be called inside compile).

@manchuck
Copy link

@epoberezkin I wound up doing that. I compiled the schemas together as part of the validation

const Ajv2019 = require('ajv/dist/2019');
const baseSchema = require('my-schema.json');
const validSchema = new Ajv2019({
    strict: true,
    strictSchema: true,
    strictNumbers: true,
    strictTypes: true,
    strictTuples: true,
    strictRequired: true,
    validateFormats: true,
    allowUnionTypes: true,
    allErrors: true,
    loadSchema: (url) => require(url),
}).compileAsync(baseSchema)
.then(() => true)
.catch((error) => {
    console.error(error)
    return false;
});

Thanks for the advice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants