[7.x] Fix issue where logs fail to calculate size of gunzip streams. (e…

…lastic#90353) (elastic#90431)
chandlerprall · Feb 5, 2021 · 6582eaa · 6582eaa
1 parent f8a354f
commit 6582eaa
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 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
@@ -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
@@ -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<string, unknown> =>
-  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
@@ -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
@@ -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