diff --git a/docs/releases/unreleased.md b/docs/releases/unreleased.md index b75bbe1..fa884a3 100644 --- a/docs/releases/unreleased.md +++ b/docs/releases/unreleased.md @@ -5,3 +5,7 @@ - All successfully read license states exit 0. Configuration, request, decoding, and output failures exit nonzero. No token or database header is sent to `/health`, and no authentication verification is performed. Normal connection selection, timeout, TLS verification, and identity opt-out settings apply. License installation and key/file configuration remain server responsibilities. Existing `ping -o json` license pass-through is unchanged. Closes [arcli#34](https://github.com/Basekick-Labs/arcli/issues/34). + +### Fixed + +- Reject reserved `time` field and tag names, including letter-case variants, before sending JSON writes. Timestamp columns must use lowercase `time`; row timestamps continue to use `t`. This prevents invalid names from causing a partially applied batch on the server. diff --git a/internal/client/msgpack_reserved_names_test.go b/internal/client/msgpack_reserved_names_test.go new file mode 100644 index 0000000..54a42ea --- /dev/null +++ b/internal/client/msgpack_reserved_names_test.go @@ -0,0 +1,54 @@ +package client + +import ( + "fmt" + "strings" + "testing" +) + +func TestJSONToMsgPackRejectsReservedTimeNames(t *testing.T) { + for _, name := range []string{"time", "Time", "TIME", "tImE"} { + for _, kind := range []string{"fields", "tags", "columns"} { + if kind == "columns" && name == "time" { + continue // the timestamp column itself is valid + } + t.Run(kind+"/"+name, func(t *testing.T) { + var row string + switch kind { + case "fields": + row = fmt.Sprintf(`{"m":"cpu","fields":{%q:1}}`, name) + case "tags": + row = fmt.Sprintf(`{"m":"cpu","fields":{"value":1},"tags":{%q:"label"}}`, name) + case "columns": + row = fmt.Sprintf(`{"m":"cpu","columns":{%q:[1700000000],"value":[1]}}`, name) + } + for _, doc := range []string{ + row, + fmt.Sprintf(`[{"m":"cpu","fields":{"value":1}},%s]`, row), + fmt.Sprintf(`{"batch":[{"m":"cpu","fields":{"value":1}},%s]}`, row), + } { + out, _, err := JSONToMsgPack(strings.NewReader(doc)) + if err == nil || !strings.Contains(err.Error(), "reserved") { + t.Errorf("%s: error = %v; want a reserved timestamp name error", doc, err) + } + if len(out) != 0 { + t.Error("invalid document produced MessagePack bytes") + } + } + }) + } + } +} + +func TestJSONToMsgPackAcceptsTimestampAndOtherNames(t *testing.T) { + for _, doc := range []string{ + `{"m":"cpu","t":1700000000,"fields":{"runtime":1,"time_value":2},"tags":{"timezone":"UTC"}}`, + `{"m":"cpu","columns":{"time":[1700000000],"value":[1]}}`, + `{"m":"cpu","fields":{"TİME":1,"time":2},"tags":{"TİME":"a","time":"b"}}`, + `{"m":"cpu","columns":{"TİME":[1],"time":[2]}}`, + } { + if _, _, err := JSONToMsgPack(strings.NewReader(doc)); err != nil { + t.Errorf("valid names rejected: %v", err) + } + } +} diff --git a/internal/client/msgpackenc.go b/internal/client/msgpackenc.go index 7595fd3..65b9262 100644 --- a/internal/client/msgpackenc.go +++ b/internal/client/msgpackenc.go @@ -250,6 +250,9 @@ func convertItem(m map[string]any, where string, sum *MsgPackDocSummary) (any, e outCols := make(map[string]any, len(cm)) n := -1 for col, v := range cm { + if col != "time" && strings.EqualFold(col, "time") { + return nil, fmt.Errorf(`%s: column name %q is reserved for the timestamp; use "time"`, where, col) + } arr, ok := v.([]any) if !ok { return nil, fmt.Errorf("%s: column %q must be an array (the server would drop it)", where, col) @@ -284,6 +287,9 @@ func convertItem(m map[string]any, where string, sum *MsgPackDocSummary) (any, e } outFields := make(map[string]any, len(fields)) for k, v := range fields { + if strings.EqualFold(k, "time") { + return nil, fmt.Errorf(`%s: field name %q is reserved for the timestamp; use "t" for row timestamps`, where, k) + } cv, err := convertScalar(v, fmt.Sprintf("%s: field %q", where, k), false) if err != nil { return nil, err @@ -315,6 +321,9 @@ func convertItem(m map[string]any, where string, sum *MsgPackDocSummary) (any, e } outTags := make(map[string]any, len(tm)) for k, v := range tm { + if strings.EqualFold(k, "time") { + return nil, fmt.Errorf(`%s: tag name %q is reserved for the timestamp`, where, k) + } switch tv := v.(type) { case string: outTags[k] = tv diff --git a/internal/commands/write_reserved_names_test.go b/internal/commands/write_reserved_names_test.go new file mode 100644 index 0000000..5c928fa --- /dev/null +++ b/internal/commands/write_reserved_names_test.go @@ -0,0 +1,40 @@ +package commands + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +func TestWriteJSONRejectsReservedNamesBeforeRequest(t *testing.T) { + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + _, _ = io.Copy(io.Discard, r.Body) + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"error":"partial write: 1 points dropped; the other points were written"}`) + })) + defer srv.Close() + writeTestConfig(t, srv.URL, "tok") + for _, bad := range []string{ + `{"m":"cpu","fields":{"time":1}}`, + `{"m":"cpu","fields":{"value":1},"tags":{"TIME":"label"}}`, + `{"m":"cpu","columns":{"Time":[1700000000],"value":[1]}}`, + } { + cmd := newWriteCmd() + cmd.SetIn(strings.NewReader(`{"batch":[{"m":"cpu","fields":{"value":1}},` + bad + `]}`)) + out, _, err := execCmd(t, cmd, "--format", "json") + if err == nil || !strings.Contains(err.Error(), "reserved") { + t.Errorf("error = %v; want local reserved-name validation", err) + } + if out != "" { + t.Errorf("invalid document printed success: %q", out) + } + } + if got := requests.Load(); got != 0 { + t.Errorf("invalid batches reached the write endpoint %d times", got) + } +}