From 8b0145c3a11b94a8638f518d291097ee0550ccee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= Date: Mon, 24 Oct 2022 15:33:21 +0200 Subject: [PATCH] Support brotli compression on the server side (#142334) * Use brotli compression * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * Add integration test for brotli support * Use import instead of require() * Suppress build error on importing brok * [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' * add brok as explicit package dep * add `server.compression.brotli` config settings * update documentation * fix test utils * fix more test configs * add tests for endpoints too * remove against endpoint for now Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: pgayvallet --- docs/setup/settings.asciidoc | 5 +++ package.json | 1 + .../core-http-server-internal/BUILD.bazel | 2 ++ .../__snapshots__/http_config.test.ts.snap | 4 +++ .../src/http_config.test.ts | 27 ++++++++++++++++ .../src/http_config.ts | 10 +++++- .../src/http_server.test.ts | 32 +++++++++++++++++-- .../src/http_server.ts | 12 ++++++- .../core-http-server-mocks/src/test_utils.ts | 2 +- .../http/cookie_session_storage.test.ts | 2 +- .../http/http_server.test.ts | 2 +- .../http/lifecycle_handlers.test.ts | 2 +- test/api_integration/apis/core/compression.ts | 23 ++++++++++--- test/api_integration/config.js | 1 + yarn.lock | 10 +++++- 15 files changed, 121 insertions(+), 14 deletions(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 69316af1593a1..c32d305b22d9a 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -386,6 +386,11 @@ Specifies an array of trusted hostnames, such as the {kib} host, or a reverse proxy sitting in front of it. This determines whether HTTP compression may be used for responses, based on the request `Referer` header. This setting may not be used when <> is set to `false`. *Default: `none`* +`server.compression.brotli.enabled`:: +Set to `true` to enable brotli (br) compression format. +Note: browsers not supporting brotli compression will fallback to using gzip instead. +This setting may not be used when <> is set to `false`. *Default: `false`* + [[server-securityResponseHeaders-strictTransportSecurity]] `server.securityResponseHeaders.strictTransportSecurity`:: Controls whether the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security[`Strict-Transport-Security`] header is used in all responses to the client from the {kib} server, and specifies what value is used. Allowed values are any text value or diff --git a/package.json b/package.json index 452cc5c7dca6e..8a71f04064b8a 100644 --- a/package.json +++ b/package.json @@ -455,6 +455,7 @@ "bitmap-sdf": "^1.0.3", "blurhash": "^2.0.1", "brace": "0.11.1", + "brok": "^5.0.2", "byte-size": "^8.1.0", "canvg": "^3.0.9", "cbor-x": "^1.3.3", diff --git a/packages/core/http/core-http-server-internal/BUILD.bazel b/packages/core/http/core-http-server-internal/BUILD.bazel index ab10546a3ddcc..214bb5833b7a9 100644 --- a/packages/core/http/core-http-server-internal/BUILD.bazel +++ b/packages/core/http/core-http-server-internal/BUILD.bazel @@ -44,6 +44,7 @@ RUNTIME_DEPS = [ "@npm//@hapi/cookie", "@npm//@hapi/inert", "@npm//elastic-apm-node", + "@npm//brok", "//packages/kbn-utils", "//packages/kbn-std", "//packages/kbn-config-schema", @@ -68,6 +69,7 @@ TYPES_DEPS = [ "@npm//moment", "@npm//@elastic/numeral", "@npm//lodash", + "@npm//brok", "@npm//@hapi/hapi", "@npm//@hapi/boom", "@npm//@hapi/cookie", diff --git a/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap b/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap index 65ac08f6ce5f7..bb81f7b3bc924 100644 --- a/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap +++ b/packages/core/http/core-http-server-internal/src/__snapshots__/http_config.test.ts.snap @@ -42,6 +42,10 @@ exports[`has defaults for config 1`] = ` Object { "autoListen": true, "compression": Object { + "brotli": Object { + "enabled": false, + "quality": 3, + }, "enabled": true, }, "cors": Object { diff --git a/packages/core/http/core-http-server-internal/src/http_config.test.ts b/packages/core/http/core-http-server-internal/src/http_config.test.ts index 26a42f27a794b..ec9fc41ed02fd 100644 --- a/packages/core/http/core-http-server-internal/src/http_config.test.ts +++ b/packages/core/http/core-http-server-internal/src/http_config.test.ts @@ -390,6 +390,33 @@ describe('with compression', () => { }); }); +describe('compression.brotli', () => { + describe('enabled', () => { + it('defaults to `false`', () => { + expect(config.schema.validate({}).compression.brotli.enabled).toEqual(false); + }); + }); + describe('quality', () => { + it('defaults to `3`', () => { + expect(config.schema.validate({}).compression.brotli.quality).toEqual(3); + }); + it('does not accepts value superior to `11`', () => { + expect(() => + config.schema.validate({ compression: { brotli: { quality: 12 } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[compression.brotli.quality]: Value must be equal to or lower than [11]."` + ); + }); + it('does not accepts value inferior to `0`', () => { + expect(() => + config.schema.validate({ compression: { brotli: { quality: -1 } } }) + ).toThrowErrorMatchingInlineSnapshot( + `"[compression.brotli.quality]: Value must be equal to or greater than [0]."` + ); + }); + }); +}); + describe('cors', () => { describe('allowOrigin', () => { it('list cannot be empty', () => { diff --git a/packages/core/http/core-http-server-internal/src/http_config.ts b/packages/core/http/core-http-server-internal/src/http_config.ts index 9cb636156c5e8..1fae2568edffd 100644 --- a/packages/core/http/core-http-server-internal/src/http_config.ts +++ b/packages/core/http/core-http-server-internal/src/http_config.ts @@ -112,6 +112,10 @@ const configSchema = schema.object( }), compression: schema.object({ enabled: schema.boolean({ defaultValue: true }), + brotli: schema.object({ + enabled: schema.boolean({ defaultValue: false }), + quality: schema.number({ defaultValue: 3, min: 0, max: 11 }), + }), referrerWhitelist: schema.maybe( schema.arrayOf( schema.string({ @@ -209,7 +213,11 @@ export class HttpConfig implements IHttpConfig { public publicBaseUrl?: string; public rewriteBasePath: boolean; public ssl: SslConfig; - public compression: { enabled: boolean; referrerWhitelist?: string[] }; + public compression: { + enabled: boolean; + referrerWhitelist?: string[]; + brotli: { enabled: boolean; quality: number }; + }; public csp: ICspConfig; public externalUrl: IExternalUrlConfig; public xsrf: { disableProtection: boolean; allowlist: string[] }; diff --git a/packages/core/http/core-http-server-internal/src/http_server.test.ts b/packages/core/http/core-http-server-internal/src/http_server.test.ts index 82debfa44c2cb..92fa63c502558 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.test.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.test.ts @@ -60,7 +60,7 @@ beforeEach(() => { maxPayload: new ByteSizeValue(1024), port: 10002, ssl: { enabled: false }, - compression: { enabled: true }, + compression: { enabled: true, brotli: { enabled: false, quality: 3 } }, requestId: { allowFromAnyIp: true, ipAllowlist: [], @@ -865,7 +865,7 @@ describe('conditional compression', () => { test('with `compression.enabled: false`', async () => { const listener = await setupServer({ ...config, - compression: { enabled: false }, + compression: { enabled: false, brotli: { enabled: false, quality: 3 } }, }); const response = await supertest(listener).get('/').set('accept-encoding', 'gzip'); @@ -873,12 +873,38 @@ describe('conditional compression', () => { expect(response.header).not.toHaveProperty('content-encoding'); }); + test('with `compression.brotli.enabled: false`', async () => { + const listener = await setupServer({ + ...config, + compression: { enabled: true, brotli: { enabled: false, quality: 3 } }, + }); + + const response = await supertest(listener).get('/').set('accept-encoding', 'br'); + + expect(response.header).not.toHaveProperty('content-encoding', 'br'); + }); + + test('with `compression.brotli.enabled: true`', async () => { + const listener = await setupServer({ + ...config, + compression: { enabled: true, brotli: { enabled: true, quality: 3 } }, + }); + + const response = await supertest(listener).get('/').set('accept-encoding', 'br'); + + expect(response.header).toHaveProperty('content-encoding', 'br'); + }); + describe('with defined `compression.referrerWhitelist`', () => { let listener: Server; beforeEach(async () => { listener = await setupServer({ ...config, - compression: { enabled: true, referrerWhitelist: ['foo'] }, + compression: { + enabled: true, + referrerWhitelist: ['foo'], + brotli: { enabled: false, quality: 3 }, + }, }); }); diff --git a/packages/core/http/core-http-server-internal/src/http_server.ts b/packages/core/http/core-http-server-internal/src/http_server.ts index 766fa131349e1..4e4bf17d7a17a 100644 --- a/packages/core/http/core-http-server-internal/src/http_server.ts +++ b/packages/core/http/core-http-server-internal/src/http_server.ts @@ -21,6 +21,8 @@ import type { Duration } from 'moment'; import { firstValueFrom, Observable } from 'rxjs'; import { take } from 'rxjs/operators'; import apm from 'elastic-apm-node'; +// @ts-expect-error no type definition +import Brok from 'brok'; import type { Logger, LoggerFactory } from '@kbn/logging'; import type { InternalExecutionContextSetup } from '@kbn/core-execution-context-server-internal'; import { isSafeMethod } from '@kbn/core-http-router-server-internal'; @@ -147,9 +149,17 @@ export class HttpServer { ): Promise { const serverOptions = getServerOptions(config); const listenerOptions = getListenerOptions(config); + this.config = config; this.server = createServer(serverOptions, listenerOptions); await this.server.register([HapiStaticFiles]); - this.config = config; + if (config.compression.brotli.enabled) { + await this.server.register({ + plugin: Brok, + options: { + compress: { quality: config.compression.brotli.quality }, + }, + }); + } // It's important to have setupRequestStateAssignment call the very first, otherwise context passing will be broken. // That's the only reason why context initialization exists in this method. diff --git a/packages/core/http/core-http-server-mocks/src/test_utils.ts b/packages/core/http/core-http-server-mocks/src/test_utils.ts index 2b9658693dce7..bb260ae23c908 100644 --- a/packages/core/http/core-http-server-mocks/src/test_utils.ts +++ b/packages/core/http/core-http-server-mocks/src/test_utils.ts @@ -35,7 +35,7 @@ const createConfigService = () => { cors: { enabled: false, }, - compression: { enabled: true }, + compression: { enabled: true, brotli: { enabled: false } }, xsrf: { disableProtection: true, allowlist: [], diff --git a/src/core/server/integration_tests/http/cookie_session_storage.test.ts b/src/core/server/integration_tests/http/cookie_session_storage.test.ts index 713ed2dc9edfd..1041ed66872dd 100644 --- a/src/core/server/integration_tests/http/cookie_session_storage.test.ts +++ b/src/core/server/integration_tests/http/cookie_session_storage.test.ts @@ -53,7 +53,7 @@ configService.atPath.mockImplementation((path) => { ssl: { verificationMode: 'none', }, - compression: { enabled: true }, + compression: { enabled: true, brotli: { enabled: false } }, xsrf: { disableProtection: true, allowlist: [], diff --git a/src/core/server/integration_tests/http/http_server.test.ts b/src/core/server/integration_tests/http/http_server.test.ts index 1b9da1f0fddde..313421fa05eca 100644 --- a/src/core/server/integration_tests/http/http_server.test.ts +++ b/src/core/server/integration_tests/http/http_server.test.ts @@ -31,7 +31,7 @@ describe('Http server', () => { maxPayload: new ByteSizeValue(1024), port: 10002, ssl: { enabled: false }, - compression: { enabled: true }, + compression: { enabled: true, brotli: { enabled: false } }, requestId: { allowFromAnyIp: true, ipAllowlist: [], diff --git a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts index 6e72afd4f4e58..26c17a17b41bb 100644 --- a/src/core/server/integration_tests/http/lifecycle_handlers.test.ts +++ b/src/core/server/integration_tests/http/lifecycle_handlers.test.ts @@ -52,7 +52,7 @@ describe('core lifecycle handlers', () => { cors: { enabled: false, }, - compression: { enabled: true }, + compression: { enabled: true, brotli: { enabled: false } }, name: kibanaName, securityResponseHeaders: { // reflects default config diff --git a/test/api_integration/apis/core/compression.ts b/test/api_integration/apis/core/compression.ts index c175fe4b9862e..c4b119692f4bb 100644 --- a/test/api_integration/apis/core/compression.ts +++ b/test/api_integration/apis/core/compression.ts @@ -12,10 +12,10 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); - describe('compression', () => { + const compressionSuite = (url: string) => { it(`uses compression when there isn't a referer`, async () => { await supertest - .get('/app/kibana') + .get(url) .set('accept-encoding', 'gzip') .then((response) => { expect(response.header).to.have.property('content-encoding', 'gzip'); @@ -24,7 +24,7 @@ export default function ({ getService }: FtrProviderContext) { it(`uses compression when there is a whitelisted referer`, async () => { await supertest - .get('/app/kibana') + .get(url) .set('accept-encoding', 'gzip') .set('referer', 'https://some-host.com') .then((response) => { @@ -34,12 +34,27 @@ export default function ({ getService }: FtrProviderContext) { it(`doesn't use compression when there is a non-whitelisted referer`, async () => { await supertest - .get('/app/kibana') + .get(url) .set('accept-encoding', 'gzip') .set('referer', 'https://other.some-host.com') .then((response) => { expect(response.header).not.to.have.property('content-encoding'); }); }); + + it(`supports brotli compression`, async () => { + await supertest + .get(url) + .set('accept-encoding', 'br') + .then((response) => { + expect(response.header).to.have.property('content-encoding', 'br'); + }); + }); + }; + + describe('compression', () => { + describe('against an application page', () => { + compressionSuite('/app/kibana'); + }); }); } diff --git a/test/api_integration/config.js b/test/api_integration/config.js index 7f3f4b45298d1..ce04be64bb36e 100644 --- a/test/api_integration/config.js +++ b/test/api_integration/config.js @@ -31,6 +31,7 @@ export default async function ({ readConfigFile }) { '--elasticsearch.healthCheck.delay=3600000', '--server.xsrf.disableProtection=true', '--server.compression.referrerWhitelist=["some-host.com"]', + '--server.compression.brotli.enabled=true', `--savedObjects.maxImportExportSize=10001`, '--savedObjects.maxImportPayloadBytes=30000000', // for testing set buffer duration to 0 to immediately flush counters into saved objects. diff --git a/yarn.lock b/yarn.lock index 00d479810e668..f97953886c2a3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2283,7 +2283,7 @@ dependencies: "@hapi/hoek" "^9.0.0" -"@hapi/validate@1.x.x", "@hapi/validate@^1.1.1": +"@hapi/validate@1.x.x", "@hapi/validate@^1.1.1", "@hapi/validate@^1.1.3": version "1.1.3" resolved "https://registry.yarnpkg.com/@hapi/validate/-/validate-1.1.3.tgz#f750a07283929e09b51aa16be34affb44e1931ad" integrity sha512-/XMR0N0wjw0Twzq2pQOzPBZlDzkekGcoCtzO314BpIEsbXdYGthQUbxgkGDf4nhk1+IPDAsXqWjMohRQYO06UA== @@ -10967,6 +10967,14 @@ brfs@^2.0.0, brfs@^2.0.2: static-module "^3.0.2" through2 "^2.0.0" +brok@^5.0.2: + version "5.0.2" + resolved "https://registry.yarnpkg.com/brok/-/brok-5.0.2.tgz#b77e7203ce89d30939a5b877a9bb3acb4dffc848" + integrity sha512-mqsoOGPjcP9oltC8dD4PnRCiJREmFg+ee588mVYZgZNd8YV5Zo6eOLv/fp6HxdYffaxvkKfPHjc+sRWIkuIu7A== + dependencies: + "@hapi/hoek" "^9.0.4" + "@hapi/validate" "^1.1.3" + brorand@^1.0.1, brorand@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f"