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

nixos/nginx: Do not remove headers while proxying #100708

Merged
merged 1 commit into from Oct 18, 2020

Conversation

@fooker
Copy link
Contributor

@fooker fooker commented Oct 16, 2020

Motivation for this change

Removing the Accept-Encoding header breaks applications which may
produce already compressed content.

Removing this header is staded in the nginx docs but is ment as an
example, not as an recomendation.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Removing the `Accept-Encoding` header breaks applications which may
produce already compressed content.

Removing this header is staded in the nginx docs but is ment as an
example, not as an recomendation.
ajs124
ajs124 approved these changes Oct 16, 2020
@mweinelt
Copy link
Member

@mweinelt mweinelt commented Oct 18, 2020

Indeed, a client could tell the server that it accepts a certain encoding, but what would never reach the upstream. Why would an upstream application then use compression?

I'm not an nginx expert, but couldn't nginx handle the Accept-Encoding header itself and that is why we decide not to pass it to the upstream?

@fooker
Copy link
Contributor Author

@fooker fooker commented Oct 18, 2020

Indeed, a client could tell the server that it accepts a certain encoding, but what would never reach the upstream. Why would an upstream application then use compression?

My specific use-case is OctoPrint. They can serve already gzipped asset bundles if the encoding is enabled. If the encoding is not set, the decompress the bundle on the fly and serve it uncompressed - resulting in a higher load and response time.

I'm not an nginx expert, but couldn't nginx handle the Accept-Encoding header itself and that is why we decide not to pass it to the upstream?

AFAIK, proxy requests will not be compressed in default config. There is gzip_proxied to enable that. In this case nginx will inspect the header and forward it. If the content is already returned compressed from upstream, it will be served as is. If it is uncompressed, nginx will compress it.

@mweinelt mweinelt merged commit 4baba17 into NixOS:master Oct 18, 2020
16 checks passed
dpausp added a commit to flyingcircusio/fc-nixos that referenced this issue Jan 26, 2021
Backends should be able to serve compressed content through Nginx.
The setting, which is removed here, prevented that.
We noticed the problem for the statshost location proxy
which sent metrics uncompressed since the upgrade.
On 15.09, compression was done by telegraf.

This is also fixed in NixOS unstable:

NixOS/nixpkgs#100708

 #PL-129611
@fooker fooker deleted the nginx-encoding branch Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants