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

Guidance about Accept header for servers #1050

Open
david-perez opened this issue Jan 12, 2022 · 5 comments
Open

Guidance about Accept header for servers #1050

david-perez opened this issue Jan 12, 2022 · 5 comments
Labels
documentation This is a problem with documentation. guidance Question that needs advice or information. server This issue involves the specification for server software.

Comments

@david-perez
Copy link
Contributor

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?
@adamthom-amzn
Copy link
Contributor

adamthom-amzn commented Jan 13, 2022

  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.

@david-perez
Copy link
Contributor Author

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.

@adamthom-amzn
Copy link
Contributor

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
Copy link
Contributor

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.

@david-perez
Copy link
Contributor Author

david-perez commented Jan 14, 2022

@adamthom-amzn Sounds good. That is in fact the same S3-like use case that a user of our sSDK wants to build: smithy-lang/smithy-rs#1023 (comment)

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?

@milesziemer milesziemer added the guidance Question that needs advice or information. label Sep 13, 2022
@kstich kstich added the documentation This is a problem with documentation. label Aug 15, 2023
@kstich kstich added the server This issue involves the specification for server software. label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation. guidance Question that needs advice or information. server This issue involves the specification for server software.
Projects
None yet
Development

No branches or pull requests

4 participants