-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Port remaining 3.0.4 fixes with adjustments for 3.1.1 #3921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This copies the relevant Parameter Object fields to the Header Object instead of relying on implicit guidance. The text for the fields has been edited to reflect that only headers are being described. This also include an example of describing a header using the `content` field, and explaining why it is necessary to do so.
Co-authored-by: Ralf Handl <[email protected]>
* Replace the outdated "model" terminology with "schema" * Remove the outdated `text/plain` array example, which does not correlate with current OAS requirements * Rather than replacing the `text/plain` example direclty, enhance the example of serializing `application/json` content in `application/x-www-form-urlencoeded` request bodies.
This aligns allowReserved with style by similarly correlating it with RFC6570 operators. This will make it easier to write a more in-depth explanation of the process in an appendix. This also adds one of several appendixes to be added to clarify the most obscure details of Parameter Object and Encoding Object serialization. This clarifies the correspondence between OAS fields and RFC6570 operators, and acknowledges that some field values and combinations do not have analogues. It provides further guidance for how to use RFC6570 implementations to support these configurations. This includes a SHOULD directive regarding using RFC6570 expansion with the non-RFC6570 styles, as the use of "explode" and "allowReserved" does not otherwise make any sense. It perhaps could be a MUST. Examples are included to show both typical usage, and how to work around the lack of exact RFC6570 equivalences for certain configurations.
It's very unclear how numbers, booleans, and other non-UTF-8-string values are converted to strings, particularly for the form media types. This adds a brief appendix that acknowledges the lack of standardization, and points to resources for the few cases that do have specifications. It highlights concerns with relying on certain JSON Schema keywords or values for serialization, and suggests defining schemas of type string and requiring applications to perform the conversion prior to schema validation as a way to control the results. This also clarifies that schema validation occurs before serialization. Also add note about RFC6570 type conversions. The spec doesn't address it, but implementations often have their own rules.
This makes serializing cookie paramters and most header parameters with `schema` and `style` NOT RECOMMENDED. It is not clear that any `schema`-based serialization for cookies will produce a correct value (although the reason is sufficiently obscure that many implementations might ignore it and produce cookie-compliant output anyway). With headers, there are numerous pitfalls and only the simplest scenarios will work properly, although perhaps the warning here could be reworded to emphasize the safe scenarios more clearly. The details are relegated to an appendix, because truly, most people will not want to know. But recommending against syntactically legal configurations really does need to be explained in the spec. Also, don't use - in: header name: Cookie Because... yeah.
This adds the previously standalone security considerations document as a top-level section just before the appendices.
Guide readers to supplemental documentation, examples, related specificatioins, and extension registries. These sites answer many questions that otherwise get raised as GitHub issues.
Co-authored-by: Lorna Jane Mitchell <[email protected]>
…3861 7/11) Co-authored-by: Ralf Handl <[email protected]>
Thanks to @lornajane for the review feedback.
When we mention YAML's "Failsafe schema" we give it a lower-case "schem", as the YAML documentatio does. We also prefix it with "YAML". However, we capitalize "Schema" in "JSON Schema ruleset", which (given how much JSON Schema is used in the OAS) is a jarring overlap with "JSON Schema". This change aligns "YAML JSON schema ruleset" with "YAML Failsafe ruleset" and explicitly calls out that it is unrelated to JSON Schema.
… 1/4) The Encoding Object's `contentType` field takes a comma-separated list of either regular or wildcared media types. These are the "two types" mentioned in the previous wording – "two" here did *not* refer to a limit on the number of entries in the list. These are not exactly media-type or media-range values, as both of those include parameters. This change also moves the hard-to-follow list of default values out of the individual field cell and into its own table. It takes `Content-Disposition` out of the header field's cell and instead explains limitations on header usage, and explains how `Content-Disposition` is used for encoding. This explanation includes a suggestion on how other `multipart` formats could be used with an Encoding Object, since their unnamed parts otherwise cannot be supported. Finally, it clarifies the interaction between `contentType` and the three fields imported from the Parameter Object, by aligning the recommended (but not, for compatibility reasons, required) behavior with guidance added in 3.1.0.
Thanks to @notEthan for the comments!
This splits the Encoding Object's fixed fields table to make the usage more clear, and closer to how it is presented for the Parameter and Header Objects
Percent-encoding is a minefield, although in practice it mostly works. This appendix attempts to acknowledge the concerns and then define enough terminology and link to enough other specifications that interested readers will be able to research further details.
This aligns all examples with RFC6570 operator prefixing behavior, which was previously only shown for `matrix` and `label`. The non-RFC6570 styles (`spaceDelimited`, `pipeDelimited`, and `deepObject`) are treated as analogues of `form` and therefore prefixed with a `?`. The lack of suitablity of this for cookie parameters has been addressed with an appendix in another change, and the appendix has been stubbed out here to ensure that the link is valid. Switch the "empty" column heading to "undefined" to align with RFC6570 and make clear that it is not about `allowEmptyValue`
The PR adding Appendix B pre-dated giving the Header Object its own field tables. This adds "Header Object" to the list of relevant Objects along with the Paramter Object and the Encoding Object.
The XML Object's namespace field was changed from "URL" to "absolute URI" because relative references in a namespace are deprecated by XML, and the base URI to use for resolving them in the context of an OpenAPI Description is unclear. However, XML namespaces can include fragments, and the correct term is "non-relative URI" rather than "absolute URI" which forbids fragments. This change includes additional guidance on how XML usage and the requirements of this specification do not quite align.
Be very explicit that discriminator MUST NOT change the validation outcome, and explain the implication for the "allOf" use case.
It was a bit challenging to figure out where to put the wording about allowing extension parameters when there are multiple fixed fields tables, each in their own subsection. For the Parameter Object (the only one with multiple tables in past releases), it had been after the last table, but that got further and further away from what felt like the main part of the Object description. I thought I had put it consistently after the initial "Common Fixed Fields" table, but I put it even before that in one place (which we don't do anywhere), and I forgot to include it in the Header Object at all. This change puts it after the Common Fixed Fields table for all three, which means that for all Objects it is immediately after the first Fixed Fields table.
This was referenced Jun 21, 2024
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
clarification
requests to clarify, but not change, part of the spec
discriminator
editorial
Wording and stylistic issues
headers
links
media and encoding
Issues regarding media type support and how to encode data (outside of query/path params)
metadata
tags, info, license, contact, markdown usage, etc.
param serialization
Issues related to parameter and/or header serialization
registries
Related to any or all spec.openapis.org-hosted registries
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR really has two parts, and you can click each "series of commits" to see the diff of just that part:
format
tocontentEncoding
IMPORTANT NOTE: The 3.1.0 spec is inconsistent about whether
type: string
withoutcontentEncoding
and any other primitive type (number
,integer
,boolean
) regardless ofcontentEncoding
defaults to a Content Type oftext/plain
(per §4.8.1.4) orapplication/octet-stream
(per thecontentType
fixed fields entry). This PR settles ontext/plain
as that is what the 3.0.3contentType
fixed fields entry said, and the change fromformat
tocontentEncoding
should not have impacted that. See also #3922 for a similar fix to 3.0.4.This is a good opportunity to look for consistency although I will ask that you DO NOT hold up this PR with consistency/editorial changes that might need to also go into 3.0.4. Please either open a PR on 3.0.4 with those, or start an issue where we can track them.
I squashed the ported PR commits somewhat, and just retained separate commits when review feedback was substantial enough that I thought it warranted a separate commit for crediting or clarity purposes, or (in the case of 3861) the original PR was a grab-bag of different tiny fixes in the first place.
Ports:
allowReserved
(3.0.4) #3818