Merge pull request #132 from MapleTechLabs/fix/clickhouse-http-param-escaping

fix(clickhouse-http): escape bound parameters as ClickHouse reads them

Author: Makisuo

.changeset/clickhouse-http-param-escaping.md

@@ -0,0 +1,8 @@
+---
+"@maple-dev/effect-orm": patch
+---
+
+`ClickhouseHttpClient` escapes bound parameters the way ClickHouse reads them. A string with a
+newline, tab or NUL no longer fails with `BAD_QUERY_PARAMETER`, and a backslash is no longer
+read as the start of an escape sequence, which silently changed the value. Arrays are sent as
+ClickHouse literals, so arrays of strings now bind at all (#38).

src/clickhouse-http/client.ts

@@ -132,8 +132,24 @@ const compiler = Statement.makeCompiler({
 	onCustom: () => ["", []],
 })
 
+// ClickHouse reads a scalar HTTP query parameter in the escaped (TabSeparated) format: a raw
+// newline fails to parse, and a backslash starts an escape sequence. An array parameter is
+// read as a literal instead, so only its quoted strings are escaped.
+const escapes: Record<string, string> = { "\\": "\\\\", "\0": "\\0", "\b": "\\b", "\f": "\\f", "\n": "\\n", "\r": "\\r", "\t": "\\t" }
+
+const dateValue = (value: Date): string => value.toISOString().replace("T", " ").replace("Z", "")
+
+/** An array as a ClickHouse literal: strings single-quoted, since JSON's double quotes do not parse. */
+const arrayLiteral = (value: ReadonlyArray<unknown>): string =>
+	`[${value.map((item) =>
+		item === null || item === undefined ? "NULL"
+		: Array.isArray(item) ? arrayLiteral(item)
+		: typeof item === "number" || typeof item === "bigint" || typeof item === "boolean" ? String(item)
+		: `'${(item instanceof Date ? dateValue(item) : String(item)).replace(/[\\']/g, (c) => `\\${c}`)}'`).join(",")}]`
+
 const paramValue = (value: unknown): string =>
-	value instanceof Date ? value.toISOString().replace("T", " ").replace("Z", "") : Array.isArray(value) ? JSON.stringify(value) : String(value)
+	Array.isArray(value) ? arrayLiteral(value)
+	: (value instanceof Date ? dateValue(value) : String(value)).replace(/[\\\0\b\f\n\r\t]/g, (c) => escapes[c]!)
 
 export const make = (
 	config: ClickhouseHttpClientConfig,

tests/clickhouse-http.clickhouse.test.ts

@@ -50,6 +50,18 @@ describe.skipIf(!endpoint)("ClickhouseHttpClient on ClickHouse", () => {
 			}),
 		)
 
+		// Issue #38: these used to fail to parse or arrive changed; string arrays did not parse at all.
+		it.effect("binds strings with newlines, tabs and backslashes unchanged", () =>
+			Effect.gen(function* () {
+				const sql = yield* SqlClient.SqlClient
+				const values = ["a\nb", "a\\nb", "a\\\\b", "tab\there", "cr\r", "nul\0", "é é 🙂\n|\t\\n"]
+				for (const value of values) {
+					const rows = yield* sql`SELECT ${value} AS value, ${[value, "it's"]} AS list`
+					expect(rows).toEqual([{ value, list: [value, "it's"] }])
+				}
+			}),
+		)
+
 		it.effect("streams rows", () =>
 			Effect.gen(function* () {
 				const sql = yield* SqlClient.SqlClient