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

Validation errors in various specs #540

Open
pimterry opened this issue Feb 12, 2019 · 15 comments
Open

Validation errors in various specs #540

pimterry opened this issue Feb 12, 2019 · 15 comments

Comments

@pimterry
Copy link
Contributor

pimterry commented Feb 12, 2019

I'm using https://github.com/Mermade/oas-kit/blob/master/packages/swagger2openapi/README.md to batch convert specs to OpenAPI v3. In doing so, I'm seeing quite a few specs with errors.

It's possible that these are an issue in that tool, but having checked a few cases they do appear to be legitimate errors to me. For example, many of the Azure specs reference files like apimusers.json, which don't exist anywhere in this repo, and quite a few specs do use collectionFormat with non-array parameters, which is indeed invalid.

Here's the list:

Edit: should be fixed

  • archive.org/search/1.0.0/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • archive.org/wayback/1.0.0/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • traccar.org/4.3/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • googleapis.com/bigquerydatatransfer/v1/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • simplyrets.com/1.0.0/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • googleapis.com/cloudiot/v1/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • netlicensing.io/2.x/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • zalando.com/v1.0/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • azure.com/applicationinsights-swagger/2018-04-20/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array
  • azure.com/cognitiveservices-Face/1.0/swagger.yaml: (Patchable) collectionFormat is only applicable to param.type array

Edit: should be fixed

  • geneea.com/1.0/swagger.yaml: Could not resolve reference #/definitions/Information%20about%20a%20user%20account.
  • clarify.io/1.3.7/swagger.yaml: Could not resolve reference #/definitions/Ref%20(of%20Bundle)
  • taggun.io/1.2.90/swagger.yaml: Could not resolve reference #/definitions/Model%201
  • semantria.com/4.0/swagger.yaml: Could not resolve reference #/definitions/Request%20class
  • clever-cloud.com/1.0.0/swagger.yaml: Could not resolve reference #/definitions/Transaction%20Id
  • je-apis.com/2.0.0.0/swagger.yaml: Could not resolve reference #/definitions/JE.Api.Payments.Contracts.StatusCode%60%5B%5BJE.Api.Payments.Contracts.CheckingOut.AuthorisationReasonTypes,%20JE.Api.Payments.Contracts,%20Version=6.16.0.0,%20Culture=neutral,%20PublicKeyToken=null%5D%5D
  • epa.gov/case/1.0.0/swagger.yaml: (Patchable) parameter.type is mandatory for non-body parameters

Edit: should be fixed - Reported upstream some time ago

OpenBankingUK/opendata-api-spec-compiled#1

  • openbanking.org.uk/v1.3/swagger.yaml: (Patchable) "Status Code" is not a valid header

