microsoft / api-guidelines

Microsoft REST API Guidelines
Other
22.69k stars 2.71k forks source link

Add guidance for handling of unrecognized query/header parameter or body property #458

Open mikekistler opened 1 year ago

mikekistler commented 1 year ago

What is the expected / recommended behavior of a service for a request that contains a query or header parameter that it does not recognize? What about an unexpected body property?

And what if the param / property is recognized but only for a later api-version than the one specified in the request?

JeffreyRichter commented 1 year ago

If a client sends a query param or JSON body property not supported by the service, the service must return 400. If the client sends a request header not supported by the service, the service just ignores it.

If the query param/JSON property is defined in a later api-version than the api-version the customer is passing, then this is still a 400 because the customer-specified version does not support what the customer is passing. Also, it would be breaking behavior for V5 of the service to introduce the new query param/property and have the old client fail passing these until V5 all of a sudden shows up on the service. And even more, a service could deprecate some QP/property (which would be breaking, of course) and then introduce the QP/Property again in the future with different semantics (not breaking) which would also change client-expected behavior.

weidongxu-microsoft commented 1 year ago

I think the body property is already clear in guideline https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#rest-fail-for-unknown-fields

Agree that header can be ignored (But could it be a bit risky for some well-known ones like If-None-Match used in PUT, but service not supporting it -- assume SDK didn't catch it. The result could be that user does not intend to update an existing resource, but service go ahead and update it anyway.).

mikekistler commented 1 year ago

If the client sends a request header not supported by the service, the service just ignores it.

I think this is not quite right. The HTTP RFC (9110, which replaces 7231) says:

Other recipients SHOULD ignore unrecognized header and trailer fields.

I think this is commonly cited as the justification for ignoring unsupported headers. But there is a subtle difference between a header being "unrecognized" and being "unsupported".

The content around this statement suggests that it refers to fields that might be added in the future -- that a server implemented now would have no way to "recognize". Consider the immediately following sentence:

Adhering to these requirements allows HTTP's functionality to be extended without updating or removing deployed intermediaries.

And the immediately preceding paragraph:

New fields can be introduced without changing the protocol version if their defined semantics allow them to be safely ignored by recipients that do not recognize them; see Section 16.3.

So a new field can be introduced only if it is safe to ignore … by servers that do not "recognize" it.

How might that differ from a field that is "not supported"? Consider "if-match". Is "if-match" safe to ignore? Not really, because a client that sends if-match on a PUT or PATCH or DELETE request is requesting that the operation on be performed if the if-match condition evaluates to true. A server that ignores this and performs the operation even if the condition evaluates to false would cause silent corruption. So if if-match were not defined in HTTP 1.1, it could not be added because it is not safe to ignore.

But it is defined in HTTP 1.1 (in RFC 9110), so I think servers are not allowed to claim that it is "not recognized", even when it is "not supported". And the description of if-match later in RFC 9110 supports this interpretation.

When an origin server receives a request that selects a representation and that request includes an If-Match header field, the origin server MUST evaluate the If-Match condition per Section 13.2 prior to performing the method.

An origin server that evaluates an If-Match condition MUST NOT perform the requested method if the condition evaluates to false.

Note that the RFC says "MUST" and not "SHOULD".

There is similar language for if-non-match, if-unmodified-since, and if-range (curiously, if-modified-since only uses "SHOULD").

I did not check the entire RFC 9110, so it is possible that there are other fields which cannot be ignored by compliant servers. I did scan the content negotiation section which has a bunch of headers and found none there that could not be ignored.

So the general guidance to ignore unsupported headers seems right, but with the caveat that there are a small set of headers that cannot be ignored, including if-match, if-non-match, if-unmodified-since, and if-range. If a service receives a request with one of these headers and is not prepared to correctly process it, the request must be rejected with a 400 response.

JeffreyRichter commented 1 year ago

I think you make very sound arguments, Mike. I agree with you that state corruption will occur if if-match is sent but ignored. So, based on your arguments, how would you craft proper guidelines (considering that teams may want to add proper if-match, etc. support to their service in the future)?