Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ambiguity between Reference Object and PathItem Object #2635

Closed
char0n opened this issue Jun 29, 2021 · 16 comments · Fixed by #3355
Closed

Ambiguity between Reference Object and PathItem Object #2635

char0n opened this issue Jun 29, 2021 · 16 comments · Fixed by #3355
Labels
Milestone

Comments

@char0n
Copy link
Contributor

char0n commented Jun 29, 2021

Hi everybody, not sure if this is the right place to ask this, but I've intercepted an ambiguity while building tooling for OAS 3.1

{
  "openapi": "3.1.0",
  "components": {
    "pathItems": {
      "pathItem1": {
        "$ref": "#/components/pathItems/pathItem2",
      },
      "pathItem2": {
        "summary": "path item summary",
        "description": "path item description",
        "get": {}
      }
    }
  }
}

Given following document, pathItem1 can be interpreted either as Reference Object or Path Item Object. It's hard to tell what the author intended. He could have intended for pathItem1 to be a Reference Object. Or he may have intended pathItem1 to be a Path Item Object. As Path Item Object can have exactly the same shape as Reference Object the intention of the author is lost.

Can you please advice how to approach that?

Instead of having this ambiguity and $ref field in Path Item Object, wouldn't it be cleaner to use Reference Object explicitly in every place where Path Item Object is allowed?

Current workaround could be assert the presence of $ref field and one of the other fixed fields from Path Item Object (like get, post, etc...). If assertion like that passes, we can say with limited certainty that author intended to define a Path Item Object.

@jdesrosiers
Copy link
Contributor

I didn't notice this before. The schema doesn't validate this properly. It only allows for a Path Item Object (with no Reference Object properties) OR a Reference Object. A Path Item Object with Reference Object properties would fail validation.

What is the reason for including Reference Object properties in Path Item Object as opposed to just using a Reference Object? It's the only type that does that. It also doesn't seem very well specified. It doesn't specify how a reference works in this context, but implies that there differences from a normal Reference Object. A Reference Object has rules for merging "summary" and "description", but other properties are not merged in the referenced object. Path Item Object implies that some merging takes place, but doesn't specify how (e.g. deep or shallow merge). Is it assumed that Reference Object behaviors apply here unless otherwise specified? Do the merging rules for "summary" and "description" in a Reference Object apply?

As a workaround, I think you can always treat a $ref as part of the Path Item Object and never a Reference Object and always get what the user intended. If a Path Item Object only has Reference Object properties, it would evaluate the same as if it were a Reference Object. The only difference in processing it as a Reference Object is that any Path Item Object properties will be ignored. Since the user wouldn't include Path Item properties if they expected those properties to be ignored, we can be confident that if those properties appear, we should process it as a Path Item Object. I think that in all cases, processing it as a Path Item Object should result in the desired behavior.

@webron
Copy link
Member

webron commented Jul 1, 2021

@jdesrosiers it's an old remnant of version 2.0, where we allowed the reference without knowing back then that it cannot live alongside other properties. So back then (and now), it allows siblings and does not ignore them.
Granted, with 3.1, we could have used the current Reference Object and allow for siblings, but we didn't. It's one of the few places in the spec where the $ref is not JSON Schema Reference nor the Reference Object, but its own thing.

@char0n
Copy link
Contributor Author

char0n commented Jul 2, 2021

The schema doesn't validate this properly. It only allows for a Path Item Object (with no Reference Object properties) OR a Reference Object. A Path Item Object with Reference Object properties would fail validation.

So this technically means that if Object looks like a Reference Object, it will always be considered a Reference Object.

What is the reason for including Reference Object properties in Path Item Object as opposed to just using a Reference Object?

As @webron mentioned it's a remnant from older specifications, where components/pathItems fields did not exist and Paths Object only allows Path Item Object as a value of it's pattern fields. So including $ref field in Path Item Object was the only way to reference another Path Item Object from Paths Object.

It also doesn't seem very well specified. It doesn't specify how a reference works in this context, but implies that there differences from a normal Reference Object. A Reference Object has rules for merging "summary" and "description", but other properties are not merged in the referenced object. Path Item Object implies that some merging takes place, but doesn't specify how (e.g. deep or shallow merge)

As the merging rules as undefined by the specification, tooling authors have to come up with something that makes sense. I've chosen the same merging rules as Reference Object and Schema Object follows. Fields from referencing object will shallowly override fields from referenced object.

Is it assumed that Reference Object behaviors apply here unless otherwise specified? Do the merging rules for "summary" and "description" in a Reference Object apply?

Acoording to this: Allows for a referenced definition of this path item. The referenced structure MUST be in the form of a Path Item Object. In case a Path Item Object field appears both in the defined object and the referenced object, the behavior is undefined. See the rules for resolving Relative References. the Reference Oject behaviors don't apply and the behavior is undefined.

As a workaround, I think you can always treat a $ref as part of the Path Item Object and never a Reference Object and always get what the user intended.

This would contradict the current schema validation. IMHO better way and possibly more forward compatible one would be to stay with the current validation and consider everything that looks like a Reference Object a Reference Object.

If a Path Item Object only has Reference Object properties, it would evaluate the same as if it were a Reference Object. The only difference in processing it as a Reference Object is that any Path Item Object properties will be ignored. Since the user wouldn't include Path Item properties if they expected those properties to be ignored, we can be confident that if those properties appear, we should process it as a Path Item Object. I think that in all cases, processing it as a Path Item Object should result in the desired behavior.

So if I understand this correctly, whenever there there is a choice between Reference Object | Path Item Object and the Object defines a $ref field, we should always consider it a Path Item Object. This will have an implication on fields merging as Path Item Object doesn't define it and every tooling author can choose how to define the merging. It is also not aligned with current schema validation.

I still think that this is problematic and opens door for unnecessary interpretations. In order to reduce the complexity and eliminate the ambiguity I propose following:

  • remove $ref field from Path Item Object
  • define a Type for Paths Object field pattern as this: Path Item Object | Reference Object

This small change will make things right in backward compatible manner.

@jdesrosiers
Copy link
Contributor

So this technically means that if an Object looks like a Reference Object, it will always be considered a Reference Object.

Yes. If it has a $ref, the schema will consider it a Reference Object which means that it will be invalid if it contains any additional properties other than "summary" and "description". However, the schema is non-normative so it doesn't tell us how OpenAPI should behave. If the spec and the schema disagree, the schema is wrong.

I've chosen the same merging rules as Reference Object and Schema Object follows. Fields from referencing object will shallowly override fields from referenced object.

References in a Schema Object don't do any merging (not in any version of JSON Schema). Reference Object doesn't expressly specify a shallow merge, but I think the term "override" does imply a shallow merge.

whenever there there is a choice between Reference Object | Path Item Object and the Object defines a $ref field, we should always consider it a Path Item Object. This will have an implication on fields merging as Path Item Object doesn't define it and every tooling author can choose how to define the merging.

Right. This workaround only works if the tooling uses similar merging rules to Reference Object.

I still think that this is problematic and opens door for unnecessary interpretations.

I'm sure everyone can agree with that. I completely agree with your proposed changes.

@char0n
Copy link
Contributor Author

char0n commented Jul 5, 2021

References in a Schema Object don't do any merging (not in any version of JSON Schema). Reference Object doesn't expressly specify a shallow merge, but I think the term "override" does imply a shallow merge.

Right, there is only section of 7.7.1.1 which kind of defines what the behavior could be, and that behavior is aligned with OAS 3.1 Reference Object.

I'm sure everyone can agree with that. I completely agree with your proposed changes.

Should I push this forward by creating a PR with a proposed change?

Thank you for your time and all the answers!

@darrelmiller
Copy link
Member

From TDC call:

3.1.1 : 1) Remove the use of reference Object for WebHooks and Components/PathItems
2) Identify $ref on its own as a ReferenceObject
3) Make PathObjects array of PathItem | ReferenceObject
4) Deprecate the use of $ref alongside other operation fields?
3.2.: 1) Remove $ref from PathItem fixed fields?

