fix(clickhouse-proxy): re-inject parsed bodies by media type, not exact header match

.changeset/clickhouse-proxy-body-reinjection.md

@@ -0,0 +1,24 @@
+---
+'@hyperdx/api': patch
+---
+
+Fix `/clickhouse-proxy` request-body re-injection, which dropped every body
+whose content type was not the exact string `application/json`. The handler
+compared `req.headers['content-type']` for equality, so
+`application/json; charset=utf-8` and `application/x-www-form-urlencoded` fell
+through with `req.body` still an object: `express.json()`/`express.urlencoded()`
+had already drained the stream, `proxyReq.write(object)` threw, the throw was
+swallowed by a bare `catch`, and nothing was ever written — leaving the upstream
+blocked on a `Content-Length` worth of bytes that never arrived, so the request
+hung until the client gave up. Serialization now keys off the media type with
+parameters stripped, urlencoded bodies are re-encoded through `URLSearchParams`
+(repeated keys stay repeated instead of collapsing to `a=1%2C2`), and
+`Content-Length` is resynced to the re-serialized payload so normalization can
+no longer truncate or over-read it. Requests that no parser consumed —
+`multipart/form-data`, which `@clickhouse/client-web` emits once query params
+exceed its URL budget — are now explicitly skipped rather than throwing on
+`proxyReq.write({})`: body-parser sets `req.body = {}` before its content-type
+check, so that write always threw and forwarding only worked because httpxy went
+on to pipe the untouched stream. A write that does fail now destroys the proxied
+request, surfacing as the proxy's own 500 instead of a body-less request and an
+opaque ClickHouse 400.

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

@@ -20,8 +20,9 @@ type CapturedRequest = {
  * `@clickhouse/client-web` emits when query params exceed its URL budget
  * (regression context: ClickHouse support-escalation #8482).
  *
- * Known-latent cases (charset-suffixed JSON, urlencoded) are not covered;
- * tracked in https://github.com/hyperdxio/hyperdx/issues/2942.
+ * Charset-suffixed JSON, urlencoded bodies and Content-Length sync are covered
+ * below; before #2942 each of them hung until the client gave up. The
+ * serialization decision itself is unit-tested in clickhouseProxy.test.ts.
  */
 describe('clickhouse-proxy body forwarding', () => {
   const server = getServer();
@@ -139,6 +140,76 @@ describe('clickhouse-proxy body forwarding', () => {
     );
   });
 
+  it('forwards a charset-suffixed application/json body', async () => {
+    const { agent, team } = await getLoggedInAgent(server);
+    const connectionId = await createConnection(team._id.toString());
+
+    const payload = {
+      query: 'SELECT 1 FORMAT JSON',
+      query_params: { p0: 'x' },
+    };
+    await agent
+      .post('/clickhouse-proxy/?query_id=test-json-charset')
+      .set('x-hyperdx-connection-id', connectionId)
+      .set('Content-Type', 'application/json; charset=utf-8')
+      .send(payload)
+      .expect(200);
+
+    expect(captured).toHaveLength(1);
+    expect(JSON.parse(captured[0].body.toString())).toEqual(payload);
+    // The re-serialized payload, not the client's original framing, must set
+    // the length the upstream reads.
+    expect(captured[0].headers['content-length']).toBe(
+      String(captured[0].body.length),
+    );
+  });
+
+  it('forwards an application/x-www-form-urlencoded body', async () => {
+    const { agent, team } = await getLoggedInAgent(server);
+    const connectionId = await createConnection(team._id.toString());
+
+    await agent
+      .post('/clickhouse-proxy/?query_id=test-urlencoded')
+      .set('x-hyperdx-connection-id', connectionId)
+      .type('form')
+      .send({ query: 'SELECT 1 FORMAT JSON', param_p0: 'a b&c' })
+      .expect(200);
+
+    expect(captured).toHaveLength(1);
+    const forwarded = new URLSearchParams(captured[0].body.toString());
+    expect(forwarded.get('query')).toBe('SELECT 1 FORMAT JSON');
+    expect(forwarded.get('param_p0')).toBe('a b&c');
+    expect(captured[0].headers['content-length']).toBe(
+      String(captured[0].body.length),
+    );
+  });
+
+  it('syncs Content-Length when re-serialization changes the byte length', async () => {
+    const { agent, team } = await getLoggedInAgent(server);
+    const connectionId = await createConnection(team._id.toString());
+
+    // Pretty-printed on the wire, compact after JSON.stringify: the byte
+    // length the client announced is not the length the upstream will read.
+    const payload = { query: 'SELECT 1 FORMAT JSON' };
+    const pretty = JSON.stringify(payload, null, 4);
+    expect(Buffer.byteLength(pretty)).toBeGreaterThan(
+      Buffer.byteLength(JSON.stringify(payload)),
+    );
+
+    await agent
+      .post('/clickhouse-proxy/?query_id=test-json-length')
+      .set('x-hyperdx-connection-id', connectionId)
+      .set('Content-Type', 'application/json')
+      .send(pretty)
+      .expect(200);
+
+    expect(captured).toHaveLength(1);
+    expect(captured[0].body.toString()).toBe(JSON.stringify(payload));
+    expect(captured[0].headers['content-length']).toBe(
+      String(Buffer.byteLength(JSON.stringify(payload))),
+    );
+  });
+
   it('rejects requests without a connection id header', async () => {
     const { agent } = await getLoggedInAgent(server);
 

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

@@ -0,0 +1,148 @@
+import { planProxyBody } from '@/routers/api/clickhouseProxy';
+
+/**
+ * `bodyWasParsed` mirrors body-parser's `req._body`: true only when a parser
+ * actually drained the stream. See planProxyBody's docblock for why the shape
+ * of `req.body` cannot stand in for it.
+ */
+describe('planProxyBody', () => {
+  describe('when no parser consumed the stream', () => {
+    it('skips a multipart body-parser placeholder', () => {
+      // body-parser sets `req.body = {}` before its content-type check, so an
+      // untouched multipart request arrives looking like a parsed empty object.
+      expect(
+        planProxyBody('multipart/form-data; boundary=abc', {}, false),
+      ).toEqual({ action: 'skip' });
+    });
+
+    it('skips when there is no content-type at all', () => {
+      expect(planProxyBody(undefined, {}, false)).toEqual({ action: 'skip' });
+    });
+
+    it('skips even when the content-type looks parseable', () => {
+      // `express.json()` bails out before reading when there is no body.
+      expect(planProxyBody('application/json', {}, false)).toEqual({
+        action: 'skip',
+      });
+    });
+  });
+
+  describe('string and buffer bodies', () => {
+    it('writes a text/plain SQL body verbatim', () => {
+      const sql = 'SELECT count() FROM system.tables FORMAT JSON';
+      expect(planProxyBody('text/plain', sql, true)).toEqual({
+        action: 'write',
+        payload: sql,
+      });
+    });
+
+    it('preserves an empty string body rather than skipping it', () => {
+      expect(planProxyBody('text/plain', '', true)).toEqual({
+        action: 'write',
+        payload: '',
+      });
+    });
+
+    it('writes a buffer body unchanged', () => {
+      const buf = Buffer.from('SELECT 1');
+      expect(planProxyBody('application/octet-stream', buf, true)).toEqual({
+        action: 'write',
+        payload: buf,
+      });
+    });
+
+    it('skips a null body', () => {
+      expect(planProxyBody('application/json', null, true)).toEqual({
+        action: 'skip',
+      });
+    });
+  });
+
+  describe('json bodies', () => {
+    const payload = { query: 'SELECT 1 FORMAT JSON' };
+
+    it('serializes a bare application/json body', () => {
+      expect(planProxyBody('application/json', payload, true)).toEqual({
+        action: 'write',
+        payload: JSON.stringify(payload),
+      });
+    });
+
+    it.each([
+      'application/json; charset=utf-8',
+      'application/json;charset=UTF-8',
+      'APPLICATION/JSON; charset=utf-8',
+      'application/json ; charset=utf-8',
+    ])('serializes charset-suffixed %s', contentType => {
+      // The strict `=== 'application/json'` comparison this replaces missed
+      // every one of these, leaving an object for `proxyReq.write` to throw on.
+      expect(planProxyBody(contentType, payload, true)).toEqual({
+        action: 'write',
+        payload: JSON.stringify(payload),
+      });
+    });
+
+    it('serializes an empty parsed object', () => {
+      expect(planProxyBody('application/json', {}, true)).toEqual({
+        action: 'write',
+        payload: '{}',
+      });
+    });
+
+    it('serializes an array body', () => {
+      expect(planProxyBody('application/json', [1, 2], true)).toEqual({
+        action: 'write',
+        payload: '[1,2]',
+      });
+    });
+  });
+
+  describe('urlencoded bodies', () => {
+    it('re-serializes a parsed form body', () => {
+      const plan = planProxyBody(
+        'application/x-www-form-urlencoded',
+        { query: 'SELECT 1 FORMAT JSON', param_p0: 'a b&c' },
+        true,
+      );
+      if (plan.action !== 'write') {
+        throw new Error(`expected a write plan, got ${plan.action}`);
+      }
+
+      const forwarded = new URLSearchParams(String(plan.payload));
+      expect(forwarded.get('query')).toBe('SELECT 1 FORMAT JSON');
+      // Re-encoding has to survive a round trip, not just look plausible.
+      expect(forwarded.get('param_p0')).toBe('a b&c');
+    });
+
+    it('honours a charset suffix on the form content-type', () => {
+      expect(
+        planProxyBody(
+          'application/x-www-form-urlencoded; charset=utf-8',
+          { a: '1' },
+          true,
+        ),
+      ).toEqual({ action: 'write', payload: 'a=1' });
+    });
+
+    it('keeps repeated keys repeated instead of joining them', () => {
+      // `new URLSearchParams({a: ['1','2']})` would emit `a=1%2C2`.
+      expect(
+        planProxyBody(
+          'application/x-www-form-urlencoded',
+          { a: ['1', '2'] },
+          true,
+        ),
+      ).toEqual({ action: 'write', payload: 'a=1&a=2' });
+    });
+
+    it('drops undefined values', () => {
+      expect(
+        planProxyBody(
+          'application/x-www-form-urlencoded',
+          { a: '1', b: undefined },
+          true,
+        ),
+      ).toEqual({ action: 'write', payload: 'a=1' });
+    });
+  });
+});