Skip to content

Commit 0963453

Browse files
authored
Fix tsfile-cli clean-build race and harden CLI validation (#836)
Build: tsfile_cli_obj compiled against the staged include tree without depending on the copy_* targets that populate it, so a clean parallel build could fail with "'common/db_common.h' file not found" (deterministic via `make tsfile_cli_obj` in a fresh build dir). Add the missing add_dependencies edge. CLI hardening: - reject --offset on non-row commands and --model on write; name each read-only flag rejected by write instead of a lumped message - error when -d/-t does not match the file's data model in head/cat/sample/schema instead of silently ignoring the filter - include the storage error code in open/create failure messages - detect numeric flag overflow (ERANGE) in -n/--offset/--start/--end/--seed - cap the sample reservoir pre-allocation so a huge -n cannot abort via std::length_error; add a last-resort exception handler in main - emit null for non-finite FLOAT/DOUBLE cells in JSON output (bare nan/inf is not valid JSON) - docs: -m also applies to stats/count, count covers all tables, note that the table format buffers rows in memory
1 parent 5123d6e commit 0963453

9 files changed

Lines changed: 108 additions & 17 deletions

File tree

cpp/tools/CMakeLists.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ target_include_directories(tsfile_cli_obj PUBLIC
3030
${LIBRARY_INCLUDE_DIR}
3131
${THIRD_PARTY_INCLUDE})
3232

33+
# Library headers are compiled from the staged include tree
34+
# (LIBRARY_INCLUDE_DIR), which is populated by the copy_* targets that
35+
# tsfile depends on transitively. Without this edge a clean parallel build
36+
# can compile these sources before the headers exist.
37+
add_dependencies(tsfile_cli_obj tsfile)
38+
3339
if (ENABLE_ANTLR4)
3440
target_include_directories(tsfile_cli_obj PUBLIC
3541
${PROJECT_SOURCE_DIR}/third_party/antlr4-cpp-runtime-4/runtime/src)

cpp/tools/README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ Exit codes: `0` success, `1` usage/argument error, `2` file open/corrupt,
101101
| `stats` | Per-series `count, start_time, end_time, min, max, first, last, sum` |
102102
| `count` | Per-series row counts plus a `total` row (from statistics, no page scan) |
103103
| `head` | First N rows (default 10; use `-n`) |
104-
| `cat` | All matching rows, streamed |
104+
| `cat` | All matching rows, streamed (`table` format buffers to align columns) |
105105
| `sample` | Reproducible reservoir sample (default 10; `-n`, `--seed`) |
106106

107107
The metadata commands (`ls` / `schema` / `meta` / `stats` / `count`) answer most questions
@@ -113,15 +113,17 @@ Shared options:
113113
|---|---|
114114
| `-f, --format csv\|tsv\|json\|table` | Output format; defaults to `table` on a TTY, `tsv` when piped |
115115
| `-d, --device <id>` / `-t, --table <name>` | Scope to one device / table (mutually exclusive) |
116-
| `-m, --measurements a,b,c` | Column projection (`schema`, `head`, `cat`, `sample`) |
116+
| `-m, --measurements a,b,c` | Column projection (`schema`, `stats`, `count`, `head`, `cat`, `sample`) |
117117
| `-n, --limit N` / `--offset N` | Max rows / rows to skip (`head`, `cat`; `--offset` not valid for `sample`) |
118118
| `--start <ms>` / `--end <ms>` | Inclusive epoch-millisecond time range (`head`, `cat`, `sample`) |
119119
| `--seed N` | Reproducible sampling seed (`sample` only) |
120120
| `--no-header` | Omit the header row |
121121
| `--model tree\|table` | Force the model (otherwise auto-detected) |
122122

123123
`json` output is NDJSON (one object per line; numbers/booleans bare, other values quoted,
124-
nulls as `null`). CSV output follows RFC 4180. Timestamps are raw epoch milliseconds.
124+
nulls as `null`; non-finite floats — NaN/Inf — become `null`). CSV output follows RFC 4180.
125+
Timestamps are raw epoch milliseconds. The `table` format buffers all rows in memory to
126+
align columns, so prefer `csv`/`tsv`/`json` when dumping large files.
125127

