additionalProperties with Boolean value causes NPEs #1318
Comments
|
Here is a small test case: {
"swagger": "2.0",
"info": {
"version": "0.0.1",
"title": "test of Swagger with boolean additionalProperties in schema definition"
},
"paths": {
"/": {
"get": {
"produces": [ "application/json" ],
"description": "xxx",
"responses": {
"200": {
"schema": { "$ref": "#/definitions/Root" }
}
}
}
}
},
"definitions": {
"Root": {
"type": "object",
"properties": {
"foo": {
"type": "string"
}
},
"additionalProperties": false
}
}
} |
|
Related: swagger-api/swagger-core#1437 |
|
@ches The definition |
|
@dolmen I'm aware that Swagger adds a slight semantic override, but in regard to
My added emphasis is to stress that I'm pretty sure there is a conscious decision not to break backward compatibility implied there. |
|
@ches There is no "backward compatibility" involved here. This is just about maintaining the similar meaning, for consistency (not compatibility) with JSON Schema. |
|
@dolmen Is that an "official" stance on JSON Schema compatibility? Either way, I hope we can agree that the written spec and the implementation are currently inconsistent. |
|
@ches there is an official stance on json-schema. Swagger supports a subset of JSON schema draft-4 which should be clearly documented in the swagger-spec. The spec is the source of truth and projects may not implement all aspects of the spec 100%. If we have unimplemented features, we like to get community confirmation and feedback on the importance of the feature |
|
Thanks @fehguy. I take
and my other above quotation to mean that for the subset that the Schema Object encompasses, the types should (eventually) be completely conformant to JSON Schema. But I misread @dolmen's link above when I said that there is an inconsistency—the Swagger schema does include |
|
Sorry for taking a while to reply. So we definitely need to make this clearer in the spec (and please feel free to open a ticket on it once we clarify it). The spec itself is a bit incomplete with regard to how Unlike what's stated, Given that, you should not require having to set |
|
@webron Thanks for the clarification. The JSON schema for Swagger must be updated to match your definition to disallow About the codegen issue, note that codegen does not accept either an empty object ( Here is a test case where html generation fails: {
"swagger": "2.0",
"info": {
"version": "0.0.1",
"title": "test of Swagger of boolean additionalProperties"
},
"paths": {
"/": {
"get": {
"produces": [ "application/json" ],
"description": "xxx",
"responses": {
"200": {
"schema": { "$ref": "#/definitions/Root" }
}
}
}
}
},
"definitions": {
"Root": {
"type": "object",
"properties": {
"foo": {
"type": "string"
}
},
"additionalProperties": {}
}
}
} |
Following clarification of 'additionalProperties' for a schema by @webron: swagger-api/swagger-codegen#1318 (comment) We now accept only `false` or a schema for `additionalProperties`. `false` is the new default for `additionalProperties` (instead of `{}` which means "anything is accepted" in JSON Schema).
|
I wrote:
I just submitted OAI/OpenAPI-Specification#477 |
Following clarification of 'additionalProperties' for a schema by @webron: swagger-api/swagger-codegen#1318 (comment) We now longer accept boolean for 'additionalProperties': additionalProperties can only be disallowed (the default, when additionalProperties is not set) or of a fixed model (the schema object given as the value).
I'll come back and provide a more minimal reproduction case, just wanted to file this while it's fresh (it seems fairly trivial to reproduce, and a quick grep shows a lot of code that seems to assume
additionalPropertieswill be a map).additionalPropertiesin JSON Schema can be an object or a Boolean. When I use a Boolean value, codegen blows up with an NPE (using rev 253c46a at time of this trace):The offending spec definition in this case is this one:
This API conforms to JSON API and derives definitions from the JSON Schema definition of JSON API, which declares
additionalProperties: falsehere. The$refhere is inconsequential—commenting out theadditionalPropertiesline (and others like it in the rest of the spec) avoids the codegen failure and succeeds.The text was updated successfully, but these errors were encountered: