hyperdxio / hyperdxio/hyperdx

clickhouse-proxy: body re-injection breaks charset-suffixed JSON and urlencoded requests; multipart forwarding works only accidentally

Open
#2,942 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
9.9k
Forks
471
Avg merge
2d 4h
Merged PRs (30d)
117

Description

Context

While investigating ClickHouse/support-escalation#8482 (filter sidebar values vanishing behind query proxies — fixed client-side in #2932 by inlining facet keys), several latent bugs surfaced in the /clickhouse-proxy request-body re-injection (packages/api/src/routers/api/clickhouseProxy.ts, proxyReq handler). #2932 initially hardened them but was scoped down to keep that file untouched; this issue tracks the deferred fixes.

Latent bugs

The handler re-injects req.body after the express parsers have (possibly) consumed the request stream:

let body = _req.body;
if (_req.headers['content-type'] === 'application/json') {
  try { body = JSON.stringify(body); } catch (e) { console.error(e); }
}
try {
  proxyReq.write(body);
} catch {
  console.error(`clickhouseProxy error writing body, body is type ${typeof body}`);
}
  1. Strict content-type === 'application/json' comparison misses charset-suffixed headers (application/json; charset=utf-8). express.json() consumed the stream, the object is never re-serialized, proxyReq.write(object) throws, nothing is piped (stream already consumed) — the upstream waits for Content-Length bytes that never arrive (hang/timeout or empty-query 400).
  2. application/x-www-form-urlencoded bodies are parsed and then dropped — same failure mode as (1): parsed to an object, never re-serialized, stream consumed.
  3. Unparsed bodies (e.g. multipart/form-data) hit proxyReq.write({}) — body-parser initializes req.body = {} even for content types it skips, so the write throws on every such request. The error is swallowed with a console.error and forwarding only works accidentally because httpxy subsequently pipes the still-unconsumed raw stream. The passthrough behavior is pinned by clickhouseProxy.int.test.ts; the noisy caught-throw should be replaced with an explicit skip.
  4. Content-Length is not synced when the re-injected payload's byte length differs from the original request header (e.g. JSON.stringify normalization), risking truncated/over-long upstream reads.
  5. Write failures are silent: the request proceeds body-less instead of failing loudly, surfacing to users as an opaque 400 from ClickHouse.

Suggested fix

In the proxyReq handler: write string/Buffer bodies as-is; startsWith('application/json')JSON.stringify; urlencoded objects → URLSearchParams re-serialization; anything unparsed → skip the write and let the raw stream pipe; sync Content-Length on re-injected payloads (when no transfer-encoding); destroy the proxied request with a real error on write failure. A working implementation (with passing integration tests for all five behaviors) exists in #2932's history: hyperdxio/hyperdx@335c8f96c.

Coverage

packages/api/src/routers/api/__tests__/clickhouseProxy.int.test.ts pins the currently-working behaviors (text/plain verbatim, multipart passthrough); the JSON-charset and urlencoded cases from 335c8f96c can be restored alongside the fix.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/api/src/routers/api/clickhouseProxy.ts at the proxyReq handler, then read packages/api/src/routers/api/tests/clickhouseProxy.int.test.ts and commit 335c8f96c. Restore coverage for JSON with a charset, urlencoded bodies, content-length handling, write failures, and multipart passthrough; the integration tests should pass for all five behaviors.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, typescript
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.