Wider Azure problem - probably missing / mis-referenced files in source repo

  • azure.com/apimanagement-apimissues/2018-01-01/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/IssueContract
  • azure.com/apimanagement-apimissues/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/IssueContract
  • azure.com/azsadmin-RegionHealth/2016-05-01/swagger.yaml: Could not resolve reference #/definitions/Alert
  • azure.com/apimanagement-apimgroups/2016-10-10/swagger.yaml: Could not resolve reference ./apimusers.json#/definitions/UserContract
  • azure.com/apimanagement-apimusers/2016-10-10/swagger.yaml: Could not resolve reference ./apimgroups.json#/definitions/GroupContract
  • azure.com/apimanagement-apimusers/2017-03-01/swagger.yaml: Could not resolve reference ./apimgroups.json#/definitions/GroupContract
  • azure.com/apimanagement-apimdiagnostics/2018-01-01/swagger.yaml: Could not resolve reference ./apimloggers.json#/definitions/LoggerContract
  • azure.com/apimanagement-apimdiagnostics/2017-03-01/swagger.yaml: Could not resolve reference ./apimloggers.json#/definitions/LoggerContract
  • azure.com/apimanagement-apimapis/2016-10-10/swagger.yaml: Could not resolve reference ./apimproducts.json#/definitions/ProductContract
  • azure.com/apimanagement-apimgroups/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimusers.json#/definitions/UserContract
  • azure.com/apimanagement-apimgroups/2017-03-01/swagger.yaml: Could not resolve reference ./apimusers.json#/definitions/UserContract
  • azure.com/apimanagement-apimgroups/2018-01-01/swagger.yaml: Could not resolve reference ./apimusers.json#/definitions/UserContract
  • azure.com/apimanagement-apimproducts/2016-10-10/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/ApiContract
  • azure.com/apimanagement-apimusers/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimgroups.json#/definitions/GroupContract
  • azure.com/apimanagement-apimusers/2018-01-01/swagger.yaml: Could not resolve reference ./apimgroups.json#/definitions/GroupContract
  • azure.com/apimanagement-apimapis/2017-03-01/swagger.yaml: Could not resolve reference ./apimproducts.json#/definitions/ProductContract
  • azure.com/apimanagement-apimproducts/2017-03-01/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/ApiContract
  • azure.com/apimanagement-apimtags/2018-01-01/swagger.yaml: Could not resolve reference ./apimtagresources.json#/definitions/TagResourceContract
  • azure.com/apimanagement-apimtags/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimtagresources.json#/definitions/TagResourceContract
  • azure.com/apimanagement-apimtags/2017-03-01/swagger.yaml: Could not resolve reference ./apimtagresources.json#/definitions/TagResourceContract
  • azure.com/apimanagement-apimproducts/2018-01-01/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/ApiContract
  • azure.com/apimanagement-apimproducts/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimapis.json#/definitions/ApiContract
  • azure.com/apimanagement-apimapis/2018-01-01/swagger.yaml: Could not resolve reference ./apimdiagnostics.json#/definitions/DiagnosticContract
  • azure.com/apimanagement-apimapis/2018-06-01-preview/swagger.yaml: Could not resolve reference ./apimdiagnostics.json#/definitions/DiagnosticContract
@MikeRalphson
Copy link
Collaborator

Yes, there are some errors the validator used historically in this project does not spot.

Unfortunately it will be quite a lot of work to move away from it.

@pimterry
Copy link
Contributor Author

Ok, that makes sense, thanks. It doesn't block me at all, so I'm not too worried, I mainly just filed this for reference later.

Even if the current validator doesn't validate this automatically, is there a reason these can't be fixed up by hand? I'm going to be doing a bunch of openapi work at the moment, so I'd be happy to chip in.

@MikeRalphson
Copy link
Collaborator

I think we can fix up the collectionFormat issues in the scripts, the missing referenced files are to be honest, probably too much work to tackle by hand. If the Azure ones can be demonstrated to be errors at the source, I'm sure someone at Microsoft would be keen to get them fixed.

@MikeRalphson
Copy link
Collaborator

MikeRalphson commented Feb 13, 2019

If you're planning on integrating with the directory, please feel free to PR against the README whenever is appropriate. Just FYI, the definitions which originate in non-OpenAPI 2.0 format (primarily Amazon AWS and Google Cloud Services) will be updated to OpenAPI 3.0 by May,

@MikeRalphson
Copy link
Collaborator

Update: we now no longer use the more lax OpenAPI v2.0 validator. All issues except azure.com ones should now be fixed. The service values for these need to be aligned with upstream first before we can sort out the mess of unresolvable references.

@seriousme
Copy link

seriousme commented Jun 25, 2021

Hi,

I have been testing the ApiGuru dataset today using openapi-schema-validator and out of 2278 unique items (taking only the latest versions) it resulted in 7 failures:

    "name": "ato.gov.au",
    "name": "bluemix.net:containers",
    "name": "britbox.co.uk",
    "name": "keycloak.local",
    "name": "nasa.gov:apod",
    "name": "openapi-generator.tech",
    "name": "zoom.us",

A full description of the errors can be found at: https://github.com/seriousme/openapi-schema-validator/blob/master/test/realworld/failed.json
The script to perform the test can be found at https://github.com/seriousme/openapi-schema-validator/blob/master/test/realworld/realworld.js

openapi-schema-validator uses the OpenApi.org json schemas and AJV to validate specs.

Can you confirm that these errors are errors in the API's and not in the schemas/validation ?

Kind regards,
Hans
<edited after updating openapi-schema-validator to use draft-04 instead of draft-07, going from 18 failures to 7>

@seriousme
Copy link

seriousme commented Jun 26, 2021

I did some more analysis:

"name": "ato.gov.au",

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/ato.gov.au/0.0.6/swagger.yaml#L1004-L1006

        "400":
          $ref: "#/responses/FailedPrecondition"
          description: Individual has related resources and cannot be deleted

A response needs to contain either a $ref or a description, not both

name": "bluemix.net:containers"

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/bluemix.net/containers/3.0.0/swagger.yaml#L117-L124

        - description: Must be the content of a tar archive compressed with gzip. The
            archive must include a file called 'Dockerfile' at its root. It may
            include any number of other files which will be accessible in the
            build context.
          in: body
          name: file
          required: true
          type: file

OpenApi 3.x requires file uploads to be a requestBody type, in: body is not allowed

"name": "britbox.co.uk",

 "/itv/subscription/status/{platform}":
    get:
      "400":
        description: Bad request.
        schema:
          $ref: "#/components/schemas/ServiceError"
      "404":
        description: Not found.
        schema:
          $ref: "#/components/schemas/ServiceError"
      "415":
        description: Unsupported Media Type
        schema:
          $ref: "#/components/schemas/ServiceError"
      "500":
        description: Internal server error.
        schema:
          $ref: "#/components/schemas/ServiceError"
      default:
        description: Service error.
        schema:
          $ref: "#/components/schemas/ServiceError"
      description: Returns status of latest payment intent.
      operationId: getSubscriptionStatus
      parameters:

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/britbox.co.uk/3.730.281-ref-1-39-0/openapi.yaml#L6668-L6691
This looks like the responses are located directly beneath the get which is not allowed according to spec.

"name": "keycloak.local",

  securitySchemes:
    access_token:
      bearerFormat: null
      scheme: bearer
      type: http

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/keycloak.local/1/openapi.yaml#L8642-L8646
The scheme does not match the specification: https://spec.openapis.org/oas/v3.0.0#security-scheme-object

"name": "nasa.gov:apod",

  /apod:
    get:
      descriptions: Returns the picture of the day
      parameters:

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/nasa.gov/apod/1.0.0/openapi.yaml#L35-L38
Seems like a typo, descriptions needs to be description

"name": "openapi-generator.tech",

          schema:
            additionalProperties:
              $ref: "#/definitions/CliOption"
              originalRef: CliOption
            type: object

https://github.com/APIs-guru/openapi-directory/blob/main/APIs/openapi-generator.tech/5.1.1/swagger.yaml#L59-L63
additionalProperties must be boolean.

"name": "zoom.us",

CloudArchivedFiles:
      additionalProperties: false
      properties:
        archive_files:
          additionalItems: true
          description: An explanation about the purpose of this instance.
          items:
...

This spec is 5.1Mb so GitHub won't link to linenumbers, however there are some issues with /components/schemas/CloudArchivedFiles/

Hope this helps.

Kind regards,
Hans
<edited after updating openapi-schema-validator to use draft-04 instead of draft-07, going from 18 failures to 7>

@MikeRalphson
Copy link
Collaborator

@seriousme thank you for the report and analysis. When you are processing files named swagger.yaml remember these will be OpenAPI 2.0 and will need converting before you validate them with OpenAPI 3.x tooling. I will look at your other issues and report back.

@MikeRalphson MikeRalphson self-assigned this Jun 30, 2021
@MikeRalphson
Copy link
Collaborator

A response needs to contain either a $ref or a description, not both

According to the 3.0 spec, additional properties in a reference object are ignored (not disallowed).

@MikeRalphson
Copy link
Collaborator

additionalProperties must be boolean.

According to the v3.0 spec:

additionalProperties - Value can be boolean or object. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.

MikeRalphson added a commit that referenced this issue Jun 30, 2021
@MikeRalphson
Copy link
Collaborator

@seriousme britbox, keycloak and nasa/apod definitions fixed. Many thanks.

@seriousme
Copy link