126128
```bash
127129
BIN=cpp/build/Debug/bin/tsfile-cli

cpp/tools/cli/cli_args.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "cli/cli_args.h"
2121

22+
#include <cerrno>
2223
#include <cstdlib>
2324
#include <sstream>
2425

@@ -42,8 +43,9 @@ bool parse_ll(const std::string& s, long long& out) {
4243
return false;
4344
}
4445
char* endp = nullptr;
46+
errno = 0;
4547
long long v = std::strtoll(s.c_str(), &endp, 10);
46-
if (endp == nullptr || *endp != '\0') {
48+
if (endp == nullptr || *endp != '\0' || errno == ERANGE) {
4749
return false;
4850
}
4951
out = v;

cpp/tools/cli/run_cli.cc

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,34 @@ bool validate_write_flags(const ParsedArgs& p, std::ostream& err) {
141141
err << "Error: --header-match cannot be combined with --no-header\n";
142142
return false;
143143
}
144-
if (!p.measurements.empty() || !p.device.empty() || p.has_start ||
145-
p.has_end || p.has_seed || p.limit != -1 || p.offset != 0) {
146-
err << "Error: read-only flags are not valid for write\n";
144+
// Name the offending flag so the user does not have to guess which of
145+
// the read-only options triggered the rejection.
146+
if (!p.measurements.empty()) {
147+
err << "Error: -m/--measurements is not valid for write\n";
148+
return false;
149+
}
150+
if (!p.device.empty()) {
151+
err << "Error: -d/--device is not valid for write\n";
152+
return false;
153+
}
154+
if (p.has_start || p.has_end) {
155+
err << "Error: --start/--end are not valid for write\n";
156+
return false;
157+
}
158+
if (p.has_seed) {
159+
err << "Error: --seed is not valid for write\n";
160+
return false;
161+
}
162+
if (p.limit != -1) {
163+
err << "Error: -n/--limit is not valid for write\n";
164+
return false;
165+
}
166+
if (p.offset != 0) {
167+
err << "Error: --offset is not valid for write\n";
168+
return false;
169+
}
170+
if (!p.model.empty()) {
171+
err << "Error: --model is not valid for write\n";
147172
return false;
148173
}
149174
return true;
@@ -178,6 +203,10 @@ bool validate_read_flag_applicability(const ParsedArgs& p, std::ostream& err) {
178203
err << "Error: -n/--limit is only valid for head/cat/sample\n";
179204
return false;
180205
}
206+
if (!is_row && p.offset != 0) {
207+
err << "Error: --offset is only valid for head/cat\n";
208+
return false;
209+
}
181210
if (!is_row && (p.has_start || p.has_end)) {
182211
err << "Error: --start/--end are only valid for head/cat/sample\n";
183212
return false;
@@ -253,10 +282,30 @@ int run_cli(const std::vector<std::string>& args, std::ostream& out,
253282
storage::TsFileReader reader;
254283
int open_ret = reader.open(p.file);
255284
if (open_ret != 0) {
256-
err << "Error: cannot open or corrupted file: " << p.file << "\n";
285+
err << "Error: cannot open " << p.file << ": "
286+
<< error_code_message(open_ret) << " (code " << open_ret << ")\n";
257287
return kExitFile;
258288
}
259289

290+
// head/cat/sample/schema dispatch on the data model and would silently
291+
// ignore the scope flag of the other model; reject that instead.
292+
if (p.command == "head" || p.command == "cat" || p.command == "sample" ||
293+
p.command == "schema") {
294+
const bool table_model = is_table_model(p, reader);
295+
if (table_model && !p.device.empty()) {
296+
err << "Error: -d/--device does not apply to the table model; "
297+
"use -t/--table (or force --model tree)\n";
298+
reader.close();
299+
return kExitUsage;
300+
}
301+
if (!table_model && !p.table.empty()) {
302+
err << "Error: -t/--table does not apply to the tree model; "
303+
"use -d/--device (or force --model table)\n";
304+
reader.close();
305+
return kExitUsage;
306+
}
307+
}
308+
260309
bool stdout_tty = TSFILE_ISATTY(TSFILE_FILENO(stdout)) != 0;
261310
OutputFormat fmt = resolve_format(p.format, stdout_tty);
262311

cpp/tools/commands/cmd_write.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,10 @@ int cmd_write(const ParsedArgs& args, std::ostream& /*out*/,
281281
#ifdef _WIN32
282282
flags |= O_BINARY;
283283
#endif
284-
if (file.create(args.output, flags, 0666) != 0) {
285-
err << "Error: cannot create output: " << args.output << "\n";
284+
int cret = file.create(args.output, flags, 0666);
285+
if (cret != 0) {
286+
err << "Error: cannot create output " << args.output << ": "
287+
<< error_code_message(cret) << " (code " << cret << ")\n";
286288
return kExitFile;
287289
}
288290
auto* schema = new storage::TableSchema(args.table, col_schemas);

cpp/tools/format/output_format.cc

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ std::string json_escape(const std::string& s) {
222222
return out;
223223
}
224224

225+
namespace {
226+
227+
// FLOAT/DOUBLE cells render non-finite values as nan/inf tokens, which have
228+
// no JSON representation; finite numbers never contain these letters.
229+
bool json_nonfinite(common::TSDataType t, const std::string& cell) {
230+
if (t != common::FLOAT && t != common::DOUBLE) {
231+
return false;
232+
}
233+
return cell.find_first_of("nNiI") != std::string::npos;
234+
}
235+
236+
} // namespace
237+
225238
RowWriter::RowWriter(std::ostream& out, OutputFormat fmt,
226239
std::vector<std::string> header,
227240
std::vector<common::TSDataType> types, bool no_header)
@@ -287,7 +300,11 @@ void RowWriter::write(const std::vector<std::string>& cells,
287300
if (i < is_null.size() && is_null[i]) {
288301
out_ << "null";
289302
} else if (emits_json_bare(i)) {
290-
out_ << (i < cells.size() ? cells[i] : "null");
303+
if (i >= cells.size() || json_nonfinite(types_[i], cells[i])) {
304+
out_ << "null"; // NaN/Inf: match JSON serializer practice
305+
} else {
306+
out_ << cells[i];
307+
}
291308
} else {
292309
out_ << "\"" << json_escape(i < cells.size() ? cells[i] : "")
293310
<< "\"";

cpp/tools/format/result_set_format.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "format/result_set_format.h"
2121

22+
#include <algorithm>
2223
#include <cstdio>
2324
#include <ctime>
2425
#include <iomanip>
@@ -153,7 +154,10 @@ int emit_result_set_sampled(storage::ResultSet* rs, OutputFormat fmt,
153154
}
154155

155156
std::vector<BufferedRow> reservoir;
156-
reservoir.reserve(static_cast<size_t>(limit));
157+
// Cap the pre-allocation: limit is user input and a huge -n would make
158+
// reserve() throw std::length_error before any row is read. The vector
159+
// still grows up to `limit` as rows actually arrive.
160+
reservoir.reserve(static_cast<size_t>(std::min<long long>(limit, 4096)));
157161
std::mt19937_64 rng(seed);
158162
bool has_next = false;
159163
int code = common::E_OK;

cpp/tools/skills/tsfile-cli/SKILL.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,21 @@ Single pipe-friendly C++ binary to inspect **and** import `.tsfile` (TsFile's an
4747
| `stats` | per-series `count,start,end,min,max,first,last,sum` | no |
4848
| `count` | per-series counts + `total` row | no |
4949
| `head` | first N rows (default 10, `-n`) | yes |
50-
| `cat` | all matching rows (streamed) | yes |
50+
| `cat` | all matching rows (streamed; `table` format buffers) | yes |
5151
| `sample` | reservoir sample (default 10, `-n` + `--seed`) | yes |
5252

5353
Prefer no-scan verbs (`ls/schema/meta/stats/count`) — cheap and never hit the page-decode caveat.
5454

55-
Table model + row verbs (`head/cat/sample/count`): without `-t`, only the **first** table is queried. Pass `-t <table>` to target a specific one.
55+
Table model + row verbs (`head/cat/sample`): without `-t`, only the **first** table is queried. Pass `-t <table>` to target a specific one (`count` covers all tables).
5656

5757
```
5858
opts: -f csv|tsv|json|table (default TTY→table, pipe→tsv)
5959
-d <device> | -t <table> (mutually exclusive)
6060
-m a,b,c (projection) · -n N · --offset N · --start <ms> · --end <ms> (inclusive)
6161
--seed N · --no-header · --model tree|table (else auto)
62-
applies: -m → schema/head/cat/sample · -d/-t → row cmds/schema/stats/count · --offset ∉ sample
63-
json=NDJSON (num/bool bare, else quoted, null→null) · csv=RFC4180 · ts=raw epoch ms
62+
applies: -m → schema/stats/count/head/cat/sample · -d/-t → row cmds/schema/stats/count
63+
(-d needs tree model, -t needs table model in head/cat/sample/schema) · --offset ∉ sample
64+
json=NDJSON (num/bool bare, else quoted, null→null, NaN/Inf→null) · csv=RFC4180 · ts=raw epoch ms
6465
exit: 0 ok · 1 usage · 2 file open/corrupt · 3 query/runtime
6566
```
6667

cpp/tools/tools_main.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,21 @@
1717
* under the License.
1818
*/
1919

20+
#include <exception>
2021
#include <iostream>
2122
#include <string>
2223
#include <vector>
2324

25+
#include "cli/exit_codes.h"
2426
#include "cli/run_cli.h"
2527

2628
int main(int argc, char** argv) {
2729
std::vector<std::string> args(argv + 1, argv + argc);
28-
return tsfile_cli::run_cli(args, std::cout, std::cerr);
30+
try {
31+
return tsfile_cli::run_cli(args, std::cout, std::cerr);
32+
} catch (const std::exception& e) {
33+
// Last-resort net (e.g. std::bad_alloc): report instead of aborting.
34+
std::cerr << "Error: " << e.what() << "\n";
35+
return tsfile_cli::kExitRuntime;
36+
}
2937
}

0 commit comments

Comments
 (0)