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

configurable global socket timeouts #31603

Merged
merged 15 commits into from
Jul 3, 2019
6 changes: 6 additions & 0 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ modify the landing page when opening Kibana. Supported on {ece}.
`server.host:`:: *Default: "localhost"* This setting specifies the host of the
back end server.

`server.keepaliveTimeout:`:: *Default: "120000"* The number of milliseconds to wait for additional data before restarting
the `server.socketTimeout` counter.

`server.maxPayloadBytes:`:: *Default: 1048576* The maximum payload size in bytes
for incoming server requests.

Expand All @@ -279,6 +282,9 @@ rewrite requests that are prefixed with `server.basePath` or require that they
are rewritten by your reverse proxy. This setting was effectively always `false`
before Kibana 6.3 and will default to `true` starting in Kibana 7.0.

`server.socketTimeout:`:: *Default: "120000"* The number of milliseconds to wait before closing an
inactive socket.

`server.ssl.certificate:` and `server.ssl.key:`:: Paths to the PEM-format SSL
certificate and SSL key files, respectively.

Expand Down
2 changes: 2 additions & 0 deletions src/core/server/http/__snapshots__/http_config.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions src/core/server/http/base_path_proxy_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { sample } from 'lodash';
import { DevConfig } from '../dev';
import { Logger } from '../logging';
import { HttpConfig } from './http_config';
import { createServer, getServerOptions } from './http_tools';
import { createServer, getListenerOptions, getServerOptions } from './http_tools';

const alphabet = 'abcdefghijklmnopqrztuvwxyz'.split('');

