application/vnd.api+json in yml files loses the + on resolve

[langalex]

Running my openapi.yml file though speccy resolve creates invalid files when keys include a + symbol.

Detailed description

My openapi.yml file includes keys that contain a + (file abbreviated for clarity):

get:
  responses:
    200:
      content:
        application/vnd.api+json:
          schema:
            $ref: xyz.json

When running this file through speccy resolve, the resolved file contains the original application/vnd.api+json plus another key on the same level where the plus has been replaced with a space (application/vnd.api json) and the $ref pointing (but incorrectly) to the newly inserted key.

get:
  responses:
    200:
      content:
        application/vnd.api+json:
          schema:
            $ref: "#/paths/get/responses/200/content/application~1vnd.api%2Bjson/schema"
        application/vnd.api json:
          schema:
            some: resolved data

The %2B in the $ref is actually a + again when running it through decodeURIComponent. It looks like somewhere these keys are www-form encoded back and forth and the created the wrong keys.

When removing the +, everythung works as expected and there is no more duplicate key.

I think this is a bug. Not sure if in speccy or one of the dependencies.

Your environment

• Node Version: 10.9.0
• Operating system and version (e.g. Ubuntu 16.04, Windows 7): OSX
• Speccy 0.8.6
• package-lock.json: https://gist.github.com/langalex/da5f0dd7bb9e119df0a8e92c9ae6122a

[MikeRalphson]

@langalex perhaps we can lose the package-lock.json dump for brevity's sake?

Is your response/content key either $refd externally, or the subject of a $ref elsewhere in your document?

[langalex]

@MikeRalphson sorry I thought the package-lock might be important. will do.

the $ref points to another file (a JSON-Schema file)

[MikeRalphson]

Ok, thanks, will try and reproduce.

[langalex]

wow, that was fast, thanks. let me know if I can help.

[dlouzan]

@langalex I find pretty amazing that we found the same issue with a difference of 40 minutes xD I'm copying what I wrote in #243 for reference:

Today I found I'm experiencing some issues when using response types that include + and {} characters, when resolving to external references.

Example:

  /foo:
    get:
      summary: Foo
      operationId: foo
      responses:
        200:
          $ref: "responses.yaml#/components/responses/Foo"

components:
  responses:
    Foo:
      description: Success, returning the root context and list of things
      content:
        application/ld+json:
          schema:
            $ref: "schemas.yaml#/components/schemas/Foo"

When I try to speccy resolve this generates an incorrect yaml file such as below with two equal keys, but one key missing the + (I don't think that's even valid yaml). Also JSON pointers don't seem to work properly when referencing anything including these special characters in the path.

      responses:
        '200':
          description: 'Success, returning the root context and list of things'
          content:
            application/ld+json:
              schema:
                $ref: '#/paths/~1foo/get/responses/200/content/application~1ld%2Bjson/schema'
                x-miro: 'schemas.yaml#/components/schemas/Foo'
            application/ld json:
              schema:
                title: Thing Description graph
                description: A Graph of Things in JSON-LD format.
                type: object
...

[MikeRalphson]

two equal keys, but one key missing the + (I don't think that's even valid yaml).

Just a note that if one key doesn't include the + then the keys aren't equal 😄 and a key with a space in it is valid YAML.

I'm adding some test cases over at oas-kit to narrow down where this issue is happening...

[dlouzan]

Well yes, I didn't mean that they were the same key, just that the linter was trying to create the same entry twice with different values ;-)

But I didn't know you could actually write a multi-word key without quoting, TIL, go figure xD

[dlouzan]

Another note: in my tests, the curly braces are also being url-encoded in the generated $refs:

• original request path:

  /.foos/{fooID}/.bars
    get:
      ...
  

• generated $ref:

  $ref: '#/paths/~1.foos~1%7BfooID%7D~1.bars/get/parameters/0'
  

The swagger documentation actually shows a sample with this and they don't encode them, though I'm not sure which is right:

$ref: '#/paths/~1blogs~1{blog_id}~1new~0posts'

[MikeRalphson]

Curly braces definitely do need encoding in url fragment ids / json pointers. We discussed it on an OpenAPI-Specification TSC call. I don't think we've updated the spec examples yet. The documentation on swagger.io is useful, but not authorative.

[MikeRalphson]

Fix and tests for the + problem have gone into Mermade/oas-kit@2f69f87 - had to move one whole bracket! Will be part of the v5.0.0 release of swagger2openapi which will hopefully be released this week.

[MikeRalphson]

oas-resolver v2.0.0 and reftools v1.0.5 have just been released including the fix for this issue.

[philsturgeon]

This will be fixed in v0.9.0 which will be out hopefully today.

[philsturgeon]

The fix is in #248 btw so if you can use docker or git to test that I'd appreciate it.

[philsturgeon]

To make testing easier, I have released v0.9.0-1:

npx speccy@0.9.0-1 lint foo.yaml

Please confirm if its working for you. If not, we need more tests :D

[dlouzan]

I have just tested master and got this, which is really not what i was expecting xD:

Document is not valid YAML (bad indentation?)

Just to be sure, I have just run the file through the python pyyaml module, and it didn't complain about the yaml syntax.

:-?

[pderaaij]

Can you share more details so we can look look into it?

[philsturgeon]

This is either an issue with pyaml, or an issue with js-yaml, so I'm not sure what we can do about it.

There is a suggestion to switch from js-yaml to yaml which could help us skate around the issue if it is indeed a bug in js-yaml.

[MikeRalphson]

@philsturgeon oas-kit has already moved from js-yaml to yaml, and that looks like an error from yaml. So it's those two projects who will have to fight it out.

[langalex]

works for me now 👍

[dlouzan]

@pderaaij I'm sorry i can't disclose the whole file, it's part of an internal project which I'm not at liberty to share :-/

I managed to solve the issue with heavy refactoring, at least now the content type with + characters is working properly. The project hadn't been really linted before, so there was a lot of missing references and invalid OAS 3 syntax that was (more or less) working with swagger-ui, but for which speccy reports (understandably and expected) an invalid spec.

What I can tell you is the behaviour I observed:

1. The develop branch speccy was reporting invalid yaml on a master OAS 3 file with multiple $refs to other files, some of which were invalid or non-existent. I'm sure the main file yaml had no syntax errors (I used another python validator)
2. I'm relatively sure that the issue was raised when speccy followed the invalid refs and then generated something internally invalid as yaml. As a user I would ideally expect it to try to validate the other ref'ed files and then tell me what was wrong in them.
3. After I solved the issues on the dependent files, now speccy happily reports the actual styling and spec problems

Let me know if I can help with anything else. Keep up the great work.

[MikeRalphson]

I would ideally expect it to try to validate the other ref'ed files and then tell me what was wrong in them.

Hmm, easier said than done, unless all your included files are structurally valid OAS 3 documents in and of themselves (i.e. include openapi, info.title, info.version and paths properties), though this is a personal recommendation of mine. It's not impossible to plug the validator recursively into the resolver (as a filter) so we could explore that option. Feel free to open a tracking issue over at oas-kit.

Keep up the great work.

Thanks for the kind words!

[MikeRalphson]

It might also be worth bumping up the use of the --verbose option to see if the resolver complains about a specific $refd file...

[dlouzan]

@MikeRalphson I actually reformated the included files to be valid OAS 3 documents, that was the way I found what the issues with them were :-)

It might also be worth bumping up the use of the --verbose option to see if the resolver complains about a specific $refd file...

Do you mean that you need to improve the verbose option, or that I should be able to see where the problem was with --verbose? I seem to recall I tried it with not much info available, but I can give it another try, checkout the version before the refactoring and verify.

[pderaaij]

Would be great if you can test that. My feeling is that speccy is not passing the verbose option correctly if I remember well from earlier tasks. If so I'll dive into that later this week.

[MikeRalphson]

I actually reformated the included files to be valid OAS 3 documents, that was the way I found what the issues with them were

OK, so it sounds like we should definitely make that process simpler.

And yes, it was using the --verbose option (specifically, more than once) I meant, but see @pderaaij's comment.

[dlouzan]

I have just done some tests:

Verbose output (a single --verbose, multiple ones don't seem to do anything) will at least point me in the direction of where the issue is, since when loading the $ref files, it will stop at the one causing the error.

The YAML error doesn't help much, because it gives back an escaped, single-line yaml string, and seems to point to the column where the error is, though I didn't have much luck finding exactly where the problem was in here (I even did some awk'ing to obtain the un-escaped version). The error reported is quite internal, such as:

     error: null,
     range: Range { start: 3613, end: 3622 },
     valueRange: Range { start: 3613, end: 3621 },
     props: [],
     type: 'MAP',
     value: null,
     items: [ [Object], [Object] ],
     resolved:
      YAMLMap {
        items: [Array],
        tag: 'tag:yaml.org,2002:map',
        range: [Array],
        type: 'MAP',
        spaceBefore: true } } }
Document is not valid YAML (bad indentation?)

What I have found is that the errors seem to be caused by indentation of comments, but only in some cases. Honestly, I don't know if this is yaml-compliant, but I had a couple of not indented at the same level as the parent element comments (typical IDE multiple lines comment). Other yaml parsers don't seem to have an issue with those, but the one used by speccy does.

This seems to only happen on $ref'ed files and only in some cases; if I change the comment indentation of any comment in the main file, everything is fine.

As an example, this works inside of a referenced file:

    ...
    # Something else

   # Indented not at the same level on top, works
    200About:
      description: Success, returning the about information

    # Something else
    ...

This doesn't:

     ...
    # Something else

    200About:
      description: Success, returning the about information
   # Indented not at the same level on the bottom, fails

    # Something else
...

[MikeRalphson]

Issue #243 would cover making Speccy use the same YAML parser for the initial load as oas-kit uses for resolving.

I'm working on a PR for the yaml module to improve error messages, e.g to include line and column positions, instead of character ones.

[pderaaij]

I've created #265 to ensure speccy respects the verbosity levels passed on via the CLI.