If an encoding has a qvalue strip it

[rouzier]

---

name: Pull request
about: Take in consideration the the qvalue when matching Accept-Encoding
title: ''
labels: ''
assignees: ''

1. What does this change do, exactly?

Take in consideration the the qvalue when matching Accept-Encoding.
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.3

2. Checklist

• I have written tests and verified that they fail without my change
• I have squashed any insignificant commits
• This change has comments explaining package types, values, functions, and non-obvious lines of code
• I am willing to help maintain this change if there are issues with it later

[mholt]

Thanks... someone will review this eventually. :)

What is your specific use case / need for this (advanced content negotiation)? i.e. what are your actual requirements?

[rouzier]

None.
I was reviewing the code and I noticed that qvalue was not properly handled.
So I supplied a fix.

[mholt]

Okay. I appreciate it, I really do! But since there isn't an actual use case for it yet, it isn't a huge priority for me personally (Caddy 2 is currently on my plate, and if q-factor'ed content negotiation is important, I can work it into Caddy 2). But I see that this is a decent change since it has tests; albeit the feature is only partially implemented (which is still OK).

If someone considers this a priority they are welcome to review it and I can see about merging it in, I just can't get to it myself quite right now. Thanks!

[rouzier]

Feel free to close the PR if it does not fit your needs or if you know if that section of code will be rewritten

…

On Wed., May 29, 2019, 6:17 p.m. Matt Holt, ***@***.***> wrote: Okay. I appreciate it, I really do! But since there isn't an actual use case for it yet, it isn't a huge priority for me personally (Caddy 2 is currently on my plate, and if q-factor'ed content negotiation is important, I can work it into Caddy 2). But I see that this is a decent change since it has tests; albeit the feature is only partially implemented (which is still OK). If someone considers this a priority they are welcome to review it and I can see about merging it in, I just can't get to it myself quite right now. Thanks! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2630>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATACXBYNX2QGCWFZUQXC4LPX36INANCNFSM4HQVOHAQ> .

[mholt]

FWIW, Caddy 2 does have content negotiation respecting q-values with regards to dynamic encodings: https://sourcegraph.com/github.com/caddyserver/caddy@0d3f99e85a089a6a772ae38f426b5cd5f2f4583f/-/blob/modules/caddyhttp/encode/encode.go#L232

However, content negotiation in general still needs work. I am tracking this in #2665. Although, I am not sure if a matcher is the right approach anymore. (Discuss before implementing!) But would be happy to have your help on it if you're interested.