karenetheridge / OpenAPI-Modern

Validate HTTP requests and responses against an OpenAPI v3.1 document
https://metacpan.org/release/OpenAPI-Modern/
Other
6 stars 3 forks source link

Handle references to path-items (3.1.1, 3.2) #71

Open karenetheridge opened 8 months ago

karenetheridge commented 8 months ago

Because /paths might contain references to path-items (this is allowed by the spec but not by the 3.1.0 schema, but may be fixed in 3.1.1), meaning the path-item object itself is not always at /paths/$path_template and the operation may not be at /paths/$path_template/$method:

karenetheridge commented 8 months ago

partial duplicate of #53

karenetheridge commented 7 months ago

relevant changes to the spec and spec schema:

karenetheridge commented 7 months ago

When traversing the document:

When evaluating at runtime:

Maybe for 3.1.x we can just opt to not support $refs inside path-items at all (which is now allowed via https://github.com/OAI/OpenAPI-Specification/pull/3355) - to avoid having to implement all the above logic. In 3.2.0 the logic is changing (https://github.com/OAI/OpenAPI-Specification/pull/2657) -- I think it should change to a path-item entry either being a normal definition, OR just a $ref (but not a combination of both), so we can treat ref resolution here in the normal way when in find_path - when we find a matching path, resolve the entire $ref chain down to the end to find the final path-item object, where we'll find both the path-item-level parameters and the operation-level parameters, all together (because we don't use $refs at the operation level (yet!!)).

karenetheridge commented 7 months ago

For 3.1.1, we can check for and disallow $refs, even in 3.1.0, because the published schema allows for path-item-or-reference (albeit in a convoluted way that allows stacking, which is the bit we don't want to support).

For 3.2 we need to be serious about proper support, which also means we can't assume that path_item_path is under /paths, and also $state->{initial_schema_uri} may change after calling find_path (the path-item could be in another document), which means we need to pass the state object around a bit more. Note that any subsequent $ref keywords must be resolved against the new document, not the original.

We also aren't doing as many cross-checks between values in the options hash as we should be.

karenetheridge commented 7 months ago

If operation_id is provided to find_path, and the operation lives somewhere else and would be reached by a $ref, the calculated path_template is incorrect. We don't even need to use a $ref to /components/pathItems:

paths:
  /hello:
    $ref: '#/paths/~1goodbye'  
  /goodbye:
    get:
      operationId: greeting

If provided a request like GET /hello and operationId "greeting", we will use the operation_path of /paths/~1goodbye/get and then unjsonp it to ('', 'paths', '/goodbye', 'get') which will give us the correct path_item_path (after following $refs from /paths/~1hello) but the path_template for this, /goodbye, is not correct for this request.

If the path-item lives in /components, the unjsonp operation will fail entirely. Also if the path-item lives in another document, the current mechanism for lookup up operation_paths will fail (see #73).

This also now means it would be no longer possible to call find_path with just operation_path and path_captures -- we cannot infer the path_template from operation_path as the operation might be shared between multiple paths. However, validate_request will always have the request available, so we can perform the path_template and path-item lookup from that, and validate_response doesn't care about anything in the path-item level at all: it doesn't use path_captures and only needs the operation-level data. Therefore, we should rework find_path to not error if it fails to calculate the path-item path or is not provided with the path_captures, and let validate_request bomb out instead. Or, alternatively, we should split up the functionality of find_path into separate methods, to allow performing only the core needs for validate_response.

karenetheridge commented 7 months ago

splitting up find_path:

for validate_response:

for validate_request:

karenetheridge commented 5 months ago

Note that the semantics for $ref in path-item will be changing from what we initially thought -- https://github.com/OAI/OpenAPI-Specification/pull/3860/files

The published schema for 3.1.0 will be changing to allow sibling $ref, with all the complications of multiple overlapping definitions; we may want to simply not support this, or disallow adding multiple operation and parameter definitions at each sub-level of $ref (and only respect the definitions in the final object).

karenetheridge commented 1 month ago

see also https://github.com/OAI/OpenAPI-Specification/issues/3734 - this won't be changing in 3.1.1 nor 3.2.