Repository navigation
fix(clickhouse-http): escape bound parameters as ClickHouse reads them - #132
Confidence 10/10 · No issues found
🟢 Confidence 10/10 · safe to merge. I exercised the new escaping on adversarial inputs and it round-trips; the live test covers both layers, leaving only the duplicated literal escaping.
1 note · tests covered · risk medium
Rewrites HTTP param serialization in ClickableHttpClient so scalars are TabSeparated-escaped and arrays are sent as ClickHouse literals. The escaping is correct and the live test pins both layers; safe to merge.
paramValuenow escapes scalars as ClickHouse's escaped (TabSeparated) format- Array params are sent as ClickHouse literals instead of
JSON.stringify - New live test round-trips control characters and backslashes as scalars and arrays
Findings
🔵 Note · F1 · Array elements re-escape ClickHouse strings instead of quoteClickHouseString
maintainability · src/clickhouse-http/client.ts:148
Array elements get their own backslash-and-quote escaping inlined here, while src/sql/sql-fragment.ts:24 already exports escapeClickHouseString for ClickHouse literals — including the ; hex escape added there for gateways that split statements on the raw byte. The two copies must be kept in sync by hand, and a hazard fixed in one is missed by the other.
Import `quoteClickHouseString` from `../sql/index` and use it for the element at line 148, dropping the inline `.replace(/[\\']/g, …)`.
What was checked
- Ran the new escaping on adversarial inputs: trailing backslash,
it's, nested arrays, NUL — each round-trips byte-identically - Escape map mirrors ClickHouse's own
writeEscapedString(\b,\f,\0included);'stays raw, as the existingo'brientest relies on - CI sets
EFFECT_ORM_CLICKHOUSE_URLfortest:release, so the opt-in live suite runs against both ClickHouse builders
790635b · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
Check notice on line 148 in src/clickhouse-http/client.ts
maple-review-bot / Maple / review
maintainability: Array elements re-escape ClickHouse strings instead of `quoteClickHouseString`
Array elements get their own backslash-and-quote escaping inlined here, while `src/sql/sql-fragment.ts:24` already exports `escapeClickHouseString` for ClickHouse literals — including the `;` hex escape added there for gateways that split statements on the raw byte. The two copies must be kept in sync by hand, and a hazard fixed in one is missed by the other.