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

Handling undefined request parameters #110

Closed
pvdbosch opened this issue Feb 28, 2023 · 9 comments
Closed

Handling undefined request parameters #110

pvdbosch opened this issue Feb 28, 2023 · 9 comments

Comments

@pvdbosch
Copy link
Contributor

pvdbosch commented Feb 28, 2023

Formulate guidelines how an API should handle request parameters (in JSON body, query param or other) that aren't defined in OpenAPI.

For response elements, the guideline is for clients to ignore unknown properties to be able to evolve an API:

  • Service consumers should be aware that JSON objects can always be extended with additional JSON properties. This shouldn’t break the client code. The unknown properties must be ignored.

For request parameters, this makes less sense however:

  • having a request parameter ignored, means that the API won't fulfill the clients' expectations
  • client implementation errors (with wrong param name) are more common than server-side and could go unnoticed

A (very specific) counter-example is an Angular paging table view component which may send additional query params to an API that can be safely ignored.

OpenAPI (JSON Schema) allows additional JSON properties by default, but for query parameters the situation is less clear.

Some interesting reads on this topic:

@pvdbosch
Copy link
Contributor Author

pvdbosch commented Mar 15, 2023

eHealth encountered a problem where a typo in a client did go unnoticed because it wasn't validated, so seems useful to them.

Default Axway APIGW behavior is to strip unknown query params which is also quite dangerous (but this behavior can be changed to pass through any unknown query param).

Other use case: support for complex search queries on a lot of parameters, e.g. "?age=...", "?age_lt=...", "?age_gt=..."

For JSON payloads, strictly speaking additional properties are by default allowed, but this behavior can probably be configured in most validators (I'll verify this). Using a different schema for request (disallowing additional props) and response (allowing additional props) has too much impact however.

WG decided that:

  • default expected behavior should be to produce a bad request problem (InputValidationProblem) for unknown path/query/body parameters, but specific APIs may deviate if there's a good reason to do so.
  • If a REST API does support undefined params, this should be explictly documented in the OpenAPI file in the operation level description.
  • Unknown HTTP request headers should always be allowed; they're usually not business-critical and only technical (not business-critical). Lots of frameworks and software generate HTTP headers.

We could consider standardizing upon an OpenAPI extension keyword to declare support of undefined parameters (e.g. x-additional-query-params: like OpenAPI's additionalProperties/patternProperties), but this doesn't seem worthwhile for now.

I'll create a PR with a rule.

We should still discuss if another type of input validation issue than "schemaViolation" should be returned within the bad request problem response.

@pvdbosch
Copy link
Contributor Author

pvdbosch commented May 5, 2023

I couldn't find any option to configure commonly used openapi validators to fail on unknown JSON properties during request processing.

One feature request was refused, saying that the schema should be changed instead to additionalProperties: false (e.g. with some pre-processing mechanism). This could be a hassle, cause this makes it difficult to use the same schema in request and response.
For query parameters, there's no way to specify in OpenAPI that unknown params should be (dis)allowed: OAI/OpenAPI-Specification#2511

The Jackson library can be tuned to fail or ignore unknown JSON properties:

@pvdbosch
Copy link
Contributor Author

pvdbosch commented May 8, 2023

Proposal for issue type: urn:problem-type:belgif:input-validation:unknownRequestParameter (within badRequest problem response)

@jpraet
Copy link
Contributor

jpraet commented May 9, 2023

For consistency with #113, where issue types like urn:problem-type:belgif:input-validation:mutuallyExclusiveParameters and urn:problem-type:belgif:input-validation:incompleteParameterGroup are proposed:

maybe urn:problem-type:belgif:input-validation:unknownParameter?

Or should we add the Request to the other issue types? urn:problem-type:belgif:input-validation:mutuallyExclusiveRequestParameters, urn:problem-type:belgif:input-validation:incompleteRequestParameterGroup? That becomes quite verbose.

@pvdbosch
Copy link
Contributor Author

pvdbosch commented May 9, 2023

I agree to drop "request", it's redundant with "input-validation"

@pvdbosch
Copy link
Contributor Author

TODO: check if those using GCloud's APIGW can work together for implementation
Agreed to standardize on 'urn:problem-type:belgif:input-validation:unknownParameter'

(only in combination with in: query or in: body ; in : path => ResourceNotFound , in: header => unknown is allowed for http headers)

@jpraet
Copy link
Contributor

jpraet commented May 12, 2023

Maybe an explicit mention should be made in the guidelines that this rule does not apply for API's that consume JSON events / notifications / webhooks via an HTTP POST? A new property added by the publisher of the events should not cause an error at the consumer side. Just like unknown properties should be ignored by the client when consuming data retrieved from a server.

@pvdbosch
Copy link
Contributor Author

Unknown parameters should indeed be accepted in any webhook/callback request.
Webhooks/callbacks aren't documented in the guide however, and I don't yet know of any implementations following the guide using them; so mentioning them could cause more confusion than clarity.

We'd also have to clearly define semantics around callbacks (client, service, API, ...) to avoid confusion because the API (application programming interface) is defined by the sender of the events in such a case.

@jpraet
Copy link
Contributor

jpraet commented May 21, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants