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

Schema does not enforce requirement of array items object #174

Closed
whitlockjc opened this issue Oct 23, 2014 · 15 comments
Closed

Schema does not enforce requirement of array items object #174

whitlockjc opened this issue Oct 23, 2014 · 15 comments
Labels

Comments

@whitlockjc
Copy link
Member

In the 1.2 and 2.0 specifications, it says this regarding the items property: Required if type is "array". The problem is I can create a property like this without the JSON schema validator throwing an error:

"ids": {
  "type": "array"
}
@webron
Copy link
Member

webron commented Oct 23, 2014

That's correct. I'm not sure if it would be a good idea to fix it in the schema.

The JSON Schema of JSON Schema (yes, that's confusing) is permissive on such things even though the schema being validated may not be meaningful in terms of the supposed restrictions.
I'm not sure if want to go out of our ways to add such restrictions in the schema ourselves because we would end up having to add far more restrictions than just the one you mentioned here.

@whitlockjc
Copy link
Member Author

Having a lax schema kind of defeats the point of a schema at all. Not a big deal, I can work around it but I was surprised when this came up.

@webron
Copy link
Member

webron commented Oct 23, 2014

Yes, which is why I'm surprised at JSON Schema's schema as well.

As you know, there are a few validations that can't be covered by the schema anyways and have to be manually checked for.

But yeah, I admit I'd prefer seeing as much restrictions in the schema as possible. I'll try mapping these things in the next few days and we'll see how we can improve it.

Feel free to add here comments on similar issues you may encounter (I'll give you a hint, a "$ref" property can't exist alongside other properties).

@whitlockjc
Copy link
Member Author

Well, this wasn't that big a deal in 1.2 as there are only a handful of places where type was used. For 2.0, you can put schemas almost anywhere so having to check for this manually can be quite overwhelming.

@webron
Copy link
Member

webron commented Oct 23, 2014

I don't disagree which is why I'm glad you brought it up. Let's try to make
it better. It just won't be easy.
On Oct 24, 2014 12:47 AM, "Jeremy Whitlock" [email protected]
wrote:

Well, this wasn't that big a deal in 1.2 as there are only a handful of
places where type was used. For 2.0, you can put schemas almost anywhere
so having to check for this manually can be quite overwhelming.


Reply to this email directly or view it on GitHub
#174 (comment)
.

@whitlockjc
Copy link
Member Author

You got it. Any chance we can do like you do for body parameters and enforce the requirement?

@whitlockjc
Copy link
Member Author

This is pseudo:

"schema": {
  "oneOf": [
    {
      "$ref": "#/definitions/arraySchema"
    },
    {
      "$ref": "#/definitions/nonArraySchema"
    }
  ]
}

whitlockjc added a commit to apigee-127/swagger-tools that referenced this issue Oct 23, 2014
* This should eventually be handld by schema validation:
    OAI/OpenAPI-Specification#174

Fixes #62
@webron
Copy link
Member

webron commented Oct 24, 2014

It actually doesn't end there. Other validation properties may also be type-specific. For example, min/maxLength are only relevant to strings, multipleOf is only relevant to numbers and so on.

I think the main idea is to mention in the schema what's require but not necessarily what's valid. For example, if you combine minLength with a numeric type, the effect is that the minLength is just ignored.
However, I think that supporting tools should give a warning to the user that this combination exists just to make sure they are aware that it doesn't make sense.

This is different than the case than the one you originally mentioned where "items" is mandatory with type array.

In any case, I'd like to spend some time going over possible use cases and decide which ones should be added.

@whitlockjc
Copy link
Member Author

I wonder if we should treat a missing items the way JSON Schema handles missing additionalProperties and default to {} which means it matches anything. I think from a consistency perspective, this would make sense and it's pretty simple to support from a tooling perspective.

The only thing driving this right now is an inconsistency between the specification documentation and the JSON Schemas. What if we were to just remove this requirement from the Swagger specification documents and support it as discussed above?

@webron
Copy link
Member

webron commented Jun 15, 2015

Wish we could, but really, at this point it seems a bit late for it. I'll do my best trying to enforce it in the schema, though if I recall correctly, it was hell (and possibly impossible).

@whitlockjc
Copy link
Member Author

Well, it's moot now. If it's here, I'll continue doing what I'm doing. I'm just saying the lack of items is handled inconsistently when compared with other places where JSON Schema has optional things, like additionalProperties.

@webron
Copy link
Member

webron commented Jun 15, 2015

There are a few inconsistencies in the spec :-/ Just need to try and improve in the next version.

@handrews
Copy link
Member

Not sure if this is still an active concern. if anyone is still curious as to why JSON Schema works this way, it has to do with the difference between assertions, annotations, and data model keywords (which is discussed more clearly in the soon-to-be-published draft-07).

items is a data model keyword. It applies a subschema to part of an instance based on the data model (in particular, if the instance is an array, it applies the subschema to ever element of an array and ANDs the results- contrast with contains which applies the subschema and ORs the results) Note the "if it's an array, then apply the subschema" approach, with the implicit "else don't do anything for this keyword" clause.

type is an assertion. It evaluates a constraint against the instance and produces a boolean result.

The third category, annotations, include keywords like title and default.

Data model keywords themselves never cause validation to fail. They apply subschemas which have assertions, and it is the assertions in the subschemas which cause the pass or fail.

This seems weird for use cases like OpenAPI. But consider using JSON Schema to scan arbitrary data for objectionable language with a regex. You don't know or care what data types you'll get. Objects, arrays, strings, numbers, booleans, nulls, nested structures of arbitrary complexity, etc. You just want to apply your regex using pattern to every string in the document.

If item behaved as an assertion, this would be very hard. You could do it with anyOf but it would be very complicated. Preserving the data model vs assertion split makes it easy to do this sort of "validation" with JSON Schema.

FWIW, JSON Schema does in fact treat a lack of items the same as "items": {}, which only works if items is not an assertion of array-ness.

@handrews
Copy link
Member

@webron I think this is more or less a dup of #1891 although there is more information here. Fairly certain we don't need to keep both of them open.

@handrews
Copy link
Member

We decided not to fix this in the other issue since the schema is not intended to catch all errors, and this is against old OAS versions that aren't being worked on anymore anyway. I think this is safe to close.

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

No branches or pull requests

3 participants