Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use brotli for all compressed HTTP responses #1882

Open
r4co0n opened this issue Oct 7, 2021 · 7 comments
Open

Use brotli for all compressed HTTP responses #1882

r4co0n opened this issue Oct 7, 2021 · 7 comments
Assignees
Labels
feature New feature or request hacktoberfest Good for newcomers low Low priority

Comments

@r4co0n
Copy link
Contributor

r4co0n commented Oct 7, 2021

Describe the bug

Currently, not all compressed files being served by Apache use the same compression algorithm. Apparently, we wanted to enable brotli-compression only for some file types, similar to what is done with gzip compression for NGINX in Nextcloud's example configuration.

However, previously files seem to have been compressed using gzip, which now remains the compression algorithm for some responses not explicitly listed in the apache config.

Additionally, please note that our implementation of gzip compression seems to strip the Etag-header, not following the workaround in Nextcloud's example configuration. (GET https://nextcloud.example.org/ocs/v2.php/apps/notifications/api/v2/notifications, served uncompressed, contains the Response Header ETag, .../heartbeat as documented below, compressed with gzip, does not.

To Reproduce

Steps to reproduce the behavior:

  1. Open the web devolopment tools of your favorite browser and open the request logging tool. (For Firefox: [Ctrl]+[Alt]+[K], select tab "Network")
  2. Go to some webpage served by your nextcloud instance (I used the dashboard.)
  3. Inspect the requests made, and find that some do use compression, but gzip, indicated by the Response Header Content-Encoding: gzip. These include:

Thanks to @pachulo for additional documentation regarding his PR #1447 following my question in release PR #1880.

I will try to find out why our Apache thinks to deliver stuff gzip-compressed and report back. P.S.: I already found some code in nextcloud/server compressing the responses using gzip itself, so Apache is just passing on already compressed responses, it seems.

In my opinion, some paths could (continue to) be excluded from compression, such as .../heartbeat, .../notifications and probably some other URLs we would need to identify. If this is helpful or a needlessly complex optimization remains up for debate.

Expected behavior

All compressed files delivered by Apache use brotli-compression as introduced with #1447.

Snap version

$ snap list nextcloud
Name       Version      Rev    Tracking       Publisher   Notes
nextcloud  22.2.0snap1  28575  latest/stable  nextcloud✓  -
@r4co0n
Copy link
Contributor Author

r4co0n commented Oct 7, 2021

Looking through nextcloud/server's code and mentions of gzip further, it seems that this is married to the application without a configuration option. We could only remove gzip from every request's header Accept-Encoding declarations, if I'm not mistaken, which somehow seems excessive.

Maybe this will become an issue that is closed without further action. Though this seems to be like something to raise upstream, at least, I don't really understand why there are differences in the compression type used for text/css, depending on if ;charset=UTF-8 is appended.

@kyrofa
Copy link
Member

kyrofa commented Oct 7, 2021

If this is helpful or a needlessly complex optimization remains up for debate.

Any discussion is helpful! Sadly it's out of my realm of expertise, so I will defer to @pachulo.

@github-actions
Copy link

github-actions bot commented Dec 7, 2021

This issue is stale because it has been without activity for 60 days. It will be closed after 7 more days of inactivity.

@github-actions github-actions bot added the Stale label Dec 7, 2021
@pachulo
Copy link
Member

pachulo commented Dec 8, 2021

Sorry, I forgot to check this. I will try to do these holidays.

@pachulo pachulo self-assigned this Dec 8, 2021
@pachulo pachulo removed the Stale label Dec 8, 2021
@github-actions
Copy link

github-actions bot commented Feb 7, 2022

This issue is stale because it has been without activity for 60 days. It will be closed after 7 more days of inactivity.

@github-actions github-actions bot added the Stale label Feb 7, 2022
@pachulo pachulo reopened this Feb 14, 2022
@pachulo pachulo added hacktoberfest Good for newcomers low Low priority and removed Stale labels Feb 14, 2022
@kyrofa kyrofa added the feature New feature or request label Feb 14, 2022
@kyrofa
Copy link
Member

kyrofa commented Feb 14, 2022

@pachulo issues won't be touched by the stale bot if they're triaged to use one of these labels.

@StarSmasher44
Copy link

Seems like this is rather old.... But has there been any improvements upon moving/utilizing brotli over Gzip?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request hacktoberfest Good for newcomers low Low priority
Projects
None yet
Development

No branches or pull requests

4 participants