json-schema icon indicating copy to clipboard operation
json-schema copied to clipboard

Incorrect behavior of strict vs. default modes

Open pdlug opened this issue 10 years ago • 22 comments

The behavior of strict mode is a bit misleading (and incorrect IMHO). JSON schema provides required already but in the default execution of the json-schema validator it ignores this property. The expected behavior is that default validation checks that any keys present in the document being validated match the definition in the schema and that all keys listed as required in the schema are present. Strict mode would then make a bit more sense to change all keys to required though I'd argue that's a bit misleading as well and strict mode should really just imply that there are no keys present in the validation target that are not defined in the schema as well. Specifying that all keys present in the schema are required really should be a separate option since this is a different validation case (perhaps require_all: true?).

In summary I propose:

Default validation:

  • Invalid unless all keys in the input document match the definition in the schema.
  • Invalid unless all keys listed as required in the schema are present.
  • Keys present in the input document but not present in the schema are ignored (i.e don't affect validation).

Strict validation:

  • Default rules + invalid if any keys are present in the input document but not defined in the schema.

Require all:

  • Strict validation + invalid unless all keys defined in the schema are present in the input document.

I'm happy to take a stab at a patch if I can get some consensus that this is the right thing to do.

pdlug avatar Jul 11 '14 19:07 pdlug

It's unclear to me exactly why strict mode exists.

Since at least draft-02, there has been an additionalProperties schema property that:

  • if false, only properties explicitly defined in the schema are permitted
  • if an object, it's another schema, and all additional properties must conform it

As for required, this test seems to suggest it works without strict mode: https://github.com/hoxworth/json-schema/blob/756ec68/test/test_jsonschema_draft4.rb#L195-L220

(There's a similar test for draft3)

Given additionalProperties and required properties, I think you should be able to get everything you want without an additional mode -- even without strict mode!

pd avatar Jul 20 '14 13:07 pd

additionalProperties and required definitely allows one to write really strict schemas. The desire for the strict option, however, was initially to allow easy re-use of lenient pre-existing schemas in a strict way without requiring extending the schema. The side effect is that the library begins to intrude slightly on the domain of the schema language itself, possibly encouraging users to rely on validator features rather than schema language features.

I wouldn't mind adding a new require_all option as a complement to strict, but this issue brings up a good point that we should figure out a way to be slightly careful about how we present options that validate schemas in non-spec ways.

hoxworth avatar Jul 21 '14 16:07 hoxworth

I hope this pull request is useful. I implemented pdlug's suggestion of a require_all option. It defaults to true, so keeping the current strict behavior. I turned it to false in my project, and now only fields that are listed in required are actually required.

My pull request only implements this for the draft4 validator. I was not sure if it made sense to do it for the other validators.

ninkibah avatar Nov 26 '14 16:11 ninkibah

I'm going to throw in a crazy idea - maybe we should try to limit json-schema to validation, as it appears in the spec. Keep it lean and keep to the spec. It seems like inserting defaults, optional formatters, and even strict mode are all non-standard extensions and all have lead to headaches (in varying degrees). So far as I can tell, both require_all and strict mode basically override the way that json-schema validation works (ie. according to the spec). So far as I can tell, you can achieve all of this just by adding additionalProperties: false, and by doing that your schema will work identically in other validators as well.

I don't mean to oppose this feature in particular, but I feel like there's a broad pattern emerging between a number of features like this.

iainbeeston avatar Nov 26 '14 16:11 iainbeeston

Iain, do you mean to tell me that if I had just added additionalProperties: false to my schemas, that I could have validated my data without using strict mode?

But, yes, I would be for the default validation following the spec, without all these bells and whistles.

ninkibah avatar Nov 26 '14 16:11 ninkibah

So far as I can tell, strict mode is equivalent to setting additionalProperties: false on every property in your schema, and require all is equivalent to required: true and additionalProperties: false on every property in your schema (and my argument is that using required and additionalProperties is better because it works in every json schema validator, not just this one).

The way the json-schema gem works right now (without strict mode) is correct - the json schema spec only validates properties in the data that have the same key as properties defined in your schema (unless you use additionalProperties or required).


Correction: you specify additionalProperties on objects (not on individual properties)

iainbeeston avatar Nov 26 '14 16:11 iainbeeston

As I understood, require_all would only require each property in the schema to be present but not forbid additional ones (as strict mode would).

However, I'm very with @iainbeeston here. The problem is not that I am against fancy features, but first rule should be if json-schema is given a schema and some data it should behave according to the spec. For any additional things I'd propose two rules:

  • they have to be opt-in
  • having them in the lib must not complicate the structure of the validator too much (where the definition of what is too much is a consensus found here in discussions).

If we decide to add a require_all option then it should not be turned on by default in my opinion.

To repeat and emphasise my opinion: People that just want to do JSON-Schema validation should add json-schema as dependency throw in schemas and data and get what they would get in any other language/library adhering to the spec.

Actually this could affect $ref resolution/fetching (#152). If the spec says, validators should load refs by default, then json-schema maybe should do that by default and security bonuses are opt-in!?

RST-J avatar Nov 26 '14 18:11 RST-J

Yeah, if that's what's in the spec, then I think we should adhere to it... Even if we disagree (but in that case definitely have an opt-in flag)

iainbeeston avatar Nov 26 '14 19:11 iainbeeston

FWIW there's a thoughtbot post where they use strict: true so if people copy that matcher they may lose some time trying to figure out why for example the parser isn't respecting required lists, etc. I don't really get what strict is for either, it ends up just distorting the schema you've specified.

tristil avatar Jun 08 '15 22:06 tristil

Sorry for being late to the party but is strict option is still needed since behaviors are defined in schemas?

freiden avatar Jul 02 '15 09:07 freiden

@freiden What exactly do you mean? Strict is a makro so to say which prohibits any properties not covered by the schema (default in JSON schema is to accept any properties about which nothing is stated in a schema) and at the same time make every property required which is listed in a schema (properties not explicitly stated as required can be missing by default).

It is not necessary to have this as one can achieve this by hand. However, it was introduced as shortcut so one has not to do this tedious steps to convert relaxed schemas to strict ones, iirc.

RST-J avatar Jul 06 '15 08:07 RST-J

If I understand well the purpose is to validate schemas based on the specfication, so validations of fields should be managed by the schema to prevent unwanted behavior in context where you doesn't use this gem for validations. For example, I'm using the gem to validate my schema during tests but in my API I don't need the validation when sending my data the rest of the time.

freiden avatar Jul 10 '15 12:07 freiden

@iainbeeston correct me if I'm wrong, but as stated here under the base schema section, additionalProperties is specified at the object level, not for every property; it would be a nightmare otherwise. The same is stated in the Understanding JSON Schema "web book" (I know there's a better name for this...)

cec avatar Aug 13 '15 08:08 cec

@cec That's correct. If you're referring to my comment above - that's a typo (should have said for every object, or maybe "for every property")

iainbeeston avatar Aug 13 '15 09:08 iainbeeston

@iainbeeston I'd say for every object. It's a very nice property anyway :)

cec avatar Aug 13 '15 14:08 cec

@iainbeeston @RST-J I recently began a new project where an extensive use of combining schemas is required and this made me realise that @pdlug proposal is definitely necessary. Allow me to explain.

We use JSON Schema to describe responses of our APIs endpoints, similar to specifying a protocol. When doing so, it is of utmost importance to be able to programmatically ensure that API clients do not receive unexpected data.

"additionalProperties" : false guarantees that the protocol is not violated unintentionally when changed the API.

However, specifying this prevents from combining/extending schemas using allOf (see this issue on the json-schema repo).

The "ban unknown props mode" discussed on the referenced issue and in the proposal for draft 5 represents the change proposed by @pdlug and by @pkarman in #295 .

I think efforts should be made to implement and merge this because of the following reasons:

  • allowing props non mentioned in the scheme defeats one of the purposes of JSON Schema
  • preventing these props with additionalProperties:false would require every schema extension/combination to be a complete copy of the schema being extended, with the additional properties defined right after

I apologize for the long post and look forward to your comments.

cec avatar Feb 10 '16 17:02 cec

I was thinking of a slightly different idea ; When strict: true offer the possibility to the users to "override" the default behavior enforced by the strict mode

Example : with strict: true all property are by default "required": true ; but if the user set "optional": true or "required": false if override the behavior. The same if the user set "additionalProperties": true

I could try and dig in to the code to see if I can make a PR out of this idea

WDYT ?

Haelle avatar Nov 20 '19 08:11 Haelle

I need exactly the same behavior as you do @Haelle

I want strict validations because I want to validate that a json matches the schema and I don't trust the ones who are going to be adding fields to that JSON. So strict validation is great as it enforces everything to be defined in the schema

However, an optional attribute must be possible even in strict mode if defined in the schema. So if in the schema we have a required array that does not contain the attribute it should not be required even in strict mode.

optional: true does make sense but in non json schema standard.

required: true/false leads to problems as required should be a list of properties, not a boolean

AlexandreBernard avatar Oct 20 '22 15:10 AlexandreBernard

Hey people! Do i get it correctly that 10 year later there is sill no solution? Is there any objection against respecting "required": false in strict mode?

elik-ru avatar Jun 11 '24 09:06 elik-ru

There is was a PR that was doing exactly this, but it did not receive any comments: https://github.com/voxpupuli/json-schema/pull/406 and closed by author because of silence.

elik-ru avatar Jun 11 '24 18:06 elik-ru

I've been following this issue for far too long... Hope this gets fixed someday. As of now it is simply not possible to define optional properties when using the strict mode.

For a change, this is what I'd expect under strict mode:

  1. When the schema defines required, the validator should respect it.
// the validator should respect the "required" definition and allow "last_name" to be optional
{
  "type": "object",
  "properties": {
    "name": { "type": "string" },
    "last_name": { "type": "string" }
  },
  "required": ["name"]
}

// the validator should respect the "required" definition and allow any defined property to be optional:
{
  "type": "object",
  "properties": {
    "name": { "type": "string" },
    "last_name": { "type": "string" }
  },
  "required": []
}
  1. When the schema does not define required, the validator should treat all defined properties as required.
// the validator should treat all defined properties as required
{
  "type": "object",
  "properties": {
    "name": { "type": "string" },
    "last_name": { "type": "string" }
  }
}

Drowze avatar Jul 24 '24 12:07 Drowze

I encountered a problem this week. It was using RSwag / RSpec to test API responses against out swagger schema. We use strict mode, and this is handy I hadn't written the schema properly but I knew the response was correct. I worked my way through every error to get the swagger schema I wanted.

Next step, we use openapi-typescript to generate Typescript from our swagger.json. Problem is in JSON-Schema we use strict and there is no equivalent in openapi-typescript and every single property is marked as optional by default. So then I have to go in and marked every property as required anyway just so the Typescript is correct. All of this because as I understand it the standard does not support a strict mode. So really is the answer to this not to have a strict mode at all and adhere to the JSON Schema standard and be done with it.

In order to be happy that the schema I am testing in RSwag is going to eventually generate the Typescript we need then we should write our specs without strict mode.

Just my two cents after two days looking into this (not 10 years 😸 )

braindeaf avatar Aug 04 '24 00:08 braindeaf