Expand Down Expand Up @@ -62,7 +62,8 @@ export class BasePathProxyServer {
this.log.debug('starting basepath proxy server');

const serverOptions = getServerOptions(this.httpConfig);
this.server = createServer(serverOptions);
const listenerOptions = getListenerOptions(this.httpConfig);
this.server = createServer(serverOptions, listenerOptions);

// Register hapi plugin that adds proxying functionality. It can be configured
// through the route configuration object (see { handler: { proxy: ... } }).
Expand Down
10 changes: 10 additions & 0 deletions src/core/server/http/http_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,16 @@ describe('with TLS', () => {
expect(configValue.ssl.certificateAuthorities).toBe('/authority/');
});

test('can specify socket timeouts', () => {
const obj = {
keepaliveTimeout: 1e5,
socketTimeout: 5e5,
};
const { keepaliveTimeout, socketTimeout } = config.schema.validate(obj);
expect(keepaliveTimeout).toBe(1e5);
expect(socketTimeout).toBe(5e5);
});

test('can specify several `certificateAuthorities`', () => {
const obj = {
ssl: {
Expand Down
10 changes: 10 additions & 0 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ export const config = {
}),
rewriteBasePath: schema.boolean({ defaultValue: false }),
ssl: sslSchema,
keepaliveTimeout: schema.number({
defaultValue: 120000,
}),
socketTimeout: schema.number({
defaultValue: 120000,
}),
},
{
validate: rawConfig => {
Expand Down Expand Up @@ -90,6 +96,8 @@ export type HttpConfigType = TypeOf<typeof config.schema>;
export class HttpConfig {
public autoListen: boolean;
public host: string;
public keepaliveTimeout: number;
public socketTimeout: number;
public port: number;
public cors: boolean | { origin: string[] };
public maxPayload: ByteSizeValue;
Expand All @@ -108,6 +116,8 @@ export class HttpConfig {
this.cors = rawConfig.cors;
this.maxPayload = rawConfig.maxPayload;
this.basePath = rawConfig.basePath;
this.keepaliveTimeout = rawConfig.keepaliveTimeout;
this.socketTimeout = rawConfig.socketTimeout;
this.rewriteBasePath = rawConfig.rewriteBasePath;
this.publicDir = env.staticFilesDir;
this.ssl = new SslConfig(rawConfig.ssl || {});
Expand Down
5 changes: 3 additions & 2 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Request, Server } from 'hapi';

import { Logger, LoggerFactory } from '../logging';
import { HttpConfig } from './http_config';
import { createServer, getServerOptions } from './http_tools';
import { createServer, getListenerOptions, getServerOptions } from './http_tools';
import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth';
import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_post_auth';
import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth';
Expand Down Expand Up @@ -108,7 +108,8 @@ export class HttpServer {

public setup(config: HttpConfig): HttpServerSetup {
const serverOptions = getServerOptions(config);
this.server = createServer(serverOptions);
const listenerOptions = getListenerOptions(config);
this.server = createServer(serverOptions, listenerOptions);
this.config = config;

const basePathService = new BasePath(config.basePath);
Expand Down
42 changes: 42 additions & 0 deletions src/core/server/http/http_tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,16 @@
* under the License.
*/

import supertest from 'supertest';
import { Request, ResponseToolkit } from 'hapi';
import Joi from 'joi';

import { defaultValidationErrorHandler, HapiValidationError } from './http_tools';
import { HttpServer } from './http_server';
import { HttpConfig } from './http_config';
import { Router } from './router';
import { loggingServiceMock } from '../logging/logging_service.mock';
import { ByteSizeValue } from '@kbn/config-schema';

const emptyOutput = {
statusCode: 400,
Expand Down Expand Up @@ -57,3 +64,38 @@ describe('defaultValidationErrorHandler', () => {
}
});
});

describe('timeouts', () => {
const logger = loggingServiceMock.create();
const server = new HttpServer(logger, 'foo');

test('returns 408 on timeout error', async () => {
const router = new Router('');
router.get({ path: '/a', validate: false }, async (req, res) => {
await new Promise(resolve => setTimeout(resolve, 2000));
return res.ok({});
});
router.get({ path: '/b', validate: false }, (req, res) => res.ok({}));

const { registerRouter, server: innerServer } = await server.setup({
socketTimeout: 1000,
host: '127.0.0.1',
maxPayload: new ByteSizeValue(1024),
ssl: {},
} as HttpConfig);
registerRouter(router);

await server.start();

await supertest(innerServer.listener)
.get('/a')
.expect(408);
await supertest(innerServer.listener)
.get('/b')
.expect(200);
});

afterAll(async () => {
await server.stop();
});
});
27 changes: 23 additions & 4 deletions src/core/server/http/http_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,30 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
return options;
}

export function createServer(options: ServerOptions) {
const server = new Server(options);
export function getListenerOptions(config: HttpConfig) {
return {
keepaliveTimeout: config.keepaliveTimeout,
socketTimeout: config.socketTimeout,
};
}

interface ListenerOptions {
keepaliveTimeout: number;
socketTimeout: number;
}

export function createServer(serverOptions: ServerOptions, listenerOptions: ListenerOptions) {
const server = new Server(serverOptions);

// Revert to previous 120 seconds keep-alive timeout in Node < 8.
server.listener.keepAliveTimeout = 120e3;
server.listener.keepAliveTimeout = listenerOptions.keepaliveTimeout;
server.listener.setTimeout(listenerOptions.socketTimeout);
server.listener.on('timeout', socket => {
if (socket.writable) {
socket.end(Buffer.from('HTTP/1.1 408 Request Timeout\r\n\r\n', 'ascii'));
Copy link
Contributor

Choose a reason for hiding this comment

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

adding a test for timeout shouldn't be a problem. in http_server.test.ts:

const delay = (ms: number) => new Promise(res => setTimeout(res, ms));
test('returns 408 on timeout error', async () => {
  const router = new Router('');

  router.get({ path: '/', validate: false }, async (req, res) => {
    await delay(1000);
    return res.ok({});
  });

  const { registerRouter, server: innerServer } = await server.setup({
    ...config,
    socketTimeout: 1,
  });
  registerRouter(router);

  await server.start();

  await supertest(innerServer.listener).get('/').expect(408)
});

Copy link
Member Author

Choose a reason for hiding this comment

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

added!

} else {
socket.destroy();
}
});
server.listener.on('clientError', (err, socket) => {
if (socket.writable) {
socket.end(Buffer.from('HTTP/1.1 400 Bad Request\r\n\r\n', 'ascii'));
Expand Down
13 changes: 8 additions & 5 deletions src/core/server/http/https_redirect_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { format as formatUrl } from 'url';

import { Logger } from '../logging';
import { HttpConfig } from './http_config';
import { createServer, getServerOptions } from './http_tools';
import { createServer, getListenerOptions, getServerOptions } from './http_tools';

export class HttpsRedirectServer {
private server?: Server;
Expand All @@ -42,10 +42,13 @@ export class HttpsRedirectServer {
// Redirect server is configured in the same way as any other HTTP server
// within the platform with the only exception that it should always be a
// plain HTTP server, so we just ignore `tls` part of options.
this.server = createServer({
...getServerOptions(config, { configureTLS: false }),
port: config.ssl.redirectHttpFromPort,
});
this.server = createServer(
{
...getServerOptions(config, { configureTLS: false }),
port: config.ssl.redirectHttpFromPort,
},
getListenerOptions(config)
);

this.server.ext('onRequest', (request: Request, responseToolkit: ResponseToolkit) => {
return responseToolkit
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ describe('#get', () => {
cors: false,
host: 'host',
maxPayloadBytes: 1000,
keepaliveTimeout: 5000,
socketTimeout: 2000,
port: 1234,
rewriteBasePath: false,
ssl: { enabled: true, keyPassphrase: 'some-phrase', someNewValue: 'new' },
Expand All @@ -83,6 +85,8 @@ describe('#get', () => {
cors: false,
host: 'host',
maxPayloadBytes: 1000,
keepaliveTimeout: 5000,
socketTimeout: 2000,
port: 1234,
rewriteBasePath: false,
ssl: { enabled: false, certificate: 'cert', key: 'key' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ export class LegacyObjectToConfigAdapter extends ObjectToConfigAdapter {
port: configValue.port,
rewriteBasePath: configValue.rewriteBasePath,
ssl: configValue.ssl,
keepaliveTimeout: configValue.keepaliveTimeout,
socketTimeout: configValue.socketTimeout,
};
}

Expand Down
2 changes: 2 additions & 0 deletions src/legacy/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export default () => Joi.object({
name: Joi.string().default(os.hostname()),
host: Joi.string().hostname().default('localhost'),
port: Joi.number().default(5601),
keepaliveTimeout: Joi.number().default(120000),
socketTimeout: Joi.number().default(120000),
maxPayloadBytes: Joi.number().default(1048576),
autoListen: Joi.boolean().default(true),
defaultRoute: Joi.string().default('/app/kibana').regex(/^\//, `start with a slash`),
Expand Down