[6.4] Do not limit payload size for requests that are proxied to the …

…legacy Kibana. (#21952)

* Do not limit payload size for requests that are proxied to the legacy Kibana. (#21750)
* Adapt basepath tests to the 6.x behaviour.

src/core/server/http/http_server.ts

@@ -77,6 +77,9 @@ export class HttpServer {
             output: 'stream',
             parse: false,
             timeout: false,
+            // Having such a large value here will allow legacy routes to override
+            // maximum allowed payload size set in the core http server if needed.
+            maxBytes: Number.MAX_SAFE_INTEGER,
           },
         },
         path: '/{p*}',

src/server/base_path.test.js

@@ -20,32 +20,23 @@
 import * as kbnTestServer from '../test_utils/kbn_server';
 const basePath = '/kibana';
 
-describe('Server basePath config', function () {
+describe('Server basePath config', () => {
   let kbnServer;
-  beforeAll(async function () {
-    kbnServer = kbnTestServer.createServer({
-      server: { basePath }
-    });
+  beforeAll(async () => {
+    kbnServer = kbnTestServer.createServer({ server: { basePath } });
     await kbnServer.ready();
     return kbnServer;
   });
 
-  afterAll(async function () {
-    await kbnServer.close();
-  });
+  afterAll(async () => await kbnServer.close());
 
-  it('appends the basePath to root redirect', function (done) {
-    const options = {
+  test('appends the basePath to root redirect', async () => {
+    const resp = await kbnServer.inject({
       url: '/',
       method: 'GET'
-    };
-    kbnTestServer.makeRequest(kbnServer, options, function (res) {
-      try {
-        expect(res.payload).toMatch(/defaultRoute = '\/kibana\/app\/kibana'/);
-        done();
-      } catch (e) {
-        done(e);
-      }
     });
+
+    expect(resp).toHaveProperty('statusCode', 200);
+    expect(resp.payload).toMatch(/defaultRoute = '\/kibana\/app\/kibana'/);
   });
 });

src/server/http/__snapshots__/max_payload_size.test.js.snap

@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`fails with 400 if payload size is larger than default and route config allows 1`] = `"{\\"statusCode\\":400,\\"error\\":\\"Bad Request\\",\\"message\\":\\"Payload content length greater than maximum allowed: 200\\"}"`;

src/server/http/__snapshots__/xsrf.test.js.snap

@@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`xsrf request filter destructiveMethod: DELETE rejects requests without either an xsrf or version header: DELETE reject response 1`] = `"{\\"statusCode\\":400,\\"error\\":\\"Bad Request\\",\\"message\\":\\"Request must contain a kbn-xsrf header.\\"}"`;
+
+exports[`xsrf request filter destructiveMethod: POST rejects requests without either an xsrf or version header: POST reject response 1`] = `"{\\"statusCode\\":400,\\"error\\":\\"Bad Request\\",\\"message\\":\\"Request must contain a kbn-xsrf header.\\"}"`;
+
+exports[`xsrf request filter destructiveMethod: PUT rejects requests without either an xsrf or version header: PUT reject response 1`] = `"{\\"statusCode\\":400,\\"error\\":\\"Bad Request\\",\\"message\\":\\"Request must contain a kbn-xsrf header.\\"}"`;

src/server/http/max_payload_size.test.js

@@ -0,0 +1,70 @@
+/*
+ * 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 * as kbnTestServer from '../../test_utils/kbn_server';
+
+let kbnServer;
+async function makeServer({ maxPayloadBytesDefault, maxPayloadBytesRoute }) {
+  kbnServer = kbnTestServer.createServer({
+    server: { maxPayloadBytes: maxPayloadBytesDefault }
+  });
+
+  await kbnServer.ready();
+
+  kbnServer.server.route({
+    path: '/payload_size_check/test/route',
+    method: 'POST',
+    config: { payload: { maxBytes: maxPayloadBytesRoute } },
+    handler: function (req, reply) {
+      reply(null, req.payload.data.slice(0, 5));
+    }
+  });
+}
+
+async function makeRequest(opts) {
+  return await kbnTestServer.makeRequest(kbnServer, opts);
+}
+
+afterEach(async () => await kbnServer.close());
+
+test('accepts payload with a size larger than default but smaller than route config allows', async () => {
+  await makeServer({ maxPayloadBytesDefault: 100, maxPayloadBytesRoute: 200 });
+
+  const resp = await makeRequest({
+    url: '/payload_size_check/test/route',
+    method: 'POST',
+    payload: { data: Array(150).fill('+').join('') },
+  });
+
+  expect(resp.statusCode).toBe(200);
+  expect(resp.payload).toBe('+++++');
+});
+
+test('fails with 400 if payload size is larger than default and route config allows', async () => {
+  await makeServer({ maxPayloadBytesDefault: 100, maxPayloadBytesRoute: 200 });
+
+  const resp = await makeRequest({
+    url: '/payload_size_check/test/route',
+    method: 'POST',
+    payload: { data: Array(250).fill('+').join('') },
+  });
+
+  expect(resp.statusCode).toBe(400);
+  expect(resp.payload).toMatchSnapshot();
+});

src/server/http/version_check.test.js

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-import { fromNode } from 'bluebird';
 import { resolve } from 'path';
 import * as kbnTestServer from '../../test_utils/kbn_server';
 
@@ -27,12 +26,8 @@ const versionHeader = 'kbn-version';
 const version = require(src('../package.json')).version;
 
 describe('version_check request filter', function () {
-  function makeRequest(kbnServer, opts) {
-    return fromNode(cb => {
-      kbnTestServer.makeRequest(kbnServer, opts, (resp) => {
-        cb(null, resp);
-      });
-    });
+  async function makeRequest(kbnServer, opts) {
+    return await kbnTestServer.makeRequest(kbnServer, opts);
   }
 
   async function makeServer() {

src/server/http/xsrf.test.js

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-import { fromNode as fn } from 'bluebird';
 import { resolve } from 'path';
 import * as kbnTestServer from '../../test_utils/kbn_server';
 
@@ -31,12 +30,8 @@ const whitelistedTestPath = '/xsrf/test/route/whitelisted';
 const actualVersion = require(src('../package.json')).version;
 
 describe('xsrf request filter', function () {
-  function inject(kbnServer, opts) {
-    return fn(cb => {
-      kbnTestServer.makeRequest(kbnServer, opts, (resp) => {
-        cb(null, resp);
-      });
-    });
+  async function inject(kbnServer, opts) {
+    return await kbnTestServer.makeRequest(kbnServer, opts);
   }
 
   const makeServer = async function () {
@@ -186,7 +181,7 @@ describe('xsrf request filter', function () {
         });
 
         expect(resp.statusCode).toBe(400);
-        expect(resp.result.message).toBe('Request must contain a kbn-xsrf header.');
+        expect(resp.result).toMatchSnapshot(`${method} reject response`);
       });
 
       it('accepts whitelisted requests without either an xsrf or version header', async function () {

src/server/kbn_server.js

@@ -140,9 +140,9 @@ export default class KbnServer {
    * @return undefined
    */
   async listen() {
-    const { server } = this;
-
     await this.ready();
+
+    const { server } = this;
     await fromNode(cb => server.start(cb));
 
     if (isWorker) {

src/test_utils/kbn_server.js

@@ -25,7 +25,10 @@ import KbnServer from '../../src/server/kbn_server';
 
 const DEFAULTS_SETTINGS = {
   server: {
-    autoListen: false,
+    autoListen: true,
+    // Use the ephemeral port to make sure that tests use the first available
+    // port and aren't affected by the timing issues in test environment.
+    port: 0,
     xsrf: {
       disableProtection: true
     }
@@ -92,9 +95,12 @@ export function authOptions() {
  *
  * @param {KbnServer} kbnServer
  * @param {object}    options Any additional options or overrides for inject()
- * @param {Function}  fn The callback to pass as the second arg to inject()
  */
-export function makeRequest(kbnServer, options, fn) {
-  options = defaultsDeep({}, authOptions(), options);
-  return kbnServer.server.inject(options, fn);
+export async function makeRequest(kbnServer, options) {
+  // Since all requests to Kibana hit core http server first and only after that
+  // are proxied to the "legacy" Kibana we should inject requests through the top
+  // level Hapi server used by the core.
+  return await kbnServer.newPlatform.proxyListener.root.server.http.service.httpServer.server.inject(
+    defaultsDeep({}, authOptions(), options)
+  );
 }

src/ui/field_formats/__tests__/field_formats_mixin.js

@@ -27,26 +27,26 @@ import { createServer } from '../../../test_utils/kbn_server';
 describe('server.registerFieldFormat(createFormat)', () => {
   const sandbox = sinon.createSandbox();
 
-  let server;
+  let kbnServer;
   beforeEach(async () => {
-    const kbnServer = createServer();
+    kbnServer = createServer();
     await kbnServer.ready();
-    server = kbnServer.server;
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     sandbox.restore();
+    await kbnServer.close();
   });
 
   it('throws if createFormat is not a function', () => {
-    expect(() => server.registerFieldFormat()).to.throwError(error => {
+    expect(() => kbnServer.server.registerFieldFormat()).to.throwError(error => {
       expect(error.message).to.match(/createFormat is not a function/i);
     });
   });
 
   it('calls the createFormat() function with the FieldFormat class', () => {
     const createFormat = sinon.stub();
-    server.registerFieldFormat(createFormat);
+    kbnServer.server.registerFieldFormat(createFormat);
     sinon.assert.calledOnce(createFormat);
     sinon.assert.calledWithExactly(createFormat, sinon.match.same(FieldFormat));
   });
@@ -61,9 +61,9 @@ describe('server.registerFieldFormat(createFormat)', () => {
     class FooFormat {
       static id = 'foo'
     }
-    server.registerFieldFormat(() => FooFormat);
+    kbnServer.server.registerFieldFormat(() => FooFormat);
 
-    const fieldFormats = await server.fieldFormatServiceFactory({
+    const fieldFormats = await kbnServer.server.fieldFormatServiceFactory({
       getAll: () => ({}),
       getDefaults: () => ({})
     });