revert configurable global socket timeouts (#31603) (#41837)

elastic · Jul 24, 2019 · f027fad · f027fad
1 parent 29fa09b
commit f027fad
Show file tree

Hide file tree

Showing 13 changed files with 13 additions and 127 deletions.
diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc
@@ -209,9 +209,6 @@ Specify the position of the subdomain the URL with the token `{s}`.
 
 `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.
 
 `server.name:`:: *Default: "your-hostname"* A human-readable display name that identifies this Kibana instance.
@@ -220,9 +217,6 @@ the `server.socketTimeout` counter.
 
 `server.ssl.enabled:`:: *Default: "false"* Enables SSL for outgoing requests from the Kibana server to the browser. When set to `true`, `server.ssl.certificate` and `server.ssl.key` are required
 
-`server.socketTimeout:`:: *Default: "120000"* The number of milliseconds to wait before closing an
-inactive socket.
-
 `server.ssl.cert:`:: Path to the PEM-format SSL certificate. This file enables
 SSL for outgoing requests from the Kibana server to the browser.
 deprecated[5.3.0,Replaced by `server.ssl.certificate`]

diff --git a/src/core/server/http/__snapshots__/http_config.test.ts.snap b/src/core/server/http/__snapshots__/http_config.test.ts.snap
@@ -14,13 +14,11 @@ Object {
   "autoListen": true,
   "cors": false,
   "host": "localhost",
-  "keepaliveTimeout": 120000,
   "maxPayload": ByteSizeValue {
     "valueInBytes": 1048576,
   },
   "port": 5601,
   "rewriteBasePath": false,
-  "socketTimeout": 120000,
   "ssl": Object {
     "cipherSuites": Array [
       "ECDHE-RSA-AES128-GCM-SHA256",

diff --git a/src/core/server/http/base_path_proxy_server.ts b/src/core/server/http/base_path_proxy_server.ts
@@ -24,7 +24,7 @@ import { sample } from 'lodash';
 import { DevConfig } from '../dev';
 import { Logger } from '../logging';
 import { HttpConfig } from './http_config';
-import { createServer, getListenerOptions, getServerOptions } from './http_tools';
+import { createServer, getServerOptions } from './http_tools';
 
 const alphabet = 'abcdefghijklmnopqrztuvwxyz'.split('');
 
@@ -62,8 +62,7 @@ export class BasePathProxyServer {
     this.log.debug('starting basepath proxy server');
 
     const serverOptions = getServerOptions(this.httpConfig);
-    const listenerOptions = getListenerOptions(this.httpConfig);
-    this.server = createServer(serverOptions, listenerOptions);
+    this.server = createServer(serverOptions);
 
     // Register hapi plugin that adds proxying functionality. It can be configured
     // through the route configuration object (see { handler: { proxy: ... } }).

diff --git a/src/core/server/http/http_config.test.ts b/src/core/server/http/http_config.test.ts
@@ -127,16 +127,6 @@ describe('with TLS', () => {
     expect(config.ssl.certificateAuthorities).toBe('/authority/');
   });
 
-  test('can specify socket timeouts', () => {
-    const obj = {
-      keepaliveTimeout: 1e5,
-      socketTimeout: 5e5,
-    };
-    const { keepaliveTimeout, socketTimeout } = HttpConfig.schema.validate(obj);
-    expect(keepaliveTimeout).toBe(1e5);
-    expect(socketTimeout).toBe(5e5);
-  });
-
   test('can specify several `certificateAuthorities`', () => {
     const httpSchema = HttpConfig.schema;
     const obj = {

diff --git a/src/core/server/http/http_config.ts b/src/core/server/http/http_config.ts
@@ -61,12 +61,6 @@ const createHttpSchema = schema.object(
     }),
     rewriteBasePath: schema.boolean({ defaultValue: false }),
     ssl: SslConfig.schema,
-    keepaliveTimeout: schema.number({
-      defaultValue: 120000,
-    }),
-    socketTimeout: schema.number({
-      defaultValue: 120000,
-    }),
   },
   {
     validate: config => {
@@ -99,8 +93,6 @@ 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;
@@ -121,8 +113,6 @@ export class HttpConfig {
     this.basePath = config.basePath;
     this.rewriteBasePath = config.rewriteBasePath;
     this.publicDir = env.staticFilesDir;
-    this.keepaliveTimeout = config.keepaliveTimeout;
-    this.socketTimeout = config.socketTimeout;
     this.ssl = new SslConfig(config.ssl);
   }
 }
diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts
@@ -22,7 +22,7 @@ import { Server, ServerOptions } from 'hapi';
 import { modifyUrl } from '../../utils';
 import { Logger } from '../logging';
 import { HttpConfig } from './http_config';
-import { createServer, getListenerOptions, getServerOptions } from './http_tools';
+import { createServer, getServerOptions } from './http_tools';
 import { Router } from './router';
 
 export interface HttpServerInfo {
@@ -52,8 +52,7 @@ export class HttpServer {
     this.log.debug('starting http server');
 
     const serverOptions = getServerOptions(config);
-    const listenerOptions = getListenerOptions(config);
-    this.server = createServer(serverOptions, listenerOptions);
+    this.server = createServer(serverOptions);
 
     this.setupBasePathRewrite(this.server, config);
 

diff --git a/src/core/server/http/http_tools.test.ts b/src/core/server/http/http_tools.test.ts
@@ -19,15 +19,7 @@
 
 import { Request, ResponseToolkit } from 'hapi';
 import Joi from 'joi';
-import supertest from 'supertest';
-
-import { logger } from '../logging/__mocks__';
-
-import { ByteSizeValue } from '@kbn/config-schema';
-import { HttpConfig } from './http_config';
-import { HttpServer } from './http_server';
 import { defaultValidationErrorHandler, HapiValidationError } from './http_tools';
-import { Router } from './router';
 
 const emptyOutput = {
   statusCode: 400,
@@ -65,37 +57,3 @@ describe('defaultValidationErrorHandler', () => {
     }
   });
 });
-
-describe('timeouts', () => {
-  const server = new HttpServer(logger.get());
-
-  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 }, async (req, res) => res.ok({}));
-    server.registerRouter(router);
-
-    const config = {
-      socketTimeout: 1000,
-      host: '127.0.0.1',
-      maxPayload: new ByteSizeValue(1024),
-      ssl: {},
-    } as HttpConfig;
-
-    const { server: innerServer } = await server.start(config);
-    await supertest(innerServer.listener)
-      .get('/a')
-      .expect(408);
-    await supertest(innerServer.listener)
-      .get('/b')
-      .expect(200);
-  });
-
-  afterAll(async () => {
-    await server.stop();
-    logger.mockClear();
-  });
-});
diff --git a/src/core/server/http/http_tools.ts b/src/core/server/http/http_tools.ts
@@ -78,30 +78,11 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
   return 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);
+export function createServer(options: ServerOptions) {
+  const server = new Server(options);
 
-  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'));
-    } else {
-      socket.destroy();
-    }
-  });
+  // Revert to previous 120 seconds keep-alive timeout in Node < 8.
+  server.listener.keepAliveTimeout = 120e3;
   server.listener.on('clientError', (err, socket) => {
     if (socket.writable) {
       socket.end(Buffer.from('HTTP/1.1 400 Bad Request\r\n\r\n', 'ascii'));

diff --git a/src/core/server/http/https_redirect_server.ts b/src/core/server/http/https_redirect_server.ts
@@ -22,7 +22,7 @@ import { format as formatUrl } from 'url';
 
 import { Logger } from '../logging';
 import { HttpConfig } from './http_config';
-import { createServer, getListenerOptions, getServerOptions } from './http_tools';
+import { createServer, getServerOptions } from './http_tools';
 
 export class HttpsRedirectServer {
   private server?: Server;
@@ -42,13 +42,10 @@ 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,
-      },
-      getListenerOptions(config)
-    );
+    this.server = createServer({
+      ...getServerOptions(config, { configureTLS: false }),
+      port: config.ssl.redirectHttpFromPort,
+    });
 
     this.server.ext('onRequest', (request: Request, responseToolkit: ResponseToolkit) => {
       return responseToolkit

diff --git a/...re/server/legacy_compat/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap b/...re/server/legacy_compat/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap
@@ -6,11 +6,9 @@ Object {
   "basePath": "/abc",
   "cors": false,
   "host": "host",
-  "keepaliveTimeout": 5000,
   "maxPayload": 1000,
   "port": 1234,
   "rewriteBasePath": false,
-  "socketTimeout": 2000,
   "ssl": Object {
     "enabled": true,
     "keyPassphrase": "some-phrase",
@@ -25,11 +23,9 @@ Object {
   "basePath": "/abc",
   "cors": false,
   "host": "host",
-  "keepaliveTimeout": 5000,
   "maxPayload": 1000,
   "port": 1234,
   "rewriteBasePath": false,
-  "socketTimeout": 2000,
   "ssl": Object {
     "certificate": "cert",
     "enabled": true,
@@ -44,11 +40,9 @@ Object {
   "basePath": "/abc",
   "cors": false,
   "host": "host",
-  "keepaliveTimeout": 5000,
   "maxPayload": 1000,
   "port": 1234,
   "rewriteBasePath": false,
-  "socketTimeout": 2000,
   "ssl": Object {
     "certificate": "deprecated-cert",
     "enabled": true,
@@ -63,11 +57,9 @@ Object {
   "basePath": "/abc",
   "cors": false,
   "host": "host",
-  "keepaliveTimeout": 5000,
   "maxPayload": 1000,
   "port": 1234,
   "rewriteBasePath": false,
-  "socketTimeout": 2000,
   "ssl": Object {
     "certificate": "cert",
     "enabled": false,

diff --git a/src/core/server/legacy_compat/config/legacy_object_to_config_adapter.test.ts b/src/core/server/legacy_compat/config/legacy_object_to_config_adapter.test.ts
@@ -70,8 +70,6 @@ describe('#get', () => {
         host: 'host',
         maxPayloadBytes: 1000,
         port: 1234,
-        keepaliveTimeout: 5000,
-        socketTimeout: 2000,
         rewriteBasePath: false,
         ssl: { enabled: true, keyPassphrase: 'some-phrase', someNewValue: 'new' },
         someNotSupportedValue: 'val',
@@ -86,8 +84,6 @@ describe('#get', () => {
         host: 'host',
         maxPayloadBytes: 1000,
         port: 1234,
-        keepaliveTimeout: 5000,
-        socketTimeout: 2000,
         rewriteBasePath: false,
         ssl: { enabled: false, certificate: 'cert', key: 'key' },
         someNotSupportedValue: 'val',
@@ -102,8 +98,6 @@ describe('#get', () => {
         host: 'host',
         maxPayloadBytes: 1000,
         port: 1234,
-        keepaliveTimeout: 5000,
-        socketTimeout: 2000,
         rewriteBasePath: false,
         ssl: { enabled: true, cert: 'deprecated-cert', key: 'key' },
         someNotSupportedValue: 'val',
@@ -118,8 +112,6 @@ describe('#get', () => {
         host: 'host',
         maxPayloadBytes: 1000,
         port: 1234,
-        keepaliveTimeout: 5000,
-        socketTimeout: 2000,
         rewriteBasePath: false,
         ssl: { certificate: 'cert', key: 'key' },
         someNotSupportedValue: 'val',

diff --git a/src/core/server/legacy_compat/config/legacy_object_to_config_adapter.ts b/src/core/server/legacy_compat/config/legacy_object_to_config_adapter.ts
@@ -66,8 +66,6 @@ export class LegacyObjectToConfigAdapter extends ObjectToConfigAdapter {
       host: configValue.host,
       maxPayload: configValue.maxPayloadBytes,
       port: configValue.port,
-      keepaliveTimeout: configValue.keepaliveTimeout,
-      socketTimeout: configValue.socketTimeout,
       rewriteBasePath: configValue.rewriteBasePath,
       ssl: configValue.ssl && LegacyObjectToConfigAdapter.transformSSL(configValue.ssl),
     };

diff --git a/src/server/config/schema.js b/src/server/config/schema.js
@@ -119,8 +119,6 @@ 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`),