[core.logging] Fix calculating payload bytes for arrays and zlib streams. (#92100)

This commit is contained in:
Luke Elmers 2021-03-02 11:21:39 -07:00 committed by GitHub
parent 3dd3297371
commit 81348b8693
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 217 additions and 94 deletions

View file

@ -6,9 +6,10 @@
* Side Public License, v 1. * Side Public License, v 1.
*/ */
import { createGunzip } from 'zlib';
import mockFs from 'mock-fs'; import mockFs from 'mock-fs';
import { createReadStream } from 'fs'; import { createReadStream } from 'fs';
import { PassThrough } from 'stream';
import { createGzip, createGunzip } from 'zlib';
import { getResponsePayloadBytes } from './get_payload_size'; import { getResponsePayloadBytes } from './get_payload_size';
@ -27,9 +28,15 @@ describe('getPayloadSize', () => {
}); });
}); });
describe('handles fs streams', () => { describe('handles streams', () => {
afterEach(() => mockFs.restore()); afterEach(() => mockFs.restore());
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 () => { test('with ascii characters', async () => {
mockFs({ 'test.txt': 'heya' }); mockFs({ 'test.txt': 'heya' });
const readStream = createReadStream('test.txt'); const readStream = createReadStream('test.txt');
@ -56,9 +63,39 @@ describe('getPayloadSize', () => {
expect(result).toBe(Buffer.byteLength(data)); expect(result).toBe(Buffer.byteLength(data));
}); });
test('ignores streams that are not instances of ReadStream', async () => { describe('zlib streams', () => {
const result = getResponsePayloadBytes(createGunzip()); test('with ascii characters', async () => {
expect(result).toBe(undefined); 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);
});
});
}); });
}); });
@ -79,8 +116,17 @@ describe('getPayloadSize', () => {
expect(result).toBe(JSON.stringify(payload).length); 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', () => { 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); expect(result).toBe(undefined);
}); });
}); });

View file

@ -8,11 +8,15 @@
import { isPlainObject } from 'lodash'; import { isPlainObject } from 'lodash';
import { ReadStream } from 'fs'; import { ReadStream } from 'fs';
import { Zlib } from 'zlib';
import type { ResponseObject } from '@hapi/hapi'; import type { ResponseObject } from '@hapi/hapi';
const isBuffer = (obj: unknown): obj is Buffer => Buffer.isBuffer(obj); const isBuffer = (obj: unknown): obj is Buffer => Buffer.isBuffer(obj);
const isFsReadStream = (obj: unknown): obj is ReadStream => const isFsReadStream = (obj: unknown): obj is ReadStream =>
typeof obj === 'object' && obj !== null && 'bytesRead' in obj && obj instanceof 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'; const isString = (obj: unknown): obj is string => typeof obj === 'string';
/** /**
@ -51,11 +55,15 @@ export function getResponsePayloadBytes(
return payload.bytesRead; return payload.bytesRead;
} }
if (isZlibStream(payload)) {
return payload.bytesWritten;
}
if (isString(payload)) { if (isString(payload)) {
return Buffer.byteLength(payload); return Buffer.byteLength(payload);
} }
if (isPlainObject(payload)) { if (isPlainObject(payload) || Array.isArray(payload)) {
return Buffer.byteLength(JSON.stringify(payload)); return Buffer.byteLength(JSON.stringify(payload));
} }

View file

@ -6,12 +6,13 @@
* Side Public License, v 1. * Side Public License, v 1.
*/ */
import { createGunzip } from 'zlib';
import type { Request } from '@hapi/hapi'; import type { Request } from '@hapi/hapi';
import Boom from '@hapi/boom'; import Boom from '@hapi/boom';
import mockFs from 'mock-fs'; import mockFs from 'mock-fs';
import { createReadStream } from 'fs'; import { createReadStream } from 'fs';
import { PassThrough } from 'stream';
import { createGunzip, createGzip } from 'zlib';
import { loggerMock, MockedLogger } from '../../logging/logger.mock'; import { loggerMock, MockedLogger } from '../../logging/logger.mock';
@ -55,9 +56,22 @@ describe('getPayloadSize', () => {
}); });
}); });
describe('handles fs streams', () => { describe('handles streams', () => {
afterEach(() => mockFs.restore()); afterEach(() => mockFs.restore());
test('ignores streams that are not fs or zlib streams', async () => {
const result = getResponsePayloadBytes(
{
variety: 'stream',
source: new PassThrough(),
} as Response,
logger
);
expect(result).toBe(undefined);
});
describe('fs streams', () => {
test('with ascii characters', async () => { test('with ascii characters', async () => {
mockFs({ 'test.txt': 'heya' }); mockFs({ 'test.txt': 'heya' });
const source = createReadStream('test.txt'); const source = createReadStream('test.txt');
@ -97,17 +111,52 @@ describe('getPayloadSize', () => {
expect(result).toBe(Buffer.byteLength(data)); 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;
}
test('ignores streams that are not instances of ReadStream', async () => {
const result = getResponsePayloadBytes( const result = getResponsePayloadBytes(
{ {
variety: 'stream', variety: 'stream',
source: createGunzip(), source,
} as Response, } as Response,
logger logger
); );
expect(result).toBe(undefined); 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);
});
}); });
}); });
@ -134,7 +183,7 @@ describe('getPayloadSize', () => {
expect(result).toBe(7); expect(result).toBe(7);
}); });
test('when source is object', () => { test('when source is plain object', () => {
const payload = { message: 'heya' }; const payload = { message: 'heya' };
const result = getResponsePayloadBytes( const result = getResponsePayloadBytes(
{ {
@ -146,11 +195,26 @@ describe('getPayloadSize', () => {
expect(result).toBe(JSON.stringify(payload).length); 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( const result = getResponsePayloadBytes(
{ {
variety: 'plain', 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, } as Response,
logger logger
); );

View file

@ -8,25 +8,23 @@
import { isPlainObject } from 'lodash'; import { isPlainObject } from 'lodash';
import { ReadStream } from 'fs'; import { ReadStream } from 'fs';
import { Zlib } from 'zlib';
import { isBoom } from '@hapi/boom'; import { isBoom } from '@hapi/boom';
import type { Request } from '@hapi/hapi'; import type { Request } from '@hapi/hapi';
import { Logger } from '../../logging'; import { Logger } from '../../logging';
type Response = Request['response']; type Response = Request['response'];
const isBuffer = (src: unknown, res: Response): src is Buffer => { const isBuffer = (src: unknown, variety: string): src is Buffer =>
return !isBoom(res) && res.variety === 'buffer' && res.source === src; 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 => { const isZlibStream = (src: unknown, variety: string): src is Zlib => {
return ( return variety === 'stream' && typeof src === 'object' && src !== null && 'bytesWritten' in src;
!isBoom(res) &&
res.variety === 'stream' &&
res.source === src &&
res.source instanceof ReadStream
);
}; };
const isString = (src: unknown, res: Response): src is string => const isString = (src: unknown, variety: string): src is string =>
!isBoom(res) && res.variety === 'plain' && typeof src === 'string'; variety === 'plain' && typeof src === 'string';
/** /**
* Attempts to determine the size (in bytes) of a Hapi response * 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)); return Buffer.byteLength(JSON.stringify(response.output.payload));
} }
if (isBuffer(response.source, response)) { if (isBuffer(response.source, response.variety)) {
return response.source.byteLength; return response.source.byteLength;
} }
if (isFsReadStream(response.source, response)) { if (isFsReadStream(response.source, response.variety)) {
return response.source.bytesRead; 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); 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)); return Buffer.byteLength(JSON.stringify(response.source));
} }
} catch (e) { } catch (e) {