maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Adds workaround for hapi h2o2 proxy issue for DELETE REST calls that have bodies in developer mode (#87270)

Browse source 
## Summary

When you run Kibana in developer mode you have 3 random digits assigned to your URL and we proxy things through a [h2o2 proxy](https://github.com/hapijs/h2o2) to help the developers with development with regards to proxies

```ts
node --max-old-space-size=2048 scripts/kibana --dev
```

However when you try to send a body with the DELETE verb in the browser such as using the `security_solution` and try to delete a rule:
<img width="640" alt="Screen Shot 2021-01-04 at 8 06 15 PM" src="https://user-images.githubusercontent.com/1151048/103602057-53b92380-4ec8-11eb-972c-7e57317bccab.png">

You get an error toaster showing up who's content is "Bad Request":
<img width="457" alt="Screen Shot 2021-01-04 at 8 03 14 PM" src="https://user-images.githubusercontent.com/1151048/103601947-05a42000-4ec8-11eb-94d9-0ea834a20310.png">

The reason for this bug looks to be from our proxy usage of `h2o2` when we are in development mode where it removes `content-length` when you send a body with a `DELETE`. I created a bug and workaround for the `h2o2` project directly:
https://github.com/hapijs/h2o2/issues/124

This fix here is the workaround applied to Kibana. With this workaround applied there should be no more error toasters for developers.

Additional fixes/improvements are:
* I ported the unit tests from `src/core/server/http/http_server.test.ts` to `src/core/server/http/base_path_proxy_server.test.ts` since the `base_path_proxy_server` did not have any unit tests. I also added additional unit tests to cover the specific use cases that are in `base_path_proxy_server`
* I fixed the placement of some tests from `src/core/server/http/http_server.test.ts` where there were a few that were under the wrong describe block and I changed a few `it` -> `test` as it looks like that file should be consistent with `test` instead of odd mixture of `it`.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
This commit is contained in:
Frank Hassanabad 2021-01-11 12:15:02 -07:00 committed by GitHub
parent 379f9c9646
commit 2658855cb7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 1131 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1052
src/core/server/http/base_path_proxy_server.test.ts Normal file
View file

File diff suppressed because it is too large Load diff

21
src/core/server/http/base_path_proxy_server.ts
View file

@ -143,12 +143,25 @@ export class BasePathProxyServer {
handler: {
proxy: {
agent: this.httpsAgent,
host: this.server.info.host,
passThrough: true,
port: this.devConfig.basePathProxyTargetPort,
// typings mismatch. h2o2 doesn't support "socket"
protocol: this.server.info.protocol as HapiProxy.ProxyHandlerOptions['protocol'],
xforward: true,
mapUri: async (request) => {
return {
// Passing in this header to merge it is a workaround until this is fixed:
// https://github.com/hapijs/h2o2/issues/124
headers:
request.headers['content-length'] != null
? { 'content-length': request.headers['content-length'] }
: undefined,
uri: Url.format({
hostname: request.server.info.host,
port: this.devConfig.basePathProxyTargetPort,
protocol: request.server.info.protocol,
pathname: request.path,
query: request.query,
}),
};
},
},
},
method: '*',

127
src/core/server/http/http_server.test.ts
View file

@ -888,52 +888,48 @@ describe('conditional compression', () => {
expect(response.header).not.toHaveProperty('content-encoding');
});
});
});
describe('response headers', () => {
it('allows to configure "keep-alive" header', async () => {
const { registerRouter, server: innerServer } = await server.setup({
...config,
keepaliveTimeout: 100_000,
});
const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) =>
res.ok({ body: req.route })
);
registerRouter(router);
await server.start();
const response = await supertest(innerServer.listener)
.get('/')
.set('Connection', 'keep-alive')
.expect(200);
expect(response.header.connection).toBe('keep-alive');
expect(response.header['keep-alive']).toBe('timeout=100');
describe('response headers', () => {
test('allows to configure "keep-alive" header', async () => {
const { registerRouter, server: innerServer } = await server.setup({
...config,
keepaliveTimeout: 100_000,
});
it('default headers', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) => res.ok({ body: req.route }));
registerRouter(router);
const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) =>
res.ok({ body: req.route })
);
registerRouter(router);
await server.start();
const response = await supertest(innerServer.listener)
.get('/')
.set('Connection', 'keep-alive')
.expect(200);
await server.start();
const response = await supertest(innerServer.listener).get('/').expect(200);
expect(response.header.connection).toBe('keep-alive');
expect(response.header['keep-alive']).toBe('timeout=100');
});
const restHeaders = omit(response.header, ['date', 'content-length']);
expect(restHeaders).toMatchInlineSnapshot(`
Object {
"accept-ranges": "bytes",
"cache-control": "private, no-cache, no-store, must-revalidate",
"connection": "close",
"content-type": "application/json; charset=utf-8",
}
`);
});
test('default headers', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.get({ path: '/', validate: false }, (context, req, res) => res.ok({ body: req.route }));
registerRouter(router);
await server.start();
const response = await supertest(innerServer.listener).get('/').expect(200);
const restHeaders = omit(response.header, ['date', 'content-length']);
expect(restHeaders).toMatchInlineSnapshot(`
Object {
"accept-ranges": "bytes",
"cache-control": "private, no-cache, no-store, must-revalidate",
"connection": "close",
"content-type": "application/json; charset=utf-8",
}
`);
});
});
@ -1270,31 +1266,31 @@ describe('timeout options', () => {
},
});
});
});
test(`idleSocket timeout can be smaller than the payload timeout`, async () => {
const { registerRouter } = await server.setup(config);
test('idleSocket timeout can be smaller than the payload timeout', async () => {
const { registerRouter } = await server.setup(config);
const router = new Router('', logger, enhanceWithContext);
router.post(
{
path: '/',
validate: { body: schema.any() },
options: {
timeout: {
payload: 1000,
idleSocket: 10,
const router = new Router('', logger, enhanceWithContext);
router.post(
{
path: '/',
validate: { body: schema.any() },
options: {
timeout: {
payload: 1000,
idleSocket: 10,
},
},
},
},
(context, req, res) => {
return res.ok({ body: { timeout: req.route.options.timeout } });
}
);
(context, req, res) => {
return res.ok({ body: { timeout: req.route.options.timeout } });
}
);
registerRouter(router);
registerRouter(router);
await server.start();
await server.start();
});
});
});
@ -1329,13 +1325,14 @@ test('should return a stream in the body', async () => {
describe('setup contract', () => {
describe('#createSessionStorage', () => {
it('creates session storage factory', async () => {
test('creates session storage factory', async () => {
const { createCookieSessionStorageFactory } = await server.setup(config);
const sessionStorageFactory = await createCookieSessionStorageFactory(cookieOptions);
expect(sessionStorageFactory.asScoped).toBeDefined();
});
it('creates session storage factory only once', async () => {
test('creates session storage factory only once', async () => {
const { createCookieSessionStorageFactory } = await server.setup(config);
const create = async () => await createCookieSessionStorageFactory(cookieOptions);
@ -1343,7 +1340,7 @@ describe('setup contract', () => {
expect(create()).rejects.toThrowError('A cookieSessionStorageFactory was already created');
});
it('does not throw if called after stop', async () => {
test('does not throw if called after stop', async () => {
const { createCookieSessionStorageFactory } = await server.setup(config);
await server.stop();
expect(() => {
@ -1353,7 +1350,7 @@ describe('setup contract', () => {
});
describe('#getServerInfo', () => {
it('returns correct information', async () => {
test('returns correct information', async () => {
let { getServerInfo } = await server.setup(config);
expect(getServerInfo()).toEqual({
@ -1378,7 +1375,7 @@ describe('setup contract', () => {
});
});
it('returns correct protocol when ssl is enabled', async () => {
test('returns correct protocol when ssl is enabled', async () => {
const { getServerInfo } = await server.setup(configWithSSL);
expect(getServerInfo().protocol).toEqual('https');
@ -1386,7 +1383,7 @@ describe('setup contract', () => {
});
describe('#registerStaticDir', () => {
it('does not throw if called after stop', async () => {
test('does not throw if called after stop', async () => {
const { registerStaticDir } = await server.setup(config);
await server.stop();
expect(() => {