@seriousme thank you for the report and analysis.

You're welcome !

When you are processing files named swagger.yaml remember these will be OpenAPI 2.0 and will need converting before you validate them with OpenAPI 3.x tooling. I will look at your other issues and report back.

My tooling does not look at filenames but checks for the contents the toplevel "swagger" or "openapi" property in the spec, so the error is probably right, but my interpretation of the error was wrong ;-)

additionalProperties - Value can be boolean or object. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema.

Yikes, not a smart move of the spec makers to call it the same but spec it differently. I checked the 3.0.0 schema and it does cover this difference in schema. So there must be some other issue. I will try to do some more digging on this one ;-)

But there is good news: it is fixed in 3.1.0 ;-)
Unless stated otherwise, the property definitions follow those of JSON Schema and do not add any additional semantics. Where JSON Schema indicates that behavior is defined by the application (e.g. for annotations), OAS also defers the definition of semantics to the application consuming the OpenAPI document.

To summarize, I still need to figure out why:

    "name": "ato.gov.au",
    "name": "bluemix.net:containers",
    "name": "openapi-generator.tech",
    "name": "zoom.us",

do not match the official OpenApi JSON schema definitions.
Thanks for fixing the other 3 !

Kind regards,
Hans

@seriousme
Copy link

seriousme commented Jul 2, 2021

Did some more digging as promised :-)

ato.gov.au

I loaded the ato.gov.au up in swagger editor and it also breaks at the additional attributes next to the $ref
image

The text of the spec says indeed:

This object cannot be extended with additional properties and any properties added SHALL be ignored.

https://spec.openapis.org/oas/v3.0.0#reference-object
However the official JSON schema says that only properties named "$ref" are allowed:

  "definitions": {
    "Reference": {
      "type": "object",
      "required": [
        "$ref"
      ],
      "patternProperties": {
        "^\\$ref$": {
          "type": "string",
          "format": "uri-reference"
        }
      }

https://github.com/seriousme/openapi-schema-validator/blob/master/schemas/v3.0/schema.json#L53-L64
So even while the written spec goes before the schema, the various tools rely on the schema and break on anything that fails the schema. So my suggestion would be to ask the author to comply to the schema ;-)
Alternatively one could ask the OAS team to fix the openAPI 3.0 JSON schema to allow for additional attributes.

openapi-generator.tech

Same explanation as for the ato.gov.au spec goes for the openapi-generator.tech schema

bluemix.net:containers

Loading the bluemix.net:containers into swagger editor gives a better explanation of what is wrong:

Semantic error at paths./build.post
Operations with parameters of "type: file" must include "multipart/form-data" in their "consumes" property
Jump to line 47
Structural error at paths./build.post.parameters.6
should NOT have additional properties
additionalProperty: type
Jump to line 85
Structural error at paths./build.post.parameters.6
should have required property 'schema'
missingProperty: schema
Jump to line 85
Semantic error at paths./build.post.parameters.6
Parameters with "type: file" must have "in: formData"
Jump to line 85

This matches the Swagger 2.0 spec section 6.4.9.1 Fixed Fields

If type is "file", the consumes MUST be either "multipart/form-data", " application/x-www-form-urlencoded" or both and the parameter MUST be in "formData".

and

schema Schema Object Required. The schema defining the type used for the body parameter.

and the type parameter only present under If in is any value other than "body":

zoom.us

Again the loading the spec up into swagger editor gives a better explanation of what is wrong:
(takes some time though, had to allow the browser to continue script processing several times ;-))

