Handle invalid Accept header values properly #2316
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2316 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 41 41
Lines 2304 2304
=======================================
Hits 2267 2267
Misses 21 21
Partials 16 16 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This PR does not finish the necessary work. Each media-range can include a quality factor, e.g. text/xml;q=0.5 and these alter the preference order.
The algorithm should be:
- split the header on
','into a list of media-ranges. - foreach media-range, split on
';'into parts - split the first part on
'/'as done in this PR - find the
qquality option from the remaining parts; if absent the default isq=1 - sort all the parts by quality, without altering the order of those that have the same
q - match the sorted list against the offered content
Note that the RFC allows other options as well as q.
| @@ -1047,19 +1047,27 @@ func (c *Context) NegotiateFormat(offered ...string) string { | |||
| return offered[0] | |||
| } | |||
| for _, accepted := range c.Accepted { | |||
| // According to RFC 2616, media-range = ( "*/*" | ( type "/" "*" ) | ( type "/" subtype ) ) | |||
There was a problem hiding this comment.
need to update this to RFC7231 section 5.3.2
https://tools.ietf.org/html/rfc7231#section-5.3.2
|
As it happens, there's a bunch of existing code to parse It's also MIT licence so we could pull the code over (and acknowledge). |
|
Thank you for your comment. My intention of the PR is to prevent gin from having out-of-range access when an invalid HTTP accept header is given. |
|
I am hoping to have the time to create a patch based on https://pkg.go.dev/github.com/rickb777/[email protected]#Accept, although that code needs to reach v1 first (a bit more testing effort needed there). |
|
I'm still developing https://pkg.go.dev/github.com/rickb777/negotiator, which has changed a lot since I last posted. It's now v0.9.0 and even now not yet stable. However, it is able to work with Gin, which is a good thing. |
NegotationFormatraises out-of-range access to an offer string with some invalid HTTP Accept header values. It happens with the current master: 3315353 .For example, the following header causes an internal server error.
I also found
NegotationFormatreturns an inappropriate result with some invalid Accept values. Please refer to theTestContextNegotiationFormatWithInvalidAccept.So I updated the implementation to handle those invalid values properly.
I found that the wildcard support for content negotiation was introduced in #1112.
But it seems there is no discussion about the old implementation.
I am sorry if I missed something.