Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify the response string length overflow and throw a specific error #357

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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()
})
})
12 changes: 11 additions & 1 deletion packages/client-node/src/utils/stream.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import Stream from 'stream'

// See https://github.com/v8/v8/commit/ea56bf5513d0cbd2a35a9035c5c2996272b8b728
const MaxStringLength = Math.pow(2, 29) - 24
slvrtrn marked this conversation as resolved.
Show resolved Hide resolved

export function isStream(obj: any): obj is Stream.Readable {
return obj !== null && typeof obj.pipe === 'function'
}
Expand All @@ -9,7 +12,14 @@ export async function getAsText(stream: Stream.Readable): Promise<string> {

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
Expand Down
12 changes: 11 additions & 1 deletion packages/client-web/src/utils/stream.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -13,7 +16,14 @@ export async function getAsText(stream: ReadableStream): Promise<string> {

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
}

Expand Down