From 81348b86934097c10133373cd1d7e08f8ff77cb5 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Tue, 2 Mar 2021 11:21:39 -0700 Subject: [PATCH] [core.logging] Fix calculating payload bytes for arrays and zlib streams. (#92100) --- .../src/utils/get_payload_size.test.ts | 108 ++++++++---- .../src/utils/get_payload_size.ts | 10 +- .../http/logging/get_payload_size.test.ts | 158 ++++++++++++------ .../server/http/logging/get_payload_size.ts | 35 ++-- 4 files changed, 217 insertions(+), 94 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 3bb97e57ca0a..01d2cf29758d 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,9 +6,10 @@ * Side Public License, v 1. */ -import { createGunzip } from 'zlib'; import mockFs from 'mock-fs'; import { createReadStream } from 'fs'; +import { PassThrough } from 'stream'; +import { createGzip, createGunzip } from 'zlib'; import { getResponsePayloadBytes } from './get_payload_size'; @@ -27,39 +28,75 @@ describe('getPayloadSize', () => { }); }); - describe('handles fs streams', () => { + describe('handles streams', () => { afterEach(() => mockFs.restore()); - test('with ascii characters', async () => { - mockFs({ 'test.txt': 'heya' }); - const readStream = createReadStream('test.txt'); - - let data = ''; - for await (const chunk of readStream) { - data += chunk; - } - - const result = getResponsePayloadBytes(readStream); - expect(result).toBe(Buffer.byteLength(data)); - }); - - test('with special characters', async () => { - mockFs({ 'test.txt': '¡hola!' }); - const readStream = createReadStream('test.txt'); - - let data = ''; - for await (const chunk of readStream) { - data += chunk; - } - - const result = getResponsePayloadBytes(readStream); - expect(result).toBe(Buffer.byteLength(data)); - }); - - test('ignores streams that are not instances of ReadStream', async () => { - const result = getResponsePayloadBytes(createGunzip()); + test('ignores streams that are not fs or zlib streams', async () => { + const result = getResponsePayloadBytes(new PassThrough()); expect(result).toBe(undefined); }); + + describe('fs streams', () => { + test('with ascii characters', async () => { + mockFs({ 'test.txt': 'heya' }); + const readStream = createReadStream('test.txt'); + + let data = ''; + for await (const chunk of readStream) { + data += chunk; + } + + const result = getResponsePayloadBytes(readStream); + expect(result).toBe(Buffer.byteLength(data)); + }); + + test('with special characters', async () => { + mockFs({ 'test.txt': '¡hola!' }); + const readStream = createReadStream('test.txt'); + + let data = ''; + for await (const chunk of readStream) { + data += chunk; + } + + const result = getResponsePayloadBytes(readStream); + expect(result).toBe(Buffer.byteLength(data)); + }); + + describe('zlib streams', () => { + test('with ascii characters', async () => { + mockFs({ 'test.txt': 'heya' }); + const readStream = createReadStream('test.txt'); + const source = readStream.pipe(createGzip()).pipe(createGunzip()); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes(source); + + expect(data).toBe('heya'); + expect(result).toBe(source.bytesWritten); + }); + + test('with special characters', async () => { + mockFs({ 'test.txt': '¡hola!' }); + const readStream = createReadStream('test.txt'); + const source = readStream.pipe(createGzip()).pipe(createGunzip()); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes(source); + + expect(data).toBe('¡hola!'); + expect(result).toBe(source.bytesWritten); + }); + }); + }); }); describe('handles plain responses', () => { @@ -79,8 +116,17 @@ describe('getPayloadSize', () => { expect(result).toBe(JSON.stringify(payload).length); }); + test('when source is array object', () => { + const payload = [{ message: 'hey' }, { message: 'ya' }]; + 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]); + class TestClass { + constructor() {} + } + const result = getResponsePayloadBytes(new TestClass()); expect(result).toBe(undefined); }); }); 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 c7aeb0e8cac2..acc517c74c2d 100644 --- a/packages/kbn-legacy-logging/src/utils/get_payload_size.ts +++ b/packages/kbn-legacy-logging/src/utils/get_payload_size.ts @@ -8,11 +8,15 @@ import { isPlainObject } from 'lodash'; import { ReadStream } from 'fs'; +import { Zlib } from 'zlib'; import type { ResponseObject } from '@hapi/hapi'; const isBuffer = (obj: unknown): obj is Buffer => Buffer.isBuffer(obj); const isFsReadStream = (obj: unknown): obj is ReadStream => typeof obj === 'object' && obj !== null && 'bytesRead' in obj && obj instanceof ReadStream; +const isZlibStream = (obj: unknown): obj is Zlib => { + return typeof obj === 'object' && obj !== null && 'bytesWritten' in obj; +}; const isString = (obj: unknown): obj is string => typeof obj === 'string'; /** @@ -51,11 +55,15 @@ export function getResponsePayloadBytes( return payload.bytesRead; } + if (isZlibStream(payload)) { + return payload.bytesWritten; + } + if (isString(payload)) { return Buffer.byteLength(payload); } - if (isPlainObject(payload)) { + if (isPlainObject(payload) || Array.isArray(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 dba5c7be30f3..30cb547dd98b 100644 --- a/src/core/server/http/logging/get_payload_size.test.ts +++ b/src/core/server/http/logging/get_payload_size.test.ts @@ -6,12 +6,13 @@ * Side Public License, v 1. */ -import { createGunzip } from 'zlib'; import type { Request } from '@hapi/hapi'; import Boom from '@hapi/boom'; import mockFs from 'mock-fs'; import { createReadStream } from 'fs'; +import { PassThrough } from 'stream'; +import { createGunzip, createGzip } from 'zlib'; import { loggerMock, MockedLogger } from '../../logging/logger.mock'; @@ -55,60 +56,108 @@ describe('getPayloadSize', () => { }); }); - describe('handles fs streams', () => { + describe('handles streams', () => { afterEach(() => mockFs.restore()); - test('with ascii characters', async () => { - mockFs({ 'test.txt': 'heya' }); - const source = createReadStream('test.txt'); - - let data = ''; - for await (const chunk of source) { - data += chunk; - } - + test('ignores streams that are not fs or zlib streams', async () => { const result = getResponsePayloadBytes( { variety: 'stream', - source, - } as Response, - logger - ); - - expect(result).toBe(Buffer.byteLength(data)); - }); - - test('with special characters', async () => { - mockFs({ 'test.txt': '¡hola!' }); - const source = createReadStream('test.txt'); - - let data = ''; - for await (const chunk of source) { - data += chunk; - } - - const result = getResponsePayloadBytes( - { - variety: 'stream', - source, - } as Response, - logger - ); - - expect(result).toBe(Buffer.byteLength(data)); - }); - - test('ignores streams that are not instances of ReadStream', async () => { - const result = getResponsePayloadBytes( - { - variety: 'stream', - source: createGunzip(), + source: new PassThrough(), } as Response, logger ); expect(result).toBe(undefined); }); + + describe('fs streams', () => { + test('with ascii characters', async () => { + mockFs({ 'test.txt': 'heya' }); + const source = createReadStream('test.txt'); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes( + { + variety: 'stream', + source, + } as Response, + logger + ); + + expect(result).toBe(Buffer.byteLength(data)); + }); + + test('with special characters', async () => { + mockFs({ 'test.txt': '¡hola!' }); + const source = createReadStream('test.txt'); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes( + { + variety: 'stream', + source, + } as Response, + logger + ); + + expect(result).toBe(Buffer.byteLength(data)); + }); + }); + + describe('zlib streams', () => { + test('with ascii characters', async () => { + mockFs({ 'test.txt': 'heya' }); + const readStream = createReadStream('test.txt'); + const source = readStream.pipe(createGzip()).pipe(createGunzip()); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes( + { + variety: 'stream', + source, + } as Response, + logger + ); + + expect(data).toBe('heya'); + expect(result).toBe(source.bytesWritten); + }); + + test('with special characters', async () => { + mockFs({ 'test.txt': '¡hola!' }); + const readStream = createReadStream('test.txt'); + const source = readStream.pipe(createGzip()).pipe(createGunzip()); + + let data = ''; + for await (const chunk of source) { + data += chunk; + } + + const result = getResponsePayloadBytes( + { + variety: 'stream', + source, + } as Response, + logger + ); + + expect(data).toBe('¡hola!'); + expect(result).toBe(source.bytesWritten); + }); + }); }); describe('handles plain responses', () => { @@ -134,7 +183,7 @@ describe('getPayloadSize', () => { expect(result).toBe(7); }); - test('when source is object', () => { + test('when source is plain object', () => { const payload = { message: 'heya' }; const result = getResponsePayloadBytes( { @@ -146,11 +195,26 @@ describe('getPayloadSize', () => { expect(result).toBe(JSON.stringify(payload).length); }); - test('returns undefined when source is not a plain object', () => { + test('when source is array object', () => { + const payload = [{ message: 'hey' }, { message: 'ya' }]; const result = getResponsePayloadBytes( { variety: 'plain', - source: [1, 2, 3], + source: payload, + } as Response, + logger + ); + expect(result).toBe(JSON.stringify(payload).length); + }); + + test('returns undefined when source is not a plain object', () => { + class TestClass { + constructor() {} + } + const result = getResponsePayloadBytes( + { + variety: 'plain', + source: new TestClass(), } as Response, logger ); diff --git a/src/core/server/http/logging/get_payload_size.ts b/src/core/server/http/logging/get_payload_size.ts index 8e6dea13e1fa..2797b4ba9f49 100644 --- a/src/core/server/http/logging/get_payload_size.ts +++ b/src/core/server/http/logging/get_payload_size.ts @@ -8,25 +8,23 @@ import { isPlainObject } from 'lodash'; import { ReadStream } from 'fs'; +import { Zlib } from 'zlib'; import { isBoom } from '@hapi/boom'; import type { Request } from '@hapi/hapi'; import { Logger } from '../../logging'; type Response = Request['response']; -const isBuffer = (src: unknown, res: Response): src is Buffer => { - return !isBoom(res) && res.variety === 'buffer' && res.source === src; +const isBuffer = (src: unknown, variety: string): src is Buffer => + variety === 'buffer' && Buffer.isBuffer(src); +const isFsReadStream = (src: unknown, variety: string): src is ReadStream => { + return variety === 'stream' && src instanceof ReadStream; }; -const isFsReadStream = (src: unknown, res: Response): src is ReadStream => { - return ( - !isBoom(res) && - res.variety === 'stream' && - res.source === src && - res.source instanceof ReadStream - ); +const isZlibStream = (src: unknown, variety: string): src is Zlib => { + return variety === 'stream' && typeof src === 'object' && src !== null && 'bytesWritten' in src; }; -const isString = (src: unknown, res: Response): src is string => - !isBoom(res) && res.variety === 'plain' && typeof src === 'string'; +const isString = (src: unknown, variety: string): src is string => + variety === 'plain' && typeof src === 'string'; /** * Attempts to determine the size (in bytes) of a Hapi response @@ -57,19 +55,26 @@ export function getResponsePayloadBytes(response: Response, log: Logger): number return Buffer.byteLength(JSON.stringify(response.output.payload)); } - if (isBuffer(response.source, response)) { + if (isBuffer(response.source, response.variety)) { return response.source.byteLength; } - if (isFsReadStream(response.source, response)) { + if (isFsReadStream(response.source, response.variety)) { return response.source.bytesRead; } - if (isString(response.source, response)) { + if (isZlibStream(response.source, response.variety)) { + return response.source.bytesWritten; + } + + if (isString(response.source, response.variety)) { return Buffer.byteLength(response.source); } - if (response.variety === 'plain' && isPlainObject(response.source)) { + if ( + response.variety === 'plain' && + (isPlainObject(response.source) || Array.isArray(response.source)) + ) { return Buffer.byteLength(JSON.stringify(response.source)); } } catch (e) {