Skip to content

Commit ad6abec

Browse files
authored
fix: Add explicit parameters limit in PostgreSQL and Redshift drivers (#8784)
1 parent e49b5f5 commit ad6abec

File tree

3 files changed

+76
-1
lines changed

3 files changed

+76
-1
lines changed

packages/cubejs-postgres-driver/src/PostgresDriver.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,8 @@ export class PostgresDriver<Config extends PostgresDriverConfiguration = Postgre
296296
values: unknown[],
297297
{ highWaterMark }: StreamOptions
298298
): Promise<StreamTableDataWithTypes> {
299+
PostgresDriver.checkValuesLimit(values);
300+
299301
const conn = await this.pool.connect();
300302

301303
try {
@@ -324,7 +326,22 @@ export class PostgresDriver<Config extends PostgresDriverConfiguration = Postgre
324326
}
325327
}
326328

329+
protected static checkValuesLimit(values?: unknown[]) {
330+
// PostgreSQL protocol allows sending up to 65535 params in a single bind message
331+
// See https://github.com/postgres/postgres/blob/REL_16_0/src/backend/tcop/postgres.c#L1698-L1708
332+
// See https://github.com/postgres/postgres/blob/REL_16_0/src/backend/libpq/pqformat.c#L428-L431
333+
// But 'pg' module does not check for params count, and ends up sending incorrect bind message
334+
// See https://github.com/brianc/node-postgres/blob/92cb640fd316972e323ced6256b2acd89b1b58e0/packages/pg-protocol/src/serializer.ts#L155
335+
// See https://github.com/brianc/node-postgres/blob/92cb640fd316972e323ced6256b2acd89b1b58e0/packages/pg-protocol/src/buffer-writer.ts#L32-L37
336+
const length = (values?.length ?? 0);
337+
if (length >= 65536) {
338+
throw new Error(`PostgreSQL protocol does not support more than 65535 parameters, but ${length} passed`);
339+
}
340+
}
341+
327342
protected async queryResponse(query: string, values: unknown[]) {
343+
PostgresDriver.checkValuesLimit(values);
344+
328345
const conn = await this.pool.connect();
329346

330347
try {

packages/cubejs-postgres-driver/test/PostgresDriver.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import { PostgresDriver } from '../src';
44

55
const streamToArray = require('stream-to-array');
66

7+
function largeParams(): Array<string> {
8+
return new Array(65536).fill('foo');
9+
}
10+
711
describe('PostgresDriver', () => {
812
let container: StartedTestContainer;
913
let driver: PostgresDriver;
@@ -56,6 +60,14 @@ describe('PostgresDriver', () => {
5660
]);
5761
});
5862

63+
test('too many params', async () => {
64+
await expect(
65+
driver.query(`SELECT 'foo'::TEXT;`, largeParams())
66+
)
67+
.rejects
68+
.toThrow('PostgreSQL protocol does not support more than 65535 parameters, but 65536 passed');
69+
});
70+
5971
test('stream', async () => {
6072
await driver.uploadTable(
6173
'test.streaming_test',
@@ -116,6 +128,20 @@ describe('PostgresDriver', () => {
116128
}
117129
});
118130

131+
test('stream (too many params)', async () => {
132+
try {
133+
await driver.stream('select * from test.streaming_test', largeParams(), {
134+
highWaterMark: 1000,
135+
});
136+
137+
throw new Error('stream must throw an exception');
138+
} catch (e: any) {
139+
expect(e.message).toEqual(
140+
'PostgreSQL protocol does not support more than 65535 parameters, but 65536 passed'
141+
);
142+
}
143+
});
144+
119145
// Note: This test MUST be the last in the list.
120146
test('release', async () => {
121147
expect(async () => {

packages/cubejs-redshift-driver/src/RedshiftDriver.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@
66

77
import { getEnv } from '@cubejs-backend/shared';
88
import { PostgresDriver, PostgresDriverConfiguration } from '@cubejs-backend/postgres-driver';
9-
import { DownloadTableCSVData, DriverCapabilities, UnloadOptions } from '@cubejs-backend/base-driver';
9+
import {
10+
DownloadTableCSVData,
11+
DriverCapabilities,
12+
StreamOptions,
13+
StreamTableDataWithTypes,
14+
UnloadOptions
15+
} from '@cubejs-backend/base-driver';
1016
import crypto from 'crypto';
1117

1218
interface RedshiftDriverExportRequiredAWS {
@@ -91,6 +97,32 @@ export class RedshiftDriver extends PostgresDriver<RedshiftDriverConfiguration>
9197
};
9298
}
9399

100+
protected static checkValuesLimit(values?: unknown[]) {
101+
// Redshift server is not exactly compatible with PostgreSQL protocol
102+
// And breaks after 32767 parameter values with `there is no parameter $-32768`
103+
// This is a bug/misbehaviour on server side, nothing we can do besides generate a more meaningful error
104+
const length = (values?.length ?? 0);
105+
if (length >= 32768) {
106+
throw new Error(`Redshift server does not support more than 32767 parameters, but ${length} passed`);
107+
}
108+
}
109+
110+
public override async stream(
111+
query: string,
112+
values: unknown[],
113+
options: StreamOptions
114+
): Promise<StreamTableDataWithTypes> {
115+
RedshiftDriver.checkValuesLimit(values);
116+
117+
return super.stream(query, values, options);
118+
}
119+
120+
protected override async queryResponse(query: string, values: unknown[]) {
121+
RedshiftDriver.checkValuesLimit(values);
122+
123+
return super.queryResponse(query, values);
124+
}
125+
94126
protected getExportBucket(
95127
dataSource: string,
96128
): RedshiftDriverExportAWS | undefined {

0 commit comments

Comments
 (0)