Skip to content

Commit 64ea1eb

Browse files
authored
Verify the response string length overflow and throw a specific error (#357)
1 parent 8b41e5b commit 64ea1eb

File tree

3 files changed

+67
-2
lines changed

3 files changed

+67
-2
lines changed
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import type { ClickHouseClient } from '@clickhouse/client-common'
2+
import { createTestClient } from '@test/utils'
3+
4+
// See https://github.com/ClickHouse/clickhouse-js/issues/106
5+
describe('invalid string length', () => {
6+
let client: ClickHouseClient
7+
afterEach(async () => {
8+
await client.close()
9+
})
10+
beforeEach(async () => {
11+
client = createTestClient()
12+
})
13+
14+
const errorMessageMatcher = jasmine.stringContaining(
15+
'consider limiting the amount of requested rows',
16+
)
17+
18+
// The client will buffer the entire response in a string for non-streamable formats.
19+
// A large amount of system.numbers selected should overflow the max allowed allocation size for a string.
20+
it('should fail with a specific error when the response string is too large - json() call', async () => {
21+
const rs = await client.query({
22+
query: 'SELECT number FROM numbers(50000000)',
23+
format: 'JSON',
24+
})
25+
await expectAsync(rs.json()).toBeRejectedWith(
26+
jasmine.objectContaining({
27+
message: errorMessageMatcher,
28+
}),
29+
)
30+
expect().nothing()
31+
})
32+
33+
it('should fail with a specific error when the response string is too large - text() call', async () => {
34+
const rs = await client.query({
35+
query: 'SELECT number FROM numbers(50000000)',
36+
format: 'JSON',
37+
})
38+
await expectAsync(rs.text()).toBeRejectedWith(
39+
jasmine.objectContaining({
40+
message: errorMessageMatcher,
41+
}),
42+
)
43+
expect().nothing()
44+
})
45+
})

packages/client-node/src/utils/stream.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import Stream from 'stream'
22

3+
// See https://github.com/v8/v8/commit/ea56bf5513d0cbd2a35a9035c5c2996272b8b728
4+
const MaxStringLength = Math.pow(2, 29) - 24
5+
36
export function isStream(obj: any): obj is Stream.Readable {
47
return obj !== null && typeof obj.pipe === 'function'
58
}
@@ -9,7 +12,14 @@ export async function getAsText(stream: Stream.Readable): Promise<string> {
912

1013
const textDecoder = new TextDecoder()
1114
for await (const chunk of stream) {
12-
text += textDecoder.decode(chunk, { stream: true })
15+
const decoded = textDecoder.decode(chunk, { stream: true })
16+
if (decoded.length + text.length > MaxStringLength) {
17+
throw new Error(
18+
'The response length exceeds the maximum allowed size of V8 String: ' +
19+
`${MaxStringLength}; consider limiting the amount of requested rows.`,
20+
)
21+
}
22+
text += decoded
1323
}
1424

1525
// flush

packages/client-web/src/utils/stream.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// See https://github.com/v8/v8/commit/ea56bf5513d0cbd2a35a9035c5c2996272b8b728
2+
const MaxStringLength = Math.pow(2, 29) - 24
3+
14
export function isStream(obj: any): obj is ReadableStream {
25
return (
36
obj !== null && obj !== undefined && typeof obj.pipeThrough === 'function'
@@ -13,7 +16,14 @@ export async function getAsText(stream: ReadableStream): Promise<string> {
1316

1417
while (!isDone) {
1518
const { done, value } = await reader.read()
16-
result += textDecoder.decode(value, { stream: true })
19+
const decoded = textDecoder.decode(value, { stream: true })
20+
if (decoded.length + result.length > MaxStringLength) {
21+
throw new Error(
22+
'The response length exceeds the maximum allowed size of V8 String: ' +
23+
`${MaxStringLength}; consider limiting the amount of requested rows.`,
24+
)
25+
}
26+
result += decoded
1727
isDone = done
1828
}
1929

0 commit comments

Comments
 (0)