Semantic error at paths./chat/users/{userId}/messages/{messageId}/status
Declared path parameter "userId" needs to be defined as a path parameter at either the path or operation level
Jump to line 14491
Semantic error at paths./chat/users/{userId}/messages/{messageId}/status
Declared path parameter "messageId" needs to be defined as a path parameter at either the path or operation level
Jump to line 14491
Semantic error at paths./im/chat/messages/{message_id}.delete.requestBody
DELETE operations cannot have a requestBody.
Jump to line 20471
Semantic error at paths./past_meetings/{meetingId}/archive_files
Declared path parameter "meetingId" needs to be defined as a path parameter at either the path or operation level
Jump to line 37315
Semantic error at paths./phone/sms/sessions/{sessionId}
Declared path parameter "sessionId" needs to be defined as a path parameter at either the path or operation level
Jump to line 48699
Semantic error at paths./phone/sms/sessions/{sessionId}/messages/{messageId}
Declared path parameter "sessionId" needs to be defined as a path parameter at either the path or operation level
Jump to line 48700
Semantic error at paths./phone/sms/sessions/{sessionId}/messages/{messageId}
Declared path parameter "messageId" needs to be defined as a path parameter at either the path or operation level
Jump to line 48700
Semantic error at paths./phone/users/{userId}/sms/sessions
Declared path parameter "userId" needs to be defined as a path parameter at either the path or operation level
Jump to line 50595
Structural error at components.schemas.CloudArchivedFiles.properties.archive_files
should NOT have additional properties
additionalProperty: additionalItems
Jump to line 82195

The schema only breaks at the Structural error at components.schemas.CloudArchivedFiles.properties.archive_files
additionalItems is not allowed in an OAS 3.0 Schema object which by now we now is not the same as a JSON schema ;-)

Hope this helps !

Hans
ps. I have setup a weekly github workflow to check the whole set of specs against the respective official schemas

@MikeRalphson
Copy link
Collaborator

ato.gov.au the schema definition for the Reference object appears to leave additionalProperties as the default true so it does allow additional properties other than $ref. Because of the "SHALL be ignored" in the spec. I don't consider this a validation error.

openapi-generator.tech ditto

bluemix.net:containers I have set this definition to auto-upgrade to OpenAPI 3.0.0 which should fix this error (please wait for CI run to complete).

zoom.us

DELETE operations cannot have a requestBody.

This also is not an error, in v3.0.0 requestBodies on methods which have undefined behaviour are ignored.

Thanks for spotting the additionalItems issue, this stray JSON Schema keyword has been removed from the list of valid keywords in v3.0.0 in my validator.

Path /chat/users/{userId}/messages/{messageId}/status is empty and in this circumstance, the in:path variables do not need to be declared. This ambiguity was clarified, but only in v3.1.0 of the spec. - I took the view that because the behaviour was not defined for v3.0.x then this was not a validation error.

MikeRalphson added a commit that referenced this issue Jul 2, 2021
@seriousme
Copy link

Thanks for the quick reply ! (and your patience ;-))

I did a test and you are completely right on the additionalProperties (not that I doubted that ;-)).

If I supply AJV draft 4 (the schema validator I use) with the schema:

{
    "$schema": "http://json-schema.org/draft-04/schema#",
    "type": "object",
    "required": [
        "$ref"
    ],
    "patternProperties": {
        "^\\$ref$": {
            "type": "string",
            "format": "uri-reference"
        }
    }
}

Then:

{
    "$ref": "#/ref",
    "description": "my description",
    "originalRef": "original"
}

Is valid !

So the combination of patternProperties and additionalProperties works as designed.
As the full schema validation failed it must be something different that explains the errors.

I think I solved the mystery:
Both the ato.gov.au and the openapi-generator.tech are V2 specs and not V3 specs !
So to explain the errors we need to look at the V2 JSON schema.

The v2.0 specification says:

6.4.17 Reference Object §

A simple object to allow referencing other definitions in the specification. It can be used to reference parameters and responses that are defined at the top level for reuse.

The Reference Object is a JSON Reference that uses a JSON Pointer as its value. For this specification, only canonical dereferencing is supported.

6.4.17.1 Fixed Fields §
Field Name Type Description
$ref string Required. The reference string.

https://spec.openapis.org/oas/v2.0#reference-object
No mentioning of "SHALL be ignored" here ;-)

And the V2.0 JSON schema is also quite strict on this one:

 "jsonReference": {
      "type": "object",
      "required": [
        "$ref"
      ],
      "additionalProperties": false,
      "properties": {
        "$ref": {
          "type": "string"
        }
      }
    }

Kind regards,
Hans

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

No branches or pull requests

3 participants