linuxserver / docker-grav

GNU General Public License v3.0
31 stars 11 forks source link

enable gzip compression in nginx by default #39

Closed psy0rz closed 8 months ago

psy0rz commented 11 months ago

Is this a new feature request?

Wanted change

Please add:

gzip on;

gzip_types
        application/atom+xml
        application/javascript
        application/json
        application/ld+json
        application/manifest+json
        application/rss+xml
        application/vnd.geo+json
        application/vnd.ms-fontobject
        application/x-font-ttf
        application/x-web-app-manifest+json
        application/xhtml+xml
        application/xml
        font/opentype
        image/bmp
        image/svg+xml
        image/x-icon
        text/cache-manifest
        text/css
        text/plain
        text/vcard
        text/vnd.rim.location.xloc
        text/vtt
        text/x-component
        text/x-cross-domain-policy;

To the default nginx.conf that is generated in /config.

Reason for change

this helps to reduce loadtime on mobile.

Proposed code change

No response

nemchik commented 11 months ago

It looks like the config you posted is sourced from https://learn.getgrav.org/17/webservers-hosting/servers/nginx#example-nginx-conf which sources from https://github.com/h5bp/server-configs-nginx/blob/main/h5bp/web_performance/compression.conf

Our configs source from https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/main/nginx/nginx.conf?ref_type=heads and SSL configs from https://ssl-config.mozilla.org/#server=nginx&version=1.24.0&config=intermediate&openssl=3.1.1&guideline=5.7

I also notice https://learn.getgrav.org/17/basics/grav-configuration#cache lists

allow_webserver_gzip This option will change the header to Content-Encoding: identity allowing gzip to be more reliably set by the webserver although this usually breaks the out-of-process onShutDown() capability. The event will still run, but it won't be out of process, and may hold up the page until the event is complete

I don't know enough about that header, or about grav's shutdown capability to know if this is cause for concern with setting the gzip stuff in the nginx configs.

I can say we definitely wouldn't ship the gzip configs in the nginx.conf but would consider shipping it in the default site conf (it should be valid there, and we do that with nextcloud).

LinuxServer-CI commented 10 months ago

This issue has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 7 months ago

This issue is locked due to inactivity