search

APIs-guru / openapi-directory

🌐 Wikipedia for Web APIs. Directory of REST API definitions in OpenAPI 2.0/3.x format
https://apis.guru/
Creative Commons Zero v1.0 Universal
3.89k stars 578 forks source link

Validation errors in various specs #540

Open pimterry opened 5 years ago

pimterry commented 5 years ago

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

Edit: should be fixed

Edit: should be fixed - Reported upstream some time ago

https://github.com/OpenBankingUK/opendata-api-spec-compiled/pull/1

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

MikeRalphson commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

seriousme commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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.

seriousme commented 3 years ago

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