Skip to content
This repository has been archived by the owner on Sep 18, 2019. It is now read-only.

Possibility to prefer one encoding over the other if both are acceptable #589

Closed
benib opened this issue Nov 29, 2017 · 2 comments · Fixed by hapijs/hapi#3717
Closed

Possibility to prefer one encoding over the other if both are acceptable #589

benib opened this issue Nov 29, 2017 · 2 comments · Fixed by hapijs/hapi#3717

Comments

@benib
Copy link

benib commented Nov 29, 2017

When using https://github.com/kanongil/brok to enable brotli compression the plugin registers a new encoding br that gets pushed to the end of the encoding array here: https://github.com/hapijs/hapi/blob/master/lib/compression.js#L52.
Therefor hapijs/accept gets the encoding with br at the last position and thus always prefers gzip over br if no explicit weights are defined in the accept-encoding header. Chrome and Firefox (and maybe others) send gzip, deflate, br. So they never get served brotli encoded content.

I would work on a PR but would like to get some guidance first.

I see these ideas, there are for sure others that are probably better:

  1. accept a third parameter on server.encoder with a priority. the default priority for all internal encodings could be 0.5. The encodings then get sorted by this value here: https://github.com/hapijs/hapi/blob/master/lib/compression.js#L40.

  2. instead of this.encodings.push use this.encodings.unshift to add new encodings here: https://github.com/hapijs/hapi/blob/master/lib/compression.js#L52. This would lead to all user added encodings being preferred over the Hapi internal ones.

  3. exposing server.sortEncodings taking a function used to sort the encodings with Array.sort. This does not seem to be a good solution as it widens the API of server.

Any other ideas and guidance on how to implement this are very much appreciated.

@kanongil
Copy link

kanongil commented Nov 29, 2017

Setting a priority is probably something that should be configurable on the server, or maybe even at the route level. Ie, option 1 & 2 won't work.

As for option 3, I don't think it is the right approach. A rework of the route.options.compression setting should work better. For example allow the option to be an array which implies the preferred order:

Joi.alternatives(
    Joi.object().pattern(/.+/, Joi.object()), /* existing schema */
    Joi.array().items(Joi.string(), Joi.object({ name: Joi.string(), settings: Joi.object() }))
).default()

This could probably be combined with option 2, to allow any newly added encodings default priority.

Btw. this is all speculative, without regard to the complexity and performance of the implementation.

@hueniverse
Copy link

Option 2 seems reasonable to me. This is not a widely used feature and when it is used, it usually means adding one more encoder...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants