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

RFC: Accept header parsing #2171

Closed
ahopkins opened this issue Jun 21, 2021 · 2 comments · Fixed by #2200
Closed

RFC: Accept header parsing #2171

ahopkins opened this issue Jun 21, 2021 · 2 comments · Fixed by #2200

Comments

@ahopkins
Copy link
Member

The Accept header in RFC 7231 § 5.3.2 has a very structured format for specifying media types that the client can accept, and also would prefer to accept.

Source

A more elaborate example is

         Accept: text/plain; q=0.5, text/html,
             text/x-dvi; q=0.8, text/x-c

   Verbally, this would be interpreted as "text/html and text/x-c are
   the equally preferred media types, but if they do not exist, then
   send the text/x-dvi representation, and if that does not exist, send
   the text/plain representation".

   Media ranges can be overridden by more specific media ranges or
   specific media types.  If more than one media range applies to a
   given type, the most specific reference has precedence.  For example,

         Accept: text/*, text/plain, text/plain;format=flowed, */*

   have the following precedence:

   1.  text/plain;format=flowed
   2.  text/plain
   3.  text/*
   4.  */*

#2162 adds some context based error formatting based upon the initial #1937. Where both of those PRs are lacking is the ability to confirm that the media being sent is warranted and acceptable.

The proposal is to add a new accessor: Request.accept that provides a simplified list of acceptable types in preference order. Where there is a tie for equal precedence, Sanic should prefer the first to be listed in the raw headers.

This change will allow for the error response to make sure that it is sending an appropriate format, and also provides the developer an easy way to select one of multiple formats for responding. The last potential benefit is that Sanic could offer a "strict mode" (defaul=False) that would raise an error if it attempts to respond with a type that is not acceptable.

@ahopkins ahopkins mentioned this issue Jun 21, 2021
3 tasks
@Tronic
Copy link
Member

Tronic commented Jun 21, 2021

While it is nice to support standards, I believe there is little practical use for full parsing of this header. Pretty much the only thing supplying more than one type is the browser requesting pages, images, scripts etc. (each with their own accepted types), and even then the lists come already in order of precedence. And that precedence might not make much sense in regards to what formats you actually want to use (e.g. audio/* instead of audio/wav which has higher precedence with Firefox).

I suppose that in operator should work on exact strings, no wildcard matching. Being standards-compliant an app would then need to match also against the wildcard versions as needed.

Not saying it shouldn't be done like you proposed, just raising points. Needs more thinking and practical use cases.

@ahopkins
Copy link
Member Author

Not saying it shouldn't be done like you proposed, just raising points. Needs more thinking and practical use cases.

Agreed. I think the practical use case (as you alluded to in #2162, and what was the original inspiration for this RFC) would be to allow Sanic to make some "smart" decisions. Of course this could be extendable, and having wildcard support might be ideal. However, I am not sure that we need to be overly concerned with that in the core project just yet.

I would be content to simply order them appropriately and let whatever implementation that plans to use the property determine how to handle the *. We would need to address it in #2162, but I don't think we need to go down the route of providing some sort of matching API. That seems too much complexity for a small side feature.

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

Successfully merging a pull request may close this issue.

2 participants