JSON parseable verification

[mlarcher]

It seems that swagger-tools validate will not check if the JSON is valid before checking if it matches the expected schema (or that there is an error when handling this check).
As a result, errors like trailing commas are not seen, but trigger an error later on when trying to parse the JSON.
Would it be possible to add this check as a pre-requisite before any other verification ?

[whitlockjc]

swagger-tools doesn't parse JSON, except when retrieving remote references, and all of its APIs take already created/parsed objects. As for when JSON parsing does occur, I'm not sure how we could attempt to validate JSON that didn't pass parsing. Do you have an example that can reproduce this?

[mlarcher]

Here is a malformed JSON that will pass swagger-tools validation even though it can't be parsed:

{
  "swagger": "2.0",
  "info": {
    "title": "User API",
    "description": "my API",
    "version": "1.0.0"
  },
  "host": "api.local.dev",
  "basePath": "/",
  "produces": [
    "application/json"
  ],
  "paths": {
    "/user": {
      "get": {
        "description": "get user",
        "parameters": [
          {
            "name": "param1",
            "in": "query",
            "type": "string",
            "required": true,
            "description": "some desc"
          },
        ],
        "responses": {
          "200": {
            "description": "result",
            "schema": {
              "type": "string"
            }
          }
        }
      }
    }
  }
}

Note the extra comma after the parameter

[whitlockjc]

So, here is the problem. We're using a YAML parser to parse the inputs, whether JSON or YAML. The reason for this is the YAML parser we use is capable of parsing both JSON and YAML. It seems that the JSON above is invalid if using JSON.parse but if I use js-yaml (YAML.safeLoad(fs.readFileSync('swagger.json', 'utf8')), it does not fail.

I'm not sure if this is a bug or a feature but I'll see if js-yaml has a way to fail or report on such things.

[whitlockjc]

It looks like I've brought this up in the past: nodeca/js-yaml#262 It seems that the only way to solve this is to have a fall-through mode where instead of using the YAML parser for everything, I try the JSON parser first and then fall through to the YAML parser.

Or...we leave it as-is.

[mlarcher]

According to js-yaml documentation, safeLoad as an option for that : json (default: false) - compatibility with JSON.parse behaviour.
cf https://github.com/nodeca/js-yaml#safeload-string---options-
Second part of the description is not so clear, though, so it should be tested.

If there is no other option, I think swagger-tools validate should indeed try the JSON parser first (if file is .json?). It's a bit odd that a validator tells us the swagger.json is fine when it has no chance of working for parsing error reasons, and feels like a validator reliability issue imho.

[whitlockjc]

I tried using it, false is the default and when I change it to true, YAML.safeLoad still parses the JSON without issue. As documented, that option is really for duplicate key handling.

As for this JSON not working, how do you mean? Can you give me a real error you're encountering? json-refs is used to load the Swagger documents so once it's loaded, and we've verified we're loading it, the fact that the JSON was not valid shouldn't matter anymore. (This isn't to say we shouldn't fix this to work around js-yaml but I'm trying to make sure I understand the scope.)

[mlarcher]

Thanks for checking out the json option.

The error case we have is when trying to load an invalid json file in a swagger instance.
We get an error saying 500 : { "error": { "message": "SyntaxError" } } http://api.temp-swagger-test.domain.com/apis.json when the json file is not parseable.

Perhaps that has something to do with the fact that the json file is on another domain...

[whitlockjc]

Swagger instance? You mean swagger-ui? Yeah, it doesn't use a YAML parser for all of its parsing. but in swagger-tools, we're serving that file so the generated JSON should be valid. Are you serving the api.json via swagger-tools or via some static file server?

That alone is reason enough to fix this I guess. The reason I took this route is because there is no 100% guaranteed way to check if an input is JSON or YAML and extension matching is not a guarantee. I will update json-refs to fix this and will link this issue in the report.

[mlarcher]

Right, I meant swagger-ui. We are serving the apis.json file via a static file server.
Thanks for your feedback.
Any idea if this can be fixed rapidly or if it'll take some time before someone can get to it ?

[whitlockjc]

I'll get it out ASAP but if it's a show stopper, just fix your JSON. ;) That requires no changes elsewhere. Now that I've got the bug in json-refs, it won't slip through the cracks.