smithy icon indicating copy to clipboard operation
smithy copied to clipboard

Guidance about `Accept` header for servers

Open david-perez opened this issue 2 years ago • 5 comments

The protocol test RestJsonNoInputAllowsAccept caught my eye.

  1. Where in the Smithy spec is it specified how servers should handle this header?
  2. Should restJson1 servers reject requests if they don't have Content-Type set and the Accept header is not set to any of these values?
  3. Should servers accept requests that don't specify Content-Type nor Accept, but whose body is empty?

david-perez avatar Jan 12 '22 16:01 david-perez

  1. In general if a behavior is well-documented in the relevant HTTP RFCs, we try not to duplicate it in the smithy spec. If there's something surprising about how we're handling Accept and Content-Type relative to the HTTP RFCs, then we should document it. Most of the behavior around Content-Type and Accept is controlled by RFC 7231

  2. (and 3.) restJson1 servers should require Content-Type to be set to application/json if the relevant operation has an input and is a structure. It should tolerate a Content-Type of application/json to be set for operations without an input, since it knows there is nothing to deserialize. Accept should include application/json if the output is a structure, or it should be set to * or omitted completely. Note that all this changes when the input or output is not a structure, such as when it is a blob, in which case Content-Type and Accept handling is delegated to the application owner.

I put a lot of SHOULDs in there because the existing server-side implementations of restJson1 are more lax about these rules than what I've described. Smithy-Typescript's behavior should be close to this unless I botched the implementation.

adamthom-amzn avatar Jan 13 '22 17:01 adamthom-amzn

These are the concrete choices I'm making for the implementation of the Rust sSDK. Feel free to close the issue if these choices are good and compliant with the specs.

  1. Should restJson1 servers reject requests if the Accept header is not set to any of these values?

I don't see any special handling for the Accept header in the restJson1 spec. HTTP RFC 7231 says:

If the header field is present in a request and none of the available representations for the response have a media type that is listed as acceptable, the origin server can either honor the header field by sending a 406 (Not Acceptable) response or disregard the header field by treating the response as if it is not subject to content negotiation.

So I'll simply disregard the Accept header entirely for restJson1.

Similarly with any other protocol that doesn't explicitly specify how to handle the Accept header.

  1. Should servers accept requests that don't specify Content-Type nor Accept, but whose body is empty?

Accept doesn't matter, as mentioned earlier. If the body is empty, I'll also ignore Content-Type, and hence accept the request.


when it is a blob, in which case Content-Type and Accept handling is delegated to the application owner

I don't understand this part. restJson1 spec says Content-Type MUST be application/octet-stream if it is a blob.

david-perez avatar Jan 13 '22 18:01 david-perez

So I'll simply disregard the Accept header entirely for restJson1.

We're using a 406 response for the TypeScript SSDK, because I think someone sending something like Accept: text/xml to a restJson1 service is probably misconfigured. But the spec allows the behavior you've selected.

restJson1 spec says Content-Type MUST be application/octet-stream if it is a blob.

Yeah I don't like it. I'm going to take this up internally.

adamthom-amzn avatar Jan 13 '22 19:01 adamthom-amzn

I'm proposing we change the spec for blob to say the Content-Type can be anything, and handling of the content is delegated to the server implementation. Does this work for you?

My mental model is this: if I wanted to have a REST API for a content repository that could receive images or videos of many different formats, Content-Type is the natural way to convey that. application/octet-stream discards important data in this use-case, and blob would be the appropriate member choice.

adamthom-amzn avatar Jan 13 '22 19:01 adamthom-amzn

@adamthom-amzn Sounds good. That is in fact the same S3-like use case that a user of our sSDK wants to build: https://github.com/awslabs/smithy-rs/pull/1023#discussion_r777568429

But they're using restXml, which currently also says that Content-Type MUST be application/octet-stream in the case of blob. Shall we change that too?

david-perez avatar Jan 14 '22 10:01 david-perez