search

APIDevTools / swagger-parser

Swagger 2.0 and OpenAPI 3.0 parser/validator
https://apitools.dev/swagger-parser
MIT License
1.1k stars 155 forks source link

Bundle of remote schema fails when schema includes relative reference #82

Open TomCools opened 6 years ago

TomCools commented 6 years ago

I have discovered an issue when a remote schema (ex. http://schema.com/user) contains another reference to a relative schema on that site (ref: "/address");

Using the resolver, i get a $refs object which contains following keys:

However, when trying to bundle(), I get the message that: "all pointers must begin with #". I have made a workaround in my code using .resolve() and then swapping out the relative path by checking all the keys in the $refs object, but I expect bundle() to just work out of the box.

fyi, this relative path thing works fine on the official Swagger Editor website.

JamesMessinger commented 6 years ago

Without seeing your actual schema, I can't say for sure what's causing the error. But it sounds like you have an invalid JSON Pointer in there somewhere.

The resolve() method only parses JSON References, not JSON Pointers, which is why you don't get an error when calling resolve(). But the bundle() method does parse JSON Pointers so it can resolve their values, and it throws this error when it encounters an invalid pointer.

TomCools commented 6 years ago

Hi, so we found the issue.

There was a schema/fizz#buzz listed in the schema instead of schema/fizz#/buzz, meaning that spec wize, it is indeed incorrect.

However, I would like to note that the same Swagger file was handled without error by the official Swagger Editor, which we ask our teams to use for editing. We use your library to create a custom view (forcing us to parse the Swagger). It is very weird to have the standard Editor allow something which is not correct in your library. I see two ways to look at this (both valid btw)

  1. The online Swagger Editor is more resilient to errors, and we can help users by making this library just as resilient for this kind of mistakes. (Action: Adjust this library to be resilient)

  2. This error is only valid because you do something which is not in the Specification. It is not because the Swagger Editor supports this mistake that this library should. (Action: Do nothing)

I will be watching this issue to see what you decided, but I hope you at least consider both of the options above.

JamesMessinger commented 6 years ago

Swagger Parser actually uses json-schema-ref-parser under the hood, which is a spec-compliant implementation of JSON Reference and JSON Pointer. I don't want to introduce non-spec-compliant behavior into that library (which is used for more than just Swagger-related stuff) just for the sake of conformity with a Swagger tool.

I suggest opening an issue on the Swagger Editor repo asking them to at least show a warning message for non-spec-compliant references.

All that said... I do think that I should improve the error message that is thrown in this case. Since it's an easy mistake to make, and a hard one to notice, it would be beneficial for me to point it out more clearly.