ClickHouse / ClickHouse/clickhouse-cpp
Exceptions produced by the library leave the socket in inconsistent state
- Dominant language
- C
- Stars
- 382
- Forks
- 209
- Avg merge
- 3h 58m
- Merged PRs (30d)
- 12
Description
## Problem
Some error paths in `clickhouse/client.cpp` stop processing a server response without consuming the rest of that response and without invalidating the connection. A later operation can then interpret bytes from the previous response as the start of a new packet.
The negotiated protocol revision does not provide a generic length around each top-level packet. After an unknown or partially decoded packet, the client usually cannot determine where the next packet begins and cannot safely skip the remaining payload.
## Count
There are **22 distinct unsafe paths** under the following counting rule:
> A path is counted when it starts consuming, or leaves pending, a server response; then exits normal response processing; and leaves the same input stream installed without draining or invalidating it.
The paths divide into these categories:
| Category | Count |
| --- | ---: |
| Packet decoding and framing failures | 14 |
| User callbacks throwing during a response | 6 |
| `Ping()` abandoning a response | 1 |
| Failed handshake during `ResetConnection()` | 1 |
| **Total** | **22** |
The path count is more useful because several failures return `false` or `std::monostate` after partial decoding and only throw later, while callback and compression exceptions originate outside the visible `throw` statements in `client.cpp`.
## Packet Decoding and Framing
### 1. Invalid top-level packet type varint
`ReceivePacket()` returns `std::monostate` when the packet type cannot be read (`clickhouse/client.cpp:683-688`). `ReadVarint64()` consumes at most ten bytes and has a `TODO skip invalid` for an overlong varint (`clickhouse/base/wire_format.cpp:50-68`). Continuation bytes and the packet body can therefore remain unread.
`NextBlock()` treats `std::monostate` as completion, resets the logical state, and permits another operation.
### 2. Unsupported top-level packet type
The default branch in `ReceivePacket()` throws immediately after reading only the packet type (`clickhouse/client.cpp:832-834`). It does not consume the packet payload or invalidate the connection.
The protocol declares several packet types that this switch does not handle, including `Totals`, `Extremes`, `TablesStatusResponse`, `PartUUIDs`, and `ReadTaskRequest` (`clickhouse/protocol.h:15-23`). `Totals` and `Extremes` are particularly relevant because they contain blocks and may occur in ordinary query results.
### 3. Unexpected `Hello` packet
The general response decoder treats `Hello` as a marker with no payload (`clickhouse/client.cpp:774-776`). A real server `Hello` has a payload containing the server name, versions, revision, timezone, display name, and patch version, as shown by `ReceiveHello()` (`clickhouse/client.cpp:1185-1224`).
If `Hello` appears during another operation, its payload is left in the stream and is interpreted as subsequent packet types.
### 4. Malformed `Data` envelope or block framing
Failures while reading the table-name prefix, block information, dimensions, column names, or type names stop before the rest of the block is consumed (`clickhouse/client.cpp:857-945`). `ReceivePacket()` then throws at `clickhouse/client.cpp:697`.
For example, `ReadString()` and `SkipString()` reject lengths greater than `0x00ffffff` without skipping the declared body. That body and the rest of the block remain unread.
### 5. Unsupported custom serialization
When a column has non-empty custom serialization metadata, `ReadBlock()` throws immediately (`clickhouse/client.cpp:906-913`). It does not consume that metadata, the column values, later columns, or later response packets.
### 6. Unsupported or malformed column type
After the column name and type string are read, `CreateColumnByType()` can fail or throw. A null result causes an exception at `clickhouse/client.cpp:923`. Because the column data has not been decoded, its values and the remainder of the block stay unread.
### 7. Column loading or block insertion failure
A column loader can return `false` or throw while processing its values. The explicit failure is raised at `clickhouse/client.cpp:916-923`. The unread data can include the rest of the current value, remaining rows, later columns, and later packets.
`Block::AppendColumn()` can also throw after one column has been decoded, while later columns are still pending.
### 8. Partially decoded `ProfileInfo`
Every failed field read in the `ProfileInfo` decoder returns `std::monostate` (`clickhouse/client.cpp:710-736`). The remaining fields and later response packets are not drained.
### 9. Partially decoded `Progress`
The `Progress` decoder has the same behavior for rows, bytes, total rows, written rows, and written bytes (`clickhouse/client.cpp:739-767`). A failed field read abandons the packet and response.
### 10. Partially decoded `Log`
A failure while skipping the log tag or reading its block returns `std::monostate` (`clickhouse/client.cpp:785-800`). Because `Log` is nonterminal, later logs, results, profile information, or `EndOfStream` may also remain pending.
### 11. Partially decoded `TableColumns`
The two length-prefixed strings in `TableColumns` are skipped independently (`clickhouse/client.cpp:803-813`). Failure in either operation abandons the rest of the packet and response.
### 12. Partially decoded `ProfileEvents`
Failure while skipping the initial string or reading the profile-events block returns `std::monostate` (`clickhouse/client.cpp:816-829`). `ProfileEvents` is nonterminal, so later packets may remain even if most of the current packet was read successfully.
### 13. Malformed or nested server `Exception`
`ReceiveException()` reads the exception fields using a short-circuiting chain (`clickhouse/client.cpp:959-968`). If one field fails, subsequent fields are not read. With the default options, it then throws `ServerError` regardless of whether decoding completed (`clickhouse/client.cpp:970-976`). With exception rethrowing disabled, `ReceivePacket()` throws `ProtocolError` at line `705`.
The decoder reads the `has_nested` flag but does not recursively consume a nested exception. If a server sends `has_nested = true`, nested bytes remain in the stream.
### 14. Compressed-input failure or abandonment
Compressed block decoding can fail because of an unsupported compression method, an excessive size, a short frame, a checksum mismatch, or a decompression error (`clickhouse/base/compressed.cpp:57-132`).
If block decoding throws with decompressed bytes still buffered, `CompressedInput` suppresses its own "some data was not read" exception during stack unwinding (`clickhouse/base/compressed.cpp:35-44`). Those decompressed bytes are discarded, while later compressed frames or response packets may remain on the underlying input stream.
## Callback Exceptions
Six callbacks can throw while a query response is being processed:
| Callback | Dispatch location |
| --- | --- |
| `OnData` | `clickhouse/client.cpp:949-950` |
| `OnDataCancelable` | `clickhouse/client.cpp:950-953` |
| `OnProfile` | `clickhouse/client.cpp:732-733` |
| `OnProgress` | `clickhouse/client.cpp:763-764` |
| `OnServerLog` | `clickhouse/client.cpp:797-798` |
| `OnProfileEvents` | `clickhouse/client.cpp:826-827` |
The current packet is generally fully decoded before these callbacks run, but the query is not necessarily complete. If a callback throws, `NextBlock()` resets the logical state and rethrows without sending `Cancel`, draining to `EndOfStream`, or invalidating the connection. Later packets from the query can then interfere with the next operation.
A normal `false` return from `OnDataCancelable` is different: it calls `SendCancel()` and normal processing continues so the response can be drained.
`OnServerException` is not counted as a separate callback case. For a valid terminal exception packet, the packet has already been fully consumed before the callback runs. It is unsafe only when combined with the malformed exception case described above.
## Other Response Paths
### 21. `Ping()` processes only one packet
`Ping()` calls `ProcessPacket()` once and requires that packet to be `Pong` (`clickhouse/client.cpp:596-609`). If an unexpected nonterminal packet is received before `Pong`, `Ping()` throws while leaving `Pong` and any later packets unread. The client remains logically idle and can immediately be reused.
### 22. Failed handshake during `ResetConnection()`
`ResetConnection()` installs new streams and sets the client state to idle before calling `Handshake()` (`clickhouse/client.cpp:612-618`). `ReceiveHello()` can return `false` after partially reading a malformed `Hello` or after reading only the type of an unexpected packet (`clickhouse/client.cpp:1185-1230`).
When this occurs through the public `ResetConnection()` API, the client object survives with the failed input stream still installed. A later operation can read the remainder of the failed handshake.
## Normal Server Exceptions
A normal server `Exception` packet is not inherently unsafe. For the expected non-nested format, `ReceiveException()` reads the code, name, display text, stack trace, and nesting flag before invoking the callback or throwing (`clickhouse/client.cpp:959-981`). A server exception is terminal, so no `EndOfStream` packet needs to be drained afterward.
The test at `ut/client_ut.cpp:1666-1682` verifies this behavior by receiving a server exception and then successfully executing another query on the same client.
The unsafe server-exception cases are malformed fields, rejected string lengths, and nested exception data that the decoder does not consume.
## Excluded Cases
Pre-I/O validation errors, such as starting an operation while the client is not idle, do not create unread server data and are not included.
Generic failures such as `std::bad_alloc` can occur almost anywhere during response decoding and can also abandon a response. They are not counted as separate finite source paths.
Exceptions after partial outbound serialization are another related issue. For example, the old-server checks at `clickhouse/client.cpp:1056` and `clickhouse/client.cpp:1077` happen after part of a query packet has already been written. They can corrupt the outbound stream, but they are excluded from the 22 inbound-stream cases described here.
## Recovery Implication
After a malformed, unsupported, or partially decoded packet, reliably draining the response is generally impossible because the client no longer knows the packet boundary. The safe recovery is to invalidate and replace the connection.
For user callback exceptions, cancellation followed by draining may be possible, but only if exception handling can guarantee that the protocol stream remains decodable. Closing or invalidating the connection is the simpler and more robust fallback.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading the response paths in clickhouse/client.cpp, including ReceivePacket(), NextBlock(), Ping(), ResetConnection(), and the callback dispatches; compare framing behavior in wire_format.cpp and compressed.cpp with packet definitions in protocol.h. Review ut/client_ut.cpp:1666-1682, then map tests for the listed failure categories and define done as preventing reuse of a stream after unsafe response processing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend-api-design, networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100