Skip to content
39 changes: 39 additions & 0 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,45 @@ test('exposes route details of incoming request to a route handler', async () =>
});
});

describe('conditional compression', () => {
test('disables compression when there is a referer', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);

const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) =>
// we need the large body here so that compression would normally be used
res.ok({ body: 'hello'.repeat(500), headers: { 'Content-Type': 'text/html; charset=UTF-8' } })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we add a comment why we make such a long body?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, great idea. I will make it so.

);
registerRouter(router);

await server.start();
const response = await supertest(innerServer.listener)
.get('/')
.set('accept-encoding', 'gzip')
.set('referer', 'http://some-other-site/');

expect(response.header).not.toHaveProperty('content-encoding');
});

test(`enables compression when there isn't a referer`, async () => {
const { registerRouter, server: innerServer } = await server.setup(config);

const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) =>
// we need the large body here so that compression will be used
res.ok({ body: 'hello'.repeat(500), headers: { 'Content-Type': 'text/html; charset=UTF-8' } })
);
registerRouter(router);

await server.start();
const response = await supertest(innerServer.listener)
.get('/')
.set('accept-encoding', 'gzip');

expect(response.header).toHaveProperty('content-encoding', 'gzip');
});
});

describe('setup contract', () => {
describe('#createSessionStorage', () => {
it('creates session storage factory', async () => {
Expand Down
18 changes: 18 additions & 0 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export class HttpServer {

const basePathService = new BasePath(config.basePath);
this.setupBasePathRewrite(config, basePathService);
this.setupConditionalCompression();

return {
registerRouter: this.registerRouter.bind(this),
Expand Down Expand Up @@ -175,6 +176,23 @@ export class HttpServer {
});
}

private setupConditionalCompression() {
if (this.server === undefined) {
throw new Error('Server is not created yet');
}

this.server.ext('onRequest', (request, h) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to use registerOnPreAuth here because it doesn't expose the info. I'm assuming this generally isn't something we want to support, so I'm using the HapiJS interfaces directly. If you all think we should modify the KibanaRequest to include this, just let me know.

// whenever there is a referrer, don't use compression even if the client supports it
if (request.info.referrer !== '') {
this.log.debug(
`Not using compression because there is a referer: ${request.info.referrer}`
);
request.info.acceptEncoding = '';
}
return h.continue;
});
}

private registerOnPostAuth(fn: OnPostAuthHandler) {
if (this.server === undefined) {
throw new Error('Server is not created yet');
Expand Down
46 changes: 35 additions & 11 deletions test/api_integration/apis/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,45 @@
* specific language governing permissions and limitations
* under the License.
*/
import expect from '@kbn/expect';

export default function ({ getService }) {
const supertest = getService('supertest');

describe('core request context', () => {
it('provides access to elasticsearch', async () => (
await supertest
.get('/requestcontext/elasticsearch')
.expect(200, 'Elasticsearch: true')
));
describe('core', () => {
describe('request context', () => {
it('provides access to elasticsearch', async () => (
await supertest
.get('/requestcontext/elasticsearch')
.expect(200, 'Elasticsearch: true')
));

it('provides access to SavedObjects client', async () => (
await supertest
.get('/requestcontext/savedobjectsclient')
.expect(200, 'SavedObjects client: {"page":1,"per_page":20,"total":0,"saved_objects":[]}')
));
it('provides access to SavedObjects client', async () => (
await supertest
.get('/requestcontext/savedobjectsclient')
.expect(200, 'SavedObjects client: {"page":1,"per_page":20,"total":0,"saved_objects":[]}')
));
});

describe('compression', () => {
it(`uses compression when there isn't a referer`, async () => {
await supertest
.get('/app/kibana')
.set('accept-encoding', 'gzip')
.then(response => {
expect(response.headers).to.have.property('content-encoding', 'gzip');
});
});

it(`doesn't use compression when there is a referer`, async () => {
await supertest
.get('/app/kibana')
.set('accept-encoding', 'gzip')
.set('referer', 'https://www.google.com')
.then(response => {
expect(response.headers).not.to.have.property('content-encoding');
});
});
});
});
}
1 change: 1 addition & 0 deletions test/api_integration/apis/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@ export default function ({ loadTestFile }) {
loadTestFile(require.resolve('./status'));
loadTestFile(require.resolve('./stats'));
loadTestFile(require.resolve('./ui_metric'));
loadTestFile(require.resolve('./core'));
});
}