ch.Parameters does not escape quotes/backslashes in string values
- Dominant language
- Go
- Stars
- 428
- Forks
- 84
- Avg merge
- 9d 6h
- Merged PRs (30d)
- 4
Description
## Summary
`ch.Parameters` formats each value with `fmt.Sprintf("'%v'", v)` and sends it as a custom setting / query parameter. That wraps the value in single quotes but does **not** escape `'` or `\` inside the value, so a string like `it's` becomes `'it's'` on the wire and can break the server's quoted-value reader (`readQuoted` / Field dump restore).
This is related to, but **not the same as**, [clickhouse-go#1834](https://github.com/ClickHouse/clickhouse-go/pull/1834) / [#1792](https://github.com/ClickHouse/clickhouse-go/issues/1792).
## Current behavior
```go
// query_params.go
Value: fmt.Sprintf("'%v'", v),
```
`proto.Parameter.Encode` then `PutString`s that text as-is. There is no Field-dump escaper.
Documented / tested contract today is that callers supply **pre-escaped** Escaped-format text (e.g. `` `line 1\\nline 2` `` for a newline), and raw tab/newline are expected to be rejected by the server. That contract can stay.
## Gap
Even under the pre-escaped contract, values that contain a literal `'` or `\` need escaping for the outer quoted dump. The helper does not do that:
| Go value | Wire today | Problem |
|---|---|---|
| `it's` | `'it's'` | premature end of quoted string |
| `a\b` | `'a\b'` | bare `\` starts an escape sequence in `readQuoted` |
Callers can work around this by building `[]proto.Parameter` by hand with correct quotes/escapes, but the helper should be safe for ordinary strings.
## Suggested fix (minimal)
Escape `\` and `'` when building the quoted value in `ch.Parameters` (inverse of the server's quoted-string reader), e.g. `it's` → `'it\'s'`, `a\b` → `'a\\b'`. Keep the existing pre-escaped Escaped-format contract for `\n` / `\t` / etc. unless maintainers want a separate literal-value API.
## Optional follow-up (design)
Whether to also auto-escape raw control characters (`\t`, `\n`, `\r`, `\0`) so ordinary Go strings round-trip, similar to clickhouse-go's `Named` path after #1834 — or keep rejecting them and document manual escaping as today.
## References
- clickhouse-go PR: https://github.com/ClickHouse/clickhouse-go/pull/1834
- clickhouse-go issue: https://github.com/ClickHouse/clickhouse-go/issues/1792
- Discussion on that PR about fixing ch-go first: this path is independent; clickhouse-go does not use `ch.Parameters` / ch-go's `Query` encoder for its `Named` / HTTP parameter binding.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in query_params.go at the fmt.Sprintf("'%v'", v) construction and inspect proto.Parameter.Encode, then compare it with the server's readQuoted / Field dump restore behavior. Add coverage for literal quotes and backslashes while preserving the existing pre-escaped \n and \t contract; done means those values round-trip without breaking quoted parsing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100