Should we continue to support "merging" PathItems?

@MikeRalphson MikeRalphson added this to the v3.2.0 milestone Jul 9, 2021
@char0n
Copy link
Contributor Author

char0n commented Jul 9, 2021

@darrelmiller IMHO 3.1.1 - 1 and 3.1.1 - 3 contradict each other. If our aim is to remove $ref field field from Path Item Object in next breaking change release following steps seems like more appropriate:

3.1.1

The aim here is provide clarification to limit ambiguity

  1. Clarification - Change Type Reference Object | Path Item Object in all places within spec to Path Item Object
  2. Deprecate the use of $ref fixed field in Path Item Object

3.2.0

The aim here is to have only one referencing mechanism iwithing OAS 3.2 spec (reducing complexity) - Referece Object (JSON Schema spec has it's own $ref mechanism)

  1. Remove $ref from Path Item Object fixed fields
  2. Change Type Path Item Object in all places within spec to Reference Object | Path Item Object

@MikeRalphson
Copy link
Member

MikeRalphson commented Jul 9, 2021

I've scanned ~200,000 OpenAPI documents and apart from my test file, I've found precisely 3 occurrences of a pathItem object having a $ref and one or more operation objects (all OpenAPI 2.0).

All three files are exactly the same and are in test folders of the pyswagger/pyopenapi projects:

@char0n
Copy link
Contributor Author

char0n commented Jul 12, 2021

During the next week I should have some numbers from SwaggerHub as well.

char0n added a commit to swaggerexpert/OpenAPI-Specification that referenced this issue Jul 23, 2021
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
char0n added a commit to swaggerexpert/OpenAPI-Specification that referenced this issue Jul 23, 2021
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
char0n added a commit to swaggerexpert/OpenAPI-Specification that referenced this issue Jul 23, 2021
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
char0n added a commit to swaggerexpert/OpenAPI-Specification that referenced this issue Jul 23, 2021
 The  field has been deprecated in favor of
using Reference Object in OAS 3.2.0 and forward.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
char0n added a commit to swaggerexpert/OpenAPI-Specification that referenced this issue Jul 23, 2021
Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
@char0n
Copy link
Contributor Author

char0n commented Jul 23, 2021

As discussed and agreed on TDC, I've issued a 3 PRs that fix the ambiguity in progressive steps between 3.1.1 and 3.2.:

@karenetheridge
Copy link
Member

karenetheridge commented Nov 3, 2021

I didn't notice this before. The schema doesn't validate this properly. It only allows for a Path Item Object (with no Reference Object properties) OR a Reference Object. A Path Item Object with Reference Object properties would fail validation.

It's worse than that... the json schema does not allow for this document at all:

---
openapi: 3.1.0
info:
  title: ..
  version: ..
components:
  pathItems:
    /foo:
      ...
paths:
  /foo:
    $ref: '#/components/pathItems/~1foo'

Because the top level paths is defined as #/$defs/paths which is an object of path-items, not path-items-or-references. I think we could resolve all issues by letting /paths/<path> be an object of path-items-or-references, and amending the spec to match.

(which is exactly what @char0n proposed in #2635 (comment).)

webron pushed a commit that referenced this issue Mar 10, 2022
* Deprecate Path Item Object ref field (#2635)

 The  field has been deprecated in favor of
using Reference Object in OAS 3.2.0 and forward.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>

* Deprecate Path Item Object ref field specific usecases

Refs #2635

* fix change
@karenetheridge
Copy link
Member

Because the top level paths is defined as #/$defs/paths which is an object of path-items, not path-items-or-references. I think we could resolve all issues by letting /paths/ be an object of path-items-or-references, and amending the spec to match.

Would a PR to this effect be accepted? I am encountering tooling errors when trying to use a $ref for a path-item that exactly duplicates another path-item (except for a different path template), because the schema document does not accept this, and the tool uses schema validation to parse the document.

@gimbimloki
Copy link

gimbimloki commented Oct 22, 2022

Hello. When OAI 3.2.0 is released, could I use Reference Objects($ref) in Paths Object like below?

paths:
  $ref: foo.yaml
  $ref: bar.yaml

@MikeRalphson
Copy link
Member

@gimbimloki whenever 3.2.0 is released, any syntax change won't involve duplicate keys as per your example, which have undefined behaviour in both JSON and YAML.

darrelmiller pushed a commit that referenced this issue Mar 23, 2023
Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
webron added a commit that referenced this issue Mar 23, 2023
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Co-authored-by: Ron <webron@users.noreply.github.com>
charjr pushed a commit to charjr/OpenAPI-Specification that referenced this issue Apr 27, 2023
* Deprecate Path Item Object ref field (OAI#2635)

 The  field has been deprecated in favor of
using Reference Object in OAS 3.2.0 and forward.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>

* Deprecate Path Item Object ref field specific usecases

Refs OAI#2635

* fix change
charjr pushed a commit to charjr/OpenAPI-Specification that referenced this issue Apr 27, 2023
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Co-authored-by: Ron <webron@users.noreply.github.com>
charjr pushed a commit to charjr/OpenAPI-Specification that referenced this issue Apr 27, 2023
* Deprecate Path Item Object ref field (OAI#2635)

 The  field has been deprecated in favor of
using Reference Object in OAS 3.2.0 and forward.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>

* Deprecate Path Item Object ref field specific usecases

Refs OAI#2635

* fix change
charjr pushed a commit to charjr/OpenAPI-Specification that referenced this issue Apr 27, 2023
This change provides a clarification to limit ambiguity
between Path Item Object and Reference Object.

Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Co-authored-by: Ron <webron@users.noreply.github.com>
@karenetheridge
Copy link
Member

What's the status of this? It appears to have stalled just before execution of all the action items listed.

@jdesrosiers
Copy link
Contributor

Because the top level paths is defined as #/$defs/paths which is an object of path-items, not path-items-or-references. I think we could resolve all issues by letting /paths/<path> be an object of path-items-or-references, and amending the spec to match.

I think that change would cover any real use cases, but I don't think it completely brings the schema in line with the spec. That change would not allow properties next to $ref other than description and summary (as defined by ReferenceObject), but the PathItemObject seems to allow any properties that are part of PathItemObject alongside the $ref (even tho it's undefined how to interpret those properties).

The get the specified behavior, I think we'd have to add $ref the PathItem schema and change path-item-or-reference to be use anyOf instead of if/then.

Stranger6667 pushed a commit to schemathesis/schemathesis that referenced this issue Oct 5, 2024
Schemathesis uses static version of the current published version
of the OpenApi 3.1 spec metaschema
(https://spec.openapis.org/oas/3.1/schema/2022-10-07)
to validate openapi 3.1 spec documents. Unfortunately, the published
version has at least one known bug in which the schema for `paths`
references the definition of a concrete `path-item` instead of
`path-item-or-reference`, which might still be technically incorrect
when it comes handling the case of ref and sibling fields, but is
correct according to the documented definition of a pathItemObject.

This oversight has been noticed multiple times
OAI/OpenAPI-Specification#3298
OAI/OpenAPI-Specification#2635 (comment)
OAI/OpenAPI-Specification#2635 (comment)
OAI/OpenAPI-Specification#3513
OAI/OpenAPI-Specification#2657 (comment)

And finally fixed in Feb 2024
OAI/OpenAPI-Specification#3355
with a slightly bigger rework of the pathItem schema.

Sadly, due to confusion about how to release fixes in schemas
OAI/OpenAPI-Specification#151 (comment)
this change has not been published anywhere except schema.yaml in the
git repo, not even in schema.json, which appearantly only gets refreshed
once per release of the metaschema
OAI/OpenAPI-Specification#3355 (comment)

This commit updates the stored schema from the most up-to-date 3.1.0
schema.yaml from 0035208 to close the bug and make spec-valid openapi
spec files that use $ref under path finally validate correctly in
schemathesis. It also adds a corresponding regression test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants