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

Selectively disable compression based on the incoming request. Fixes #10377 #10378

Merged
merged 2 commits into from Jan 11, 2019

Conversation

Projects
None yet
3 participants
@georgyberdyshev
Copy link
Contributor

commented Dec 14, 2018

Hello @benjamn,

this a PR to fix #10377

Currently Auto-compress forces gzip compression, that cannot be disabled and therefore interferes with the possibility to enable a different Content-Encoding on a frontend-proxy.

Adding an environment variable to disable Auto-compress will allow to use Brotli compression through a front-end proxy, such as nginx, and provide better compression ratio than gzip.

METEOR_DISABLE_AUTO_COMPRESS=1

For more details, please see:
#10377

Thanks, Georgy

@benjamn

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

@georgyberdyshev We're open to this solution (since it will solve your use case), but I was just wondering if your nginx proxy is stripping/disabling the Accept-Encoding header before forwarding the request to the Meteor server? Ideally the Meteor server (i.e. the compress() middleware) would honor (the absence of) that header, rather than blindly compressing everything. Alternatively/additionally, we could configure the compression middleware with a filter that would selectively disable compression based on the incoming request.

@georgyberdyshev georgyberdyshev force-pushed the georgyberdyshev:auto-compress branch from bd72fa0 to 3d0ea31 Dec 21, 2018

@georgyberdyshev

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2018

Hello @benjamn,

thank you for your reply. Meteor behind the nginx proxy correctly receives the Accept-Encoding request HTTP header:

nginx log:
10.0.2.2 - - [21/Dec/2018:17:40:51 +0000] "GET /headers HTTP/2.0" 200 595 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.28 Safari/537.36" "-" "gzip, deflate, br"

Meteor:

{ 'x-forwarded-host': 'localhost',
  'x-forwarded-proto': 'http',
  'x-forwarded-port': '80',
  'accept-language': 'en-US,en;q=0.9',
  'accept-encoding': 'gzip, deflate, br',
  accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8',
  'user-agent': 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.28 Safari/537.36',
  'upgrade-insecure-requests': '1',
  'cache-control': 'max-age=0',
  'x-no-compression': 'x-no-compression',
  connection: 'close',
  host: 'localhost',
  'x-forwarded-for': '10.0.2.2,127.0.0.1' }

I have updated the PR to use the x-no-compression HTTP request header, as this will provide more granularity and also makes an additional environment variable for Meteor obsolete.

Thanks, Georgy

@georgyberdyshev georgyberdyshev changed the title Added METEOR_DISABLE_AUTO_COMPRESS environment variable. Fixes #10377 Selectively disable compression based on the incoming request. Fixes #10377 Dec 21, 2018

Selectively disable compression based on the incoming request.
Setting the x-no-compression request header disables compression.

@georgyberdyshev georgyberdyshev force-pushed the georgyberdyshev:auto-compress branch from 3d0ea31 to 0572ccd Dec 21, 2018

Update packages/webapp/webapp_server.js
Co-Authored-By: georgyberdyshev <georgyberdyshev@users.noreply.github.com>

@benjamn benjamn merged commit 5d43d2c into meteor:release-1.8.1 Jan 11, 2019

18 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci: Clean Up Your tests passed on CircleCI!
Details
ci/circleci: Docs Your tests passed on CircleCI!
Details
ci/circleci: Get Ready Your tests passed on CircleCI!
Details
ci/circleci: Isolated Tests Your tests passed on CircleCI!
Details
ci/circleci: Test Group 0 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 1 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 10 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 2 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 3 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 4 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 5 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 6 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 7 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 8 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 9 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@georgyberdyshev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2019

Thanks @benjamn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.