Fix $ref issue with special characters (#183)

[jy95]

Attempt to fix #183 .
In my opinion ( as some of the tests uses circular structure ) , it is better to give choice to final user to use dereference / bundle if they got a similar situation like mine.

For example :

new OpenApiValidator({
    unsafeRefs: true
}).install(app)

[cdimascio]

would it be possible to use dereference instead of bundle, yet use lodash.clonedeep to copy the spec? it would be great if we can remove the need for an option

[jy95]

Give me your full code proposal and I will test on my project to tell you if it works.
If not, I am afraid it is the only solution : bundle works great on detecting circular refs ( like some of your tests ) but doesn't work with escaped paths ( as explained in the $ref parser documentation, dereference doesn't have protection against circular dependacies so it will probably fail your tests with circular $refs )...

EDIT : I added a test case in order to reproduce the bug.

[cdimascio]

the doc in dereference mentions

JSON Schema files can contain circular $ref pointers, and JSON Schema $Ref Parser fully supports them. Circular references can be resolved and dereferenced just like any other reference. However, if you intend to serialize the dereferenced schema as JSON, then you should be aware that JSON.stringify does not support circular references by default, so you will need to use a custom replacer function.

You can disable circular references by setting the dereference.circular option to false. Then, if a circular reference is found, a ReferenceError will be thrown.

Or you can choose to just ignore circular references altogether by setting the dereference.circular option to "ignore". In this case, all non-circular references will still be dereferenced as normal, but any circular references will remain in the schema.

Another option is to use the bundle method rather than the dereference method. Bundling does not result in circular references, because it simply converts external $ref pointers to internal ones.

The exprss-openapi-api validator code does end up serializing the apiDoc with JSON.stringify(). The copy method on line 26 does this. I wonder if things will work with dereference if we modify the way that copy function works. For example, we could use lodash.deepCopy or JSON.stringify with a replacer function.

If we're able to use dereference for all cases, we can avoid the need for an option. It seems to make dereference work we need to avoid serializing apiDoc since it may have circular refs

[jy95]

It is worth a try : if this passes my example of specification above then we can consider the problem solved.

[cdimascio]

🤞 let me know how it turns out

[jy95]

You can fork this branch and try (as I added a test : it is more easily to check) : I am not completly sure how you would like to change the code to handle this situation

[cdimascio]

deepclone doesn't help, unfortunately. appears we indeed require an option

perhaps instead of $unsafeRefs(since dereference doesn't necessarily mean refs are unsafe), we provide an option to use bundle or dereference. its a bit of an implementation detail that ends up getting exposed, but perhaps we can remove this option at some point in the future (if json-ref-parser provides an alternative)

Perhaps something like:

  $refParser: {
     mode: 'bundle' | 'dereference
  }

where $refParser.mode = 'bundle' is the default when not present

thoughts?

[jy95]

LGTM : Feel free to rename it as you wish

[cdimascio]

will look at this shortly and get this in

[jy95]

@cdimascio I add a test to reproduce the bug - You can try what you think in order to solve the bug.
( If your solution didn't work, you can squash commits of this PR before merging )

[jy95]

@cdimascio Sorry for the many commits : my linter rules aren't the same that Codacy ( you can squash them before merging )