From 4b0092ec8da869f746580deb5649ed631c45911c Mon Sep 17 00:00:00 2001
From: ruslandoga <doga.ruslan@gmail.com>
Date: Thu, 4 Jan 2024 11:13:35 +0800
Subject: [PATCH] Fix query string escaping (#147)

* add failing test

* fix query string escaping

* changelog
---
 CHANGELOG.md                  |  1 +
 lib/ch/query.ex               | 10 +++++++++-
 test/ch/query_string_test.exs | 23 +++++++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 test/ch/query_string_test.exs

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bf1f46c..b2c990d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - move rows payload (RowBinary, CSV, etc.) to SQL statement and remove pseudo-positional binds, making param names explicit https://github.com/plausible/ch/pull/143
 - drop `:headers` from `%Ch.Result{}` but add `:data` https://github.com/plausible/ch/pull/144
+- fix query string escaping for `\t`, `\\`, and `\n` https://github.com/plausible/ch/pull/147
 
 ## 0.2.2 (2023-12-23)
 
diff --git a/lib/ch/query.ex b/lib/ch/query.ex
index 2685e70..3e9bae2 100644
--- a/lib/ch/query.ex
+++ b/lib/ch/query.ex
@@ -123,7 +123,15 @@ defimpl DBConnection.Query, for: Ch.Query do
 
   defp encode_param(n) when is_integer(n), do: Integer.to_string(n)
   defp encode_param(f) when is_float(f), do: Float.to_string(f)
-  defp encode_param(b) when is_binary(b), do: escape_param([{"\t", "\\t"}, {"\n", "\\n"}], b)
+
+  # TODO possibly speed up
+  # For more info see
+  # https://clickhouse.com/docs/en/interfaces/http#tabs-in-url-parameters
+  # "escaped" format is the same as https://clickhouse.com/docs/en/interfaces/formats#tabseparated-data-formatting
+  defp encode_param(b) when is_binary(b) do
+    escape_param([{"\\", "\\\\"}, {"\t", "\\\t"}, {"\n", "\\\n"}], b)
+  end
+
   defp encode_param(b) when is_boolean(b), do: Atom.to_string(b)
   defp encode_param(%Decimal{} = d), do: Decimal.to_string(d, :normal)
   defp encode_param(%Date{} = date), do: Date.to_iso8601(date)
diff --git a/test/ch/query_string_test.exs b/test/ch/query_string_test.exs
new file mode 100644
index 0000000..109d1fa
--- /dev/null
+++ b/test/ch/query_string_test.exs
@@ -0,0 +1,23 @@
+defmodule Ch.QueryStringTest do
+  use ExUnit.Case, async: true
+
+  setup do
+    {:ok, conn: start_supervised!({Ch, database: Ch.Test.database()})}
+  end
+
+  # For more info see
+  # https://clickhouse.com/docs/en/interfaces/http#tabs-in-url-parameters
+  # "escaped" format is the same as https://clickhouse.com/docs/en/interfaces/formats#tabseparated-data-formatting
+  test "binaries are escaped properly", %{conn: conn} do
+    for s <- ["\t", "\n", "\\", "'", "\b", "\f", "\r", "\0"] do
+      assert Ch.query!(conn, "select {s:String}", %{"s" => s}).rows == [[s]]
+    end
+
+    # example from https://clickhouse.com/docs/en/interfaces/http#tabs-in-url-parameters
+    assert Ch.query!(conn, "select splitByChar('\t', 'abc\t123')").rows ==
+             [[["abc", "123"]]]
+
+    assert Ch.query!(conn, "select splitByChar('\t', {arg1:String})", %{"arg1" => "abc\t123"}).rows ==
+             [[["abc", "123"]]]
+  end
+end