Skip to content

Commit

Permalink
[KP] Fix Headers timeout issue (#81140)
Browse files Browse the repository at this point in the history
* temp fix request headers bug. issue 73849

* add a focused test

* move test to FTR

* adjust types

* some minors

* small adjustments
  • Loading branch information
mshustov authored Oct 22, 2020
1 parent 0823f00 commit 38a8063
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 7 deletions.
4 changes: 4 additions & 0 deletions src/core/server/http/http_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ interface ListenerOptions {
export function createServer(serverOptions: ServerOptions, listenerOptions: ListenerOptions) {
const server = new Server(serverOptions);

// remove fix + test as soon as update node.js to v12.19 https://github.com/elastic/kibana/pull/61587
server.listener.headersTimeout =
listenerOptions.keepaliveTimeout + 2 * server.listener.headersTimeout;

server.listener.keepAliveTimeout = listenerOptions.keepaliveTimeout;
server.listener.setTimeout(listenerOptions.socketTimeout);
server.listener.on('timeout', (socket) => {
Expand Down
1 change: 0 additions & 1 deletion src/core/server/http/integration_tests/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,6 @@ describe('KibanaRequest', () => {
expect(resp3.body).toEqual({ requestId: 'gamma' });
});
});

describe('request uuid', () => {
it('generates a UUID', async () => {
const { server: innerServer, createRouter } = await server.setup(setupDeps);
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/http/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ configService.atPath.mockReturnValue(
allowFromAnyIp: true,
ipAllowlist: [],
},
keepaliveTimeout: 120_000,
socketTimeout: 120_000,
} as any)
);

Expand Down
2 changes: 1 addition & 1 deletion tasks/config/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ module.exports = function () {
'--config',
'test/server_integration/http/ssl_redirect/config.js',
'--config',
'test/server_integration/http/cache/config.js',
'test/server_integration/http/platform/config.ts',
'--config',
'test/server_integration/http/ssl_with_p12/config.js',
'--config',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/

import { FtrProviderContext } from '../../services/types';
// eslint-disable-next-line import/no-default-export
export default function ({ getService }) {
export default function ({ getService }: FtrProviderContext) {
const supertest = getService('supertest');

describe('kibana server cache-control', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FtrConfigProviderContext } from '@kbn/test/types/ftr';

export default async function ({ readConfigFile }) {
// eslint-disable-next-line import/no-default-export
export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const httpConfig = await readConfigFile(require.resolve('../../config'));

return {
testFiles: [require.resolve('./')],
testFiles: [require.resolve('./cache'), require.resolve('./headers')],
services: httpConfig.get('services'),
servers: httpConfig.get('servers'),
junit: {
reportName: 'Http Cache-Control Integration Tests',
reportName: 'Kibana Platform Http Integration Tests',
},
esTestCluster: httpConfig.get('esTestCluster'),
kbnTestServer: httpConfig.get('kbnTestServer'),
Expand Down
82 changes: 82 additions & 0 deletions test/server_integration/http/platform/headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import Http from 'http';
import Url from 'url';
import { FtrProviderContext } from '../../services/types';

// @ts-ignore
import getUrl from '../../../../src/test_utils/get_url';

const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
const oneSec = 1_000;

// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const config = getService('config');

describe('headers timeout ', () => {
it('issue-73849', async () => {
const agent = new Http.Agent({
keepAlive: true,
});
const { protocol, hostname, port } = Url.parse(getUrl.baseUrl(config.get('servers.kibana')));

function performRequest() {
return new Promise((resolve, reject) => {
const req = Http.request(
{
protocol,
hostname,
port,
path: '/',
method: 'GET',
agent,
},
function (res) {
let data = '';
res.on('data', (chunk) => {
data += chunk;
});
res.on('end', () => resolve(data));
}
);

req.on('socket', (socket) => {
socket.write('GET / HTTP/1.1\r\n');
setTimeout(() => {
socket.write('Host: localhost\r\n');
}, oneSec);
setTimeout(() => {
// headersTimeout doesn't seem to fire if request headers are sent in one packet.
socket.write('\r\n');
req.end();
}, 2 * oneSec);
});

req.on('error', reject);
});
}

await performRequest();
const defaultHeadersTimeout = 40 * oneSec;
await delay(defaultHeadersTimeout + oneSec);
await performRequest();
});
});
}
27 changes: 27 additions & 0 deletions test/server_integration/services/types.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { GenericFtrProviderContext } from '@kbn/test/types/ftr';
import { services as kibanaCommonServices } from '../../common/services';
import { services as kibanaApiIntegrationServices } from '../../api_integration/services';

export type FtrProviderContext = GenericFtrProviderContext<
typeof kibanaCommonServices & { supertest: typeof kibanaApiIntegrationServices.supertest },
{}
>;

0 comments on commit 38a8063

Please sign in to comment.