fix: resolve pointers to subschemas in parameters

[Tony133]

Proposal:

When querystring, params or headers is a $ref pointing to a subschema of a shared schema:

fastify.addSchema({
  $id: 'shared',
  type: 'object',
  definitions: {
    genericQueryString: { type: 'object', properties: { order: { type: 'string' } } }
  }
})

fastify.get('/', { schema: { querystring: { $ref: 'shared#/definitions/genericQueryString' } } }, handler)

resolveLocalRef only used the schema name and ignored the rest of the pointer, so the whole shared schema was used instead of the referenced subschema. In Swagger 2.0 mode this throws Cannot create property 'in' on string 'object'.

Changes:

• resolveLocalRef follows the whole JSON pointer (with ~0/~1 unescaping), for both the #/definitions/... and #/components/schemas/... formats;
• Swagger 2.0 resolveCommonParams receives the raw shared schemas, as the OpenAPI one already did;
• the OpenAPI-only pointer walk in resolveCommonParams is removed, since it is now handled in one place for both modes.

Backward compatibility: when the pointer does not lead to a set of parameters (e.g. the existing support $ref schema test uses querystring: { $ref: 'subschema-two#/properties/hello' }, a string schema), the resolved definition is used as it has always been. Happy to drop this fallback if you prefer the stricter behavior.

Out of scope: the "PS" of #791, adding sibling properties next to $ref, is rejected by Fastify's own schema compiler (strict mode: unknown keyword). allOf + properties is the valid way to express it.

This does not overlap with #945 (different lines of the same function, verified on top of it).

Note:

• To be merged after the PR: fix: support $ref to definitions of shared schemas #952

Fixes #791

[Eomm]

TLDR: In OAS 2.0 the definitions map has no constraint on the key but from OAS 3.x, where components.schemas keys must match ^[a-zA-Z0-9\.\-_]+$.

So, this PR enable the swagger 2.0 user to use / and ~ as special chars in its schema definitions (while this is blocked on OAS 3)

---

Now, I think the codebase already does a decent job on splitting the logic between spec/openapi and spec/swagger, but lib/util/resolve-local-ref.js does not.

[Eomm]

The / should be invalid:
https://spec.openapis.org/oas/v3.1.1.html#fixed-fields-5

Note that definitions should be parameters but we convert it internally

[Tony133]

You're right, thanks. The / was only there to exercise the ~1 unescaping, it isn't needed to reproduce #791. Renamed the definition to genericQuery in both the openapi and swagger tests.

[Tony133]

Now, I think the codebase already does a decent job on splitting the logic between spec/openapi and spec/swagger, but lib/util/resolve-local-ref.js does not.

Agreed that resolve-local-ref.js shouldn't be the place that decides what's valid per spec. The ~0/~1 handling is plain RFC 6901 pointer decoding, so I saw it as spec-agnostic rather than an OAS 2.0 feature, but I see how it ends up enabling / and ~ in definition names for Swagger 2.0 users only.

With the tests now using a valid name, no test relies on the unescaping anymore. How would you prefer to handle it?

1. keep the pointer walk generic in resolve-local-ref.js and cover ~0/~1 only with a unit test in test/util.test.js;
2. move the unescaping into spec/swagger and keep spec/openapi strict;
3. drop the unescaping entirely and keep the PR to the Shared JSON schema: Cannot create property 'in' on string 'object' #791 fix.

I'd lean towards 3 to keep the scope small, happy to go either way.

Also, could you expand on "definitions should be parameters"? I want to make sure I'm not misreading the note. 😅