jcorporation / myMPD

myMPD is a standalone and mobile friendly web mpd client with a tiny footprint and advanced features.
https://jcorporation.github.io/myMPD/
GNU General Public License v3.0
421 stars 66 forks source link

Accept-Encoding not honored #738

Closed dgcampea closed 2 years ago

dgcampea commented 2 years ago

myMPD version: mympd-devel sha256:fca795dacf4665e9dc1a5898d8b09d2af430bafff3ea718dc7450f979778d30f

Describe the bug HTTP header Accept-Encoding is not honored by mympd, gzip'd response returned.

This was found out while attempting to use nginx sub_filter.

To Reproduce

Issue a GET / request without Accept-Encoding or with Accept-Encoding: identity. Response is gzip'd. This can be tested with either wget/curl or wireshark if working via reverse-proxy.

Expected behavior

HTTP header Accept-Encoding is honored.

Additional context https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding https://trac.nginx.org/nginx/ticket/999

jcorporation commented 2 years ago

The assets are embedded precompressed in the myMPD binary and there is no inflation function in myMPD. I left it out for simplicity reasons.

Is this only a theoretical issue or a real problem?

dgcampea commented 2 years ago

I wanted to test #739 first by using ngx_http_sub_module but it didn't work due to the gzip response. I'd say it's "mostly theoretical" but it might bite unexpectedly on complex reverse proxy setups.

jcorporation commented 2 years ago

Thanks, documenting this behavior should therefore be sufficient?

dgcampea commented 2 years ago

While looking for potential improvements, Chromium Lighthouse tool (under the debugger/inspect tools) spotted that /api responses aren't compressed.

I remedied this within nginx:

    location / {
        proxy_pass http://localhost:6688/;
        gzip on;
        gzip_comp_level 9;
        gzip_proxied any;
        gzip_types text/html application/json;
    }

Should mympd also start to compress the /api endpoint, as it's not a static asset, it should honor this header as it allows the proxy to decide the compression parameters or encoding (possibly using brotli instead).

As for the static asset case, instead of inflating the assets, it can return 406 Not Acceptable according to the semantics in https://httpwg.org/specs/rfc7231.html#header.accept-encoding (additional reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding)

jcorporation commented 2 years ago

The question is what is faster, compression or not in a typical lan environment. myMPD is not a highly optimized webserver and would it be never. The used http server implementation (mongoose) is in some points very basic, e.g. compression must be manually implemented.