From 4affe3432b99c5189ec8fa0cd9f7d6923d812d66 Mon Sep 17 00:00:00 2001 From: slvrtrn Date: Wed, 27 Nov 2024 14:47:21 +0100 Subject: [PATCH] Verify the response string length overflow and throw a specific error --- .../integration/invalid_string_length.test.ts | 45 +++++++++++++++++++ packages/client-node/src/utils/stream.ts | 12 ++++- packages/client-web/src/utils/stream.ts | 12 ++++- 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 packages/client-common/__tests__/integration/invalid_string_length.test.ts diff --git a/packages/client-common/__tests__/integration/invalid_string_length.test.ts b/packages/client-common/__tests__/integration/invalid_string_length.test.ts new file mode 100644 index 00000000..2bb06dce --- /dev/null +++ b/packages/client-common/__tests__/integration/invalid_string_length.test.ts @@ -0,0 +1,45 @@ +import type { ClickHouseClient } from '@clickhouse/client-common' +import { createTestClient } from '@test/utils' + +// See https://github.com/ClickHouse/clickhouse-js/issues/106 +describe('invalid string length', () => { + let client: ClickHouseClient + afterEach(async () => { + await client.close() + }) + beforeEach(async () => { + client = createTestClient() + }) + + const errorMessageMatcher = jasmine.stringContaining( + 'consider limiting the amount of requested rows', + ) + + // The client will buffer the entire response in a string for non-streamable formats. + // A large amount of system.numbers selected should overflow the max allowed allocation size for a string. + it('should fail with a specific error when the response string is too large - json() call', async () => { + const rs = await client.query({ + query: 'SELECT number FROM numbers(50000000)', + format: 'JSON', + }) + await expectAsync(rs.json()).toBeRejectedWith( + jasmine.objectContaining({ + message: errorMessageMatcher, + }), + ) + expect().nothing() + }) + + it('should fail with a specific error when the response string is too large - text() call', async () => { + const rs = await client.query({ + query: 'SELECT number FROM numbers(50000000)', + format: 'JSON', + }) + await expectAsync(rs.text()).toBeRejectedWith( + jasmine.objectContaining({ + message: errorMessageMatcher, + }), + ) + expect().nothing() + }) +}) diff --git a/packages/client-node/src/utils/stream.ts b/packages/client-node/src/utils/stream.ts index 7523828b..3e2dd68e 100644 --- a/packages/client-node/src/utils/stream.ts +++ b/packages/client-node/src/utils/stream.ts @@ -1,5 +1,8 @@ import Stream from 'stream' +// See https://github.com/v8/v8/commit/ea56bf5513d0cbd2a35a9035c5c2996272b8b728 +const MaxStringLength = Math.pow(2, 29) - 24 + export function isStream(obj: any): obj is Stream.Readable { return obj !== null && typeof obj.pipe === 'function' } @@ -9,7 +12,14 @@ export async function getAsText(stream: Stream.Readable): Promise { const textDecoder = new TextDecoder() for await (const chunk of stream) { - text += textDecoder.decode(chunk, { stream: true }) + const decoded = textDecoder.decode(chunk, { stream: true }) + if (decoded.length + text.length > MaxStringLength) { + throw new Error( + 'The response length exceeds the maximum allowed size of V8 String: ' + + `${MaxStringLength}; consider limiting the amount of requested rows.`, + ) + } + text += decoded } // flush diff --git a/packages/client-web/src/utils/stream.ts b/packages/client-web/src/utils/stream.ts index 242923b4..9356815a 100644 --- a/packages/client-web/src/utils/stream.ts +++ b/packages/client-web/src/utils/stream.ts @@ -1,3 +1,6 @@ +// See https://github.com/v8/v8/commit/ea56bf5513d0cbd2a35a9035c5c2996272b8b728 +const MaxStringLength = Math.pow(2, 29) - 24 + export function isStream(obj: any): obj is ReadableStream { return ( obj !== null && obj !== undefined && typeof obj.pipeThrough === 'function' @@ -13,7 +16,14 @@ export async function getAsText(stream: ReadableStream): Promise { while (!isDone) { const { done, value } = await reader.read() - result += textDecoder.decode(value, { stream: true }) + const decoded = textDecoder.decode(value, { stream: true }) + if (decoded.length + result.length > MaxStringLength) { + throw new Error( + 'The response length exceeds the maximum allowed size of V8 String: ' + + `${MaxStringLength}; consider limiting the amount of requested rows.`, + ) + } + result += decoded isDone = done }