5e-srd-api icon indicating copy to clipboard operation
5e-srd-api copied to clipboard

Invalid refs in OpenAPI spec

Open pimterry opened this issue 1 year ago • 4 comments

Hi! Deep within the OpenAPI spec at https://www.dnd5eapi.co/swagger/openapi.json, there's a ref that looks like this:

"$ref": "#/paths/~1api~1monsters~1%7Bindex%7D/get/responses/200/content/application~1json/schema/allOf/3/properties/actions/items/properties/damage/items"

This is attempting to reference a value with paths -> /api/monsters/{index} -> (...) but escaping the special characters along the way.

Unfortunately the second part of that ref is not encoded correctly though. The ~1 is the correct encoding of the slashes in the string, but %7B and %7D should just be literal braces. You can see exactly the same case in the official docs here, where /blogs/{blog_id}/new~posts is referenced as #/paths/~1blogs~1{blog_id}~1new~0posts. The current format is referencing a string literally containing those %-escaped braces, not actual braces, and that doesn't exist.

I've taken a look around, but honestly I'm not sure where that comes from in here. Possibly it's already been fixed, but the published spec hasn't been updated, or it's an artifact or some generation & deployment steps somewhere? Happy to help investigate if you can give me any pointers where to look.

pimterry avatar Mar 09 '23 16:03 pimterry

Exact same issue here.

h2hjastermereel avatar May 23 '23 01:05 h2hjastermereel

Hi! Deep within the OpenAPI spec at https://www.dnd5eapi.co/swagger/openapi.json, there's a ref that looks like this:

"$ref": "#/paths/~1api~1monsters~1%7Bindex%7D/get/responses/200/content/application~1json/schema/allOf/3/properties/actions/items/properties/damage/items"

This is attempting to reference a value with paths -> /api/monsters/{index} -> (...) but escaping the special characters along the way.

Unfortunately the second part of that ref is not encoded correctly though. The ~1 is the correct encoding of the slashes in the string, but %7B and %7D should just be literal braces. You can see exactly the same case in the official docs here, where /blogs/{blog_id}/new~posts is referenced as #/paths/~1blogs~1{blog_id}~1new~0posts. The current format is referencing a string literally containing those %-escaped braces, not actual braces, and that doesn't exist.

I've taken a look around, but honestly I'm not sure where that comes from in here. Possibly it's already been fixed, but the published spec hasn't been updated, or it's an artifact or some generation & deployment steps somewhere? Happy to help investigate if you can give me any pointers where to look.

Do you know how to fix it? Visual Studio fails to build using the spec because of all three of these types of lines.

h2hjastermereel avatar May 23 '23 23:05 h2hjastermereel

After some investigation, it looks like the issue isn't to do with the fact that the curly braces are escaped.

In fact, if you load openapi.yml into something like https://editor.swagger.io/, the error is "$refs must reference a valid location in the document"; however, if we swap out the escape sequences %7A and %7D for their corresponding curly braces, a new error arises: "$ref values must be RFC3986-compliant percent-encoded URIs".

This points to the fact that the escape sequences are not the problem, but the actual refs are - they're not pointing to a valid location.

Interestingly, these refs are the only ones beginning with #/paths/... rather than #/components/schemas/.... I'm not sure why '#/monster-usage' and #/monster-damage in monsters.yml both get compiled into these broken refs to #/paths/..., rather than a ref to #/components/schemas/... or simply by being embedded like the other internal refs like #/monster-armor-class are.

Anyway. Noticing this, I tried manually modifying openapi.yml, replacing "#/paths/~1api~1monsters~1%7Aindex%7D/get/responses/200/content/application~1json/schema" with "#/components/schemas/Monster" in the three faulty refs. This appears to clear up the errors.

Unfortunately, I have no idea how to make swagger-cli use the correct refs. Like I mentioned, other internal refs in monsters.yml are handled correctly, and I can't identify a difference between these two and the ones that work fine.

Inlining #/monster-usage and #/monster-damage clears up the problem, but leads to repetition of these embedded schemas, which isn't ideal.

@ecshreve I wonder if you have any insight, as our resident swagger buff and original author of the API's swagger spec?

fergcb avatar May 24 '23 20:05 fergcb

this should be fixed, probably good to close out this issue unless y'all are still seeing this problem

ecshreve avatar Jul 21 '23 22:07 ecshreve