Allow forbidding undefined query string params

[handrews]

• Fixes How to disallow unknown query parameter? #2511

[mikekistler]

I am wondering how this feature will be used in practice.

I don't think users of the API get any benefit from it. It does not "describe" any useful behavior of the API, unlike additionalProperties which can include a description that explains the meaning of additional properties. It specifies when the operation is required to fail but not when it actually will fail, since undefined query parameters can still cause failures when undefinedQueryParameters is true, the default. So users must be prepared for failures either way.

I suppose this clarifies to implementers how they should treat undefined query parameters, but this assumes that the API description is written by someone other than the team that implements the API, which in my experience is rather rare.

I do understand that, from an API governance perspective, rejecting undefined query parameters is a best practice and failures to do so should be rooted out and corrected. But I don't think adding undefinedQueryParameters helps much on this front -- and could actually hinder it, since it gives implementers an excuse for not following this best practice -- "Well, we didn't document that undefined query parameters must fail, so we don't need to fix our service".

In short, I don't see much practical value in this new keyword and would not recommend it's use in any of the APIs I work with, but if others feel strongly that it is needed I wouldn't oppose it.

[handrews]

@mikekistler according to a recent discussion I had with some folks, it's actually increasingly common to intentionally allow unexpected query parameters because of the tendency of various entities to tack various sorts of tracking parameters onto query strings.

It could be better documented here that not forbidding undefined parameters is not a guarantee that they will be handled well or at all. It's just that forbidding them ensures enforcement at all tooling levels instead of deferring to the server.

Also, you say this wouldn't be useful, but the issue has many people chiming in that they want it.

---

As a meta-topic, I want to note that this issue has been open for more than two years without a single comment saying we shouldn't do it. If the TSC doesn't want changes made, it should close the issues and say so. Not leave it open as if we might do something about it and then shut it down the instant someone takes action that looks non-controversial.

(@mikekistler this is not directed at you personally – you only just joined the TSC anyway – it's more of a general observation that we do not manage expectations well, nor do we provide good guidance to people who want to make PRs and move things forwards.)

[karenetheridge]

I'm unsure that a specific config is needed here given that it's possible to do already with style=form, explode=true, type=object, which will cause a single parameter will consume all values in the query string as a json object (and then additionalProperties: true or additionalProperties: false can be used).

This functionality should be documented as an example in the specification, as it is quite useful (it also solves the "how to allow query parameters to depend on each other" question) but not immediately obvious to those unfamiliar with json schema (or those who have written an openapi validator 😛).

[handrews]

As noted in #2511, I now think solving #1502 is a more comprehensive solution to this and several other problems. I agree with @karenetheridge that the workaround (which handles probably most cases) ought to be documented as well, but that can be handled separately for the patch releases and needn't wait to 3.2.

I'm going to close this to reduce clutter- we can revisit it if #1502 is rejected.