Skip to content

Commit be5304d

Browse files
fix(api): forward parsed ClickHouse proxy bodies (#3383)
## Summary Fix request bodies being dropped by the ClickHouse proxy after Express consumes them, including charset-suffixed JSON and URL-encoded forms. Capture the parsers' inflated bytes before decoding or parsing, then forward those bytes with matching framing. This preserves large integers, duplicate JSON keys, empty bodies, form escapes and text encodings. Unparsed multipart streams still pass through untouched. ### How to reproduce **Starting point:** the API integration stack from CONTRIBUTING.md. Install with `yarn install --immutable`, then run `make dev-int-build`. The frontend demo and Vercel local mode have no API and cannot reproduce this path. **On `main`:** see #2942. The parser consumes charset-suffixed JSON and URL-encoded forms, then the proxy tries to write the parsed object and never forwards its bytes. The charset-JSON regression case times out. **On the earlier PR commit `9a88bbf0`:** reconstruction rounds integers such as `9007199254740993`, collapses duplicate JSON keys and changes an empty body into `{}`. The new fidelity regression cases reproduce those changes. **On this branch:** run `JEST_RETRY_TIMES=0 make dev-int FILE=clickhouseProxy`. The suite uses the authenticated API and a real upstream echo server. It checks exact JSON and form bytes, UTF-16 charset preservation, gzip inflation, chunked framing, empty bodies, duplicate charset parameters, invalid JSON rejection and binary multipart passthrough. The focused unit suite is `JEST_RETRY_TIMES=0 yarn workspace @hyperdx/api exec jest --runInBand --ci --coverage=false src/utils/__tests__/proxyBody.test.ts`. Validation: 24 unit tests and 16 real-proxy integration tests passed with retries disabled. The four new fidelity regression cases fail against the earlier PR commit `9a88bbf0`. `yarn lint:fix`, API TypeScript checks and focused ESLint passed. This run used isolated ports and the published ClickStack collector image with the repository's MongoDB/ClickHouse services. Upstream code workflows still need repository approval, and Vercel deployment needs HyperDX team authorization. ### How to test on Vercel preview N/A, API-only change. ### References Closes #2942. This follows the deferred work from #2932 and the closed, unmerged attempt in #2978.
1 parent ee9f84b commit be5304d

6 files changed

Lines changed: 550 additions & 40 deletions

File tree

‎.changeset/proxy-parsed-bodies.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@hyperdx/api': patch
3+
---
4+
5+
Forward the bytes consumed by the ClickHouse proxy's body parsers without
6+
changing JSON values, form fields or text encodings. Keep request framing in
7+
sync with inflated bytes and preserve unparsed multipart streams.

‎packages/api/src/api-app.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import usageStats from './tasks/usageStats';
2222
import { registerAITelemetry } from './utils/aiTelemetry';
2323
import logger, { expressLogger } from './utils/logger';
2424
import passport from './utils/passport';
25+
import { captureProxyBody } from './utils/proxyBody';
2526

2627
registerAITelemetry();
2728

@@ -56,9 +57,15 @@ if (!config.IS_CI && config.FRONTEND_URL) {
5657

5758
app.disable('x-powered-by');
5859
app.use(compression());
59-
app.use(express.json({ limit: '32mb' }));
60-
app.use(express.text({ limit: '32mb' }));
61-
app.use(express.urlencoded({ extended: false, limit: '32mb' }));
60+
app.use(express.json({ limit: '32mb', verify: captureProxyBody }));
61+
app.use(express.text({ limit: '32mb', verify: captureProxyBody }));
62+
app.use(
63+
express.urlencoded({
64+
extended: false,
65+
limit: '32mb',
66+
verify: captureProxyBody,
67+
}),
68+
);
6269
app.use(session(sess));
6370

6471
if (!config.IS_LOCAL_APP_MODE) {

‎packages/api/src/routers/api/__tests__/clickhouseProxy.int.test.ts‎

Lines changed: 247 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { buildLogComment } from '@hyperdx/common-utils/dist/clickhouse';
22
import http from 'http';
3+
import { gzipSync } from 'zlib';
34

45
import { getLoggedInAgent, getServer } from '@/fixtures';
56
import Connection from '@/models/connection';
7+
import logger from '@/utils/logger';
68

79
// Use the real proxy instead of the blanket mock in jest.setup.ts; the int
810
// jest config transpiles it (see jest.int.config.js).
@@ -16,14 +18,11 @@ type CapturedRequest = {
1618
};
1719

1820
/**
19-
* Pins what /clickhouse-proxy forwards upstream: text/plain re-injection, the
20-
* raw-stream passthrough of multipart/form-data bodies, which
21+
* Pins what /clickhouse-proxy forwards upstream: captured parser bytes with
22+
* valid framing, the raw-stream passthrough of multipart/form-data bodies, which
2123
* `@clickhouse/client-web` emits when query params exceed its URL budget
2224
* (regression context: ClickHouse support-escalation #8482), and the
2325
* decode/re-encode the query string goes through in `pathRewrite`.
24-
*
25-
* Known-latent cases (charset-suffixed JSON, urlencoded) are not covered;
26-
* tracked in https://github.com/hyperdxio/hyperdx/issues/2942.
2726
*/
2827
describe('clickhouse-proxy forwarding', () => {
2928
const server = getServer();
@@ -66,6 +65,7 @@ describe('clickhouse-proxy forwarding', () => {
6665
});
6766

6867
afterAll(async () => {
68+
upstream.closeAllConnections();
6969
await new Promise<void>((resolve, reject) =>
7070
upstream.close(err => (err ? reject(err) : resolve())),
7171
);
@@ -101,6 +101,217 @@ describe('clickhouse-proxy forwarding', () => {
101101
expect(captured[0].headers['x-clickhouse-user']).toBe('default');
102102
});
103103

104+
it.each(['application/json', 'application/json; charset=utf-8'])(
105+
'preserves %s bytes and Content-Length',
106+
async contentType => {
107+
const { agent, team } = await getLoggedInAgent(server);
108+
const connectionId = await createConnection(team._id.toString());
109+
const body = {
110+
query: "SELECT '東京 🍰' FORMAT JSON",
111+
param_name: 'café',
112+
};
113+
const original = JSON.stringify(body, null, 2);
114+
expect(Buffer.byteLength(original)).toBeGreaterThan(original.length);
115+
116+
await agent
117+
.post('/clickhouse-proxy/?query_id=test-json')
118+
.set('x-hyperdx-connection-id', connectionId)
119+
.set('Content-Type', contentType)
120+
.send(original)
121+
.timeout({ response: 5000, deadline: 10000 })
122+
.expect(200);
123+
124+
expect(captured).toHaveLength(1);
125+
expect(captured[0].body).toEqual(Buffer.from(original));
126+
expect(captured[0].headers['content-type']).toBe(contentType);
127+
expect(captured[0].headers['content-length']).toBe(
128+
String(Buffer.byteLength(original)),
129+
);
130+
expect(captured[0].headers['transfer-encoding']).toBeUndefined();
131+
},
132+
);
133+
134+
it('preserves parsed UTF-16LE JSON bytes and charset', async () => {
135+
const { agent, team } = await getLoggedInAgent(server);
136+
const connectionId = await createConnection(team._id.toString());
137+
const body = { query: "SELECT '東京 🍰' FORMAT JSON" };
138+
const original = Buffer.from(JSON.stringify(body, null, 2), 'utf16le');
139+
140+
const proxyRequest = agent
141+
.post('/clickhouse-proxy/?query_id=test-utf16-json')
142+
.set('x-hyperdx-connection-id', connectionId)
143+
.set('Content-Type', 'application/json; charset=utf-16le')
144+
.set('Content-Length', String(original.length))
145+
.timeout({ response: 5000, deadline: 10000 });
146+
proxyRequest.write(original);
147+
await proxyRequest.expect(200);
148+
149+
expect(captured).toHaveLength(1);
150+
expect(captured[0].body).toEqual(original);
151+
expect(captured[0].headers['content-type']).toBe(
152+
'application/json; charset=utf-16le',
153+
);
154+
expect(captured[0].headers['content-length']).toBe(String(original.length));
155+
expect(captured[0].headers['content-encoding']).toBeUndefined();
156+
});
157+
158+
it('forwards parsed gzip JSON without the original content encoding', async () => {
159+
const { agent, team } = await getLoggedInAgent(server);
160+
const connectionId = await createConnection(team._id.toString());
161+
const body = { query: "SELECT 'café' FORMAT JSON" };
162+
const serialized = Buffer.from(JSON.stringify(body, null, 2));
163+
const original = gzipSync(serialized);
164+
165+
const proxyRequest = agent
166+
.post('/clickhouse-proxy/?query_id=test-gzip-json')
167+
.set('x-hyperdx-connection-id', connectionId)
168+
.set('Content-Type', 'application/json; charset=utf-8')
169+
.set('Content-Encoding', 'gzip')
170+
.set('Content-Length', String(original.length))
171+
.timeout({ response: 5000, deadline: 10000 });
172+
proxyRequest.write(original);
173+
await proxyRequest.expect(200);
174+
175+
expect(captured).toHaveLength(1);
176+
expect(captured[0].body).toEqual(serialized);
177+
expect(captured[0].headers['content-type']).toBe(
178+
'application/json; charset=utf-8',
179+
);
180+
expect(captured[0].headers['content-length']).toBe(
181+
String(serialized.length),
182+
);
183+
expect(captured[0].headers['content-encoding']).toBeUndefined();
184+
});
185+
186+
it.each([
187+
'application/x-www-form-urlencoded',
188+
'application/x-www-form-urlencoded; charset=utf-8',
189+
])('preserves %s bytes and repeated form keys', async contentType => {
190+
const { agent, team } = await getLoggedInAgent(server);
191+
const connectionId = await createConnection(team._id.toString());
192+
const original =
193+
'query=SELECT%20%27caf%C3%A9%27&param=first&param=second%20value&empty=&literal=%2B%26%3D';
194+
const entries = [
195+
['query', "SELECT 'café'"],
196+
['param', 'first'],
197+
['param', 'second value'],
198+
['empty', ''],
199+
['literal', '+&='],
200+
];
201+
202+
await agent
203+
.post('/clickhouse-proxy/?query_id=test-urlencoded')
204+
.set('x-hyperdx-connection-id', connectionId)
205+
.set('Content-Type', contentType)
206+
.send(original)
207+
.timeout({ response: 5000, deadline: 10000 })
208+
.expect(200);
209+
210+
expect(captured).toHaveLength(1);
211+
expect(captured[0].body.toString()).toBe(original);
212+
expect([...new URLSearchParams(captured[0].body.toString())]).toEqual(
213+
entries,
214+
);
215+
expect(captured[0].headers['content-type']).toBe(contentType);
216+
expect(captured[0].headers['content-length']).toBe(
217+
String(Buffer.byteLength(original)),
218+
);
219+
});
220+
221+
it('keeps chunked framing when forwarding captured JSON bytes', async () => {
222+
const { agent, team } = await getLoggedInAgent(server);
223+
const connectionId = await createConnection(team._id.toString());
224+
const body = { query: "SELECT 'café' FORMAT JSON" };
225+
const original = JSON.stringify(body, null, 2);
226+
const proxyRequest = agent
227+
.post('/clickhouse-proxy/?query_id=test-chunked-json')
228+
.set('x-hyperdx-connection-id', connectionId)
229+
.set('Content-Type', 'application/json; charset=utf-8')
230+
.set('Transfer-Encoding', 'chunked')
231+
.timeout({ response: 5000, deadline: 10000 });
232+
233+
proxyRequest.write(original.slice(0, 10));
234+
proxyRequest.write(original.slice(10));
235+
await proxyRequest.expect(200);
236+
237+
expect(captured).toHaveLength(1);
238+
expect(captured[0].body.toString()).toBe(original);
239+
expect(captured[0].headers['transfer-encoding']).toBe('chunked');
240+
expect(captured[0].headers['content-length']).toBeUndefined();
241+
});
242+
243+
it.each(['/clickhouse-proxy/', '/CLICKHOUSE-PROXY/'])(
244+
'preserves JSON number literals and duplicate keys on %s',
245+
async route => {
246+
const { agent, team } = await getLoggedInAgent(server);
247+
const connectionId = await createConnection(team._id.toString());
248+
const original =
249+
'{"id":9007199254740993,"value":1.0,"value":1e2,"query":"SELECT 1"}';
250+
251+
await agent
252+
.post(`${route}?query_id=test-json-fidelity`)
253+
.set('x-hyperdx-connection-id', connectionId)
254+
.set('Content-Type', 'application/json; charset=utf-8')
255+
.send(original)
256+
.timeout({ response: 5000, deadline: 10000 })
257+
.expect(200);
258+
259+
expect(captured).toHaveLength(1);
260+
expect(captured[0].body).toEqual(Buffer.from(original));
261+
},
262+
);
263+
264+
it('preserves an empty JSON body for query-string SQL', async () => {
265+
const { agent, team } = await getLoggedInAgent(server);
266+
const connectionId = await createConnection(team._id.toString());
267+
268+
await agent
269+
.post('/clickhouse-proxy/?query=SELECT+1')
270+
.set('x-hyperdx-connection-id', connectionId)
271+
.set('Content-Type', 'application/json')
272+
.set('Content-Length', '0')
273+
.timeout({ response: 5000, deadline: 10000 })
274+
.expect(200);
275+
276+
expect(captured).toHaveLength(1);
277+
expect(captured[0].body).toEqual(Buffer.alloc(0));
278+
expect(captured[0].headers['content-length']).toBe('0');
279+
});
280+
281+
it('preserves bytes under duplicate charset parameters', async () => {
282+
const { agent, team } = await getLoggedInAgent(server);
283+
const connectionId = await createConnection(team._id.toString());
284+
const contentType = 'application/json; charset=utf-8; charset=utf-16le';
285+
const original = Buffer.from('{"query":"SELECT 1"}', 'utf16le');
286+
const proxyRequest = agent
287+
.post('/clickhouse-proxy/?query_id=test-duplicate-charset')
288+
.set('x-hyperdx-connection-id', connectionId)
289+
.set('Content-Type', contentType)
290+
.set('Content-Length', String(original.length))
291+
.timeout({ response: 5000, deadline: 10000 });
292+
proxyRequest.write(original);
293+
await proxyRequest.expect(200);
294+
295+
expect(captured).toHaveLength(1);
296+
expect(captured[0].body).toEqual(original);
297+
expect(captured[0].headers['content-type']).toBe(contentType);
298+
expect(captured[0].headers['content-length']).toBe(String(original.length));
299+
});
300+
301+
it('still rejects invalid JSON before forwarding', async () => {
302+
const { agent, team } = await getLoggedInAgent(server);
303+
const connectionId = await createConnection(team._id.toString());
304+
305+
await agent
306+
.post('/clickhouse-proxy/')
307+
.set('x-hyperdx-connection-id', connectionId)
308+
.set('Content-Type', 'application/json')
309+
.send('{invalid')
310+
.expect(400);
311+
312+
expect(captured).toHaveLength(0);
313+
});
314+
104315
it('forwards a large multipart/form-data body byte-for-byte (unparsed stream passthrough)', async () => {
105316
const { agent, team } = await getLoggedInAgent(server);
106317
const connectionId = await createConnection(team._id.toString());
@@ -120,25 +331,40 @@ describe('clickhouse-proxy forwarding', () => {
120331
`some.rather.long.map.attribute.key.number.${i}\r\n`,
121332
);
122333
}
123-
parts.push(`--${boundary}--\r\n`);
124-
const body = parts.join('');
334+
const body = Buffer.concat([
335+
Buffer.from(parts.join('')),
336+
Buffer.from(
337+
`--${boundary}\r\n` +
338+
`Content-Disposition: form-data; name="file"; filename="bytes.bin"\r\n` +
339+
`Content-Type: application/octet-stream\r\n\r\n`,
340+
),
341+
Buffer.from([0x00, 0xff, 0x80, 0x0d, 0x0a]),
342+
Buffer.from(`\r\n--${boundary}--\r\n`),
343+
]);
125344
expect(Buffer.byteLength(body)).toBeGreaterThan(4096);
126345

127-
await agent
128-
.post('/clickhouse-proxy/?query_id=test-multipart')
129-
.set('x-hyperdx-connection-id', connectionId)
130-
.set('Content-Type', `multipart/form-data; boundary=${boundary}`)
131-
.send(body)
132-
.expect(200);
346+
const loggerError = jest.spyOn(logger, 'error');
347+
try {
348+
await agent
349+
.post('/clickhouse-proxy/?query_id=test-multipart')
350+
.set('x-hyperdx-connection-id', connectionId)
351+
.set('Content-Type', `multipart/form-data; boundary=${boundary}`)
352+
.send(body)
353+
.timeout({ response: 5000, deadline: 10000 })
354+
.expect(200);
133355

134-
expect(captured).toHaveLength(1);
135-
expect(captured[0].headers['content-type']).toBe(
136-
`multipart/form-data; boundary=${boundary}`,
137-
);
138-
expect(captured[0].body.toString()).toBe(body);
139-
expect(captured[0].headers['content-length']).toBe(
140-
String(Buffer.byteLength(body)),
141-
);
356+
expect(captured).toHaveLength(1);
357+
expect(captured[0].headers['content-type']).toBe(
358+
`multipart/form-data; boundary=${boundary}`,
359+
);
360+
expect(captured[0].body).toEqual(body);
361+
expect(captured[0].headers['content-length']).toBe(
362+
String(Buffer.byteLength(body)),
363+
);
364+
expect(loggerError).not.toHaveBeenCalled();
365+
} finally {
366+
loggerError.mockRestore();
367+
}
142368
});
143369

144370
// The browser client ships `log_comment` as a URL query param, and

‎packages/api/src/routers/api/clickhouseProxy.ts‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { getNonNullUserWithTeam } from '@/middleware/auth';
1111
import { validateRequestHeaders } from '@/middleware/validation';
1212
import { recordOperationOutcome } from '@/utils/instrumentation';
1313
import logger from '@/utils/logger';
14+
import { writeProxyBody } from '@/utils/proxyBody';
1415
import { IPV6_BRACKET_RE, isPrivateIp } from '@/utils/validators';
1516
import { objectIdSchema } from '@/utils/zod';
1617

@@ -261,22 +262,7 @@ const proxyMiddleware: RequestHandler =
261262
return res.sendStatus(405);
262263
}
263264

264-
let body = _req.body;
265-
if (_req.headers['content-type'] === 'application/json') {
266-
try {
267-
body = JSON.stringify(body);
268-
} catch (e) {
269-
console.error(e);
270-
}
271-
}
272-
273-
try {
274-
proxyReq.write(body);
275-
} catch {
276-
console.error(
277-
`clickhouseProxy error writing body, body is type ${typeof body}`,
278-
);
279-
}
265+
writeProxyBody(proxyReq, _req);
280266
},
281267
proxyRes: (proxyRes, _req, res) => {
282268
const startedAt = (res as Response).locals?.hdxProxyStartedAt;

0 commit comments

Comments
 (0)