From a971c251e9c26af9b5713f023c87eff966c5e712 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Thu, 4 Feb 2021 17:43:03 -0700 Subject: [PATCH] Fix issue where logs fail to calculate size of gunzip streams. (#90353) --- .../src/utils/get_payload_size.test.ts | 11 +++++++++ .../src/utils/get_payload_size.ts | 9 ++++--- .../http/logging/get_payload_size.test.ts | 24 +++++++++++++++++++ .../server/http/logging/get_payload_size.ts | 22 ++++++++++++----- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/packages/kbn-legacy-logging/src/utils/get_payload_size.test.ts b/packages/kbn-legacy-logging/src/utils/get_payload_size.test.ts index c70f95b9ddc1..3bb97e57ca0a 100644 --- a/packages/kbn-legacy-logging/src/utils/get_payload_size.test.ts +++ b/packages/kbn-legacy-logging/src/utils/get_payload_size.test.ts @@ -6,6 +6,7 @@ * Side Public License, v 1. */ +import { createGunzip } from 'zlib'; import mockFs from 'mock-fs'; import { createReadStream } from 'fs'; @@ -54,6 +55,11 @@ describe('getPayloadSize', () => { const result = getResponsePayloadBytes(readStream); expect(result).toBe(Buffer.byteLength(data)); }); + + test('ignores streams that are not instances of ReadStream', async () => { + const result = getResponsePayloadBytes(createGunzip()); + expect(result).toBe(undefined); + }); }); describe('handles plain responses', () => { @@ -72,6 +78,11 @@ describe('getPayloadSize', () => { const result = getResponsePayloadBytes(payload); expect(result).toBe(JSON.stringify(payload).length); }); + + test('returns undefined when source is not plain object', () => { + const result = getResponsePayloadBytes([1, 2, 3]); + expect(result).toBe(undefined); + }); }); describe('handles content-length header', () => { diff --git a/packages/kbn-legacy-logging/src/utils/get_payload_size.ts b/packages/kbn-legacy-logging/src/utils/get_payload_size.ts index de96ad700273..c7aeb0e8cac2 100644 --- a/packages/kbn-legacy-logging/src/utils/get_payload_size.ts +++ b/packages/kbn-legacy-logging/src/utils/get_payload_size.ts @@ -6,14 +6,13 @@ * Side Public License, v 1. */ -import type { ReadStream } from 'fs'; +import { isPlainObject } from 'lodash'; +import { ReadStream } from 'fs'; import type { ResponseObject } from '@hapi/hapi'; const isBuffer = (obj: unknown): obj is Buffer => Buffer.isBuffer(obj); -const isObject = (obj: unknown): obj is Record => - typeof obj === 'object' && obj !== null; const isFsReadStream = (obj: unknown): obj is ReadStream => - typeof obj === 'object' && obj !== null && 'bytesRead' in obj; + typeof obj === 'object' && obj !== null && 'bytesRead' in obj && obj instanceof ReadStream; const isString = (obj: unknown): obj is string => typeof obj === 'string'; /** @@ -56,7 +55,7 @@ export function getResponsePayloadBytes( return Buffer.byteLength(payload); } - if (isObject(payload)) { + if (isPlainObject(payload)) { return Buffer.byteLength(JSON.stringify(payload)); } diff --git a/src/core/server/http/logging/get_payload_size.test.ts b/src/core/server/http/logging/get_payload_size.test.ts index a4ab8919e8b6..dba5c7be30f3 100644 --- a/src/core/server/http/logging/get_payload_size.test.ts +++ b/src/core/server/http/logging/get_payload_size.test.ts @@ -6,6 +6,7 @@ * Side Public License, v 1. */ +import { createGunzip } from 'zlib'; import type { Request } from '@hapi/hapi'; import Boom from '@hapi/boom'; @@ -96,6 +97,18 @@ describe('getPayloadSize', () => { expect(result).toBe(Buffer.byteLength(data)); }); + + test('ignores streams that are not instances of ReadStream', async () => { + const result = getResponsePayloadBytes( + { + variety: 'stream', + source: createGunzip(), + } as Response, + logger + ); + + expect(result).toBe(undefined); + }); }); describe('handles plain responses', () => { @@ -132,6 +145,17 @@ describe('getPayloadSize', () => { ); expect(result).toBe(JSON.stringify(payload).length); }); + + test('returns undefined when source is not a plain object', () => { + const result = getResponsePayloadBytes( + { + variety: 'plain', + source: [1, 2, 3], + } as Response, + logger + ); + expect(result).toBe(undefined); + }); }); describe('handles content-length header', () => { diff --git a/src/core/server/http/logging/get_payload_size.ts b/src/core/server/http/logging/get_payload_size.ts index 6dcaf3653d84..8e6dea13e1fa 100644 --- a/src/core/server/http/logging/get_payload_size.ts +++ b/src/core/server/http/logging/get_payload_size.ts @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import type { ReadStream } from 'fs'; +import { isPlainObject } from 'lodash'; +import { ReadStream } from 'fs'; import { isBoom } from '@hapi/boom'; import type { Request } from '@hapi/hapi'; import { Logger } from '../../logging'; @@ -17,8 +18,15 @@ const isBuffer = (src: unknown, res: Response): src is Buffer => { return !isBoom(res) && res.variety === 'buffer' && res.source === src; }; const isFsReadStream = (src: unknown, res: Response): src is ReadStream => { - return !isBoom(res) && res.variety === 'stream' && res.source === src; + return ( + !isBoom(res) && + res.variety === 'stream' && + res.source === src && + res.source instanceof ReadStream + ); }; +const isString = (src: unknown, res: Response): src is string => + !isBoom(res) && res.variety === 'plain' && typeof src === 'string'; /** * Attempts to determine the size (in bytes) of a Hapi response @@ -57,10 +65,12 @@ export function getResponsePayloadBytes(response: Response, log: Logger): number return response.source.bytesRead; } - if (response.variety === 'plain') { - return typeof response.source === 'string' - ? Buffer.byteLength(response.source) - : Buffer.byteLength(JSON.stringify(response.source)); + if (isString(response.source, response)) { + return Buffer.byteLength(response.source); + } + + if (response.variety === 'plain' && isPlainObject(response.source)) { + return Buffer.byteLength(JSON.stringify(response.source)); } } catch (e) { // We intentionally swallow any errors as this information is