amadan
schedule: control.go — why-comment for TimerStore, reject trailing PUT JSON, pin tz-offset at
TimerStore's comment said what it was, not why it's a narrow
interface (activator stubs it for wiring tests; *registry.Registry
satisfies it directly). Rewrite it.
The PUT /schedule/{name} body decoder stopped at the first JSON value
and silently ignored anything appended after it. Check dec.More()
after Decode and refuse a body carrying trailing data, with a test
covering a second concatenated object, trailing prose, and a stray
comma.
Add a test pinning that a tz-offset `at` (e.g. +01:00) is accepted
and the stored row's next_at/due_at land on the UTC instant, not the
offset clock-face numbers — already correct, just untested.
2 files changed,
+57
−2
internal/schedule/control.go+15 −2internal/schedule/control_test.go+42 −0
diff --git a/internal/schedule/control.go b/internal/schedule/control.go| index 2828e5fcf..6a0b7cf0a 100644 |
| --- a/internal/schedule/control.go |
| +++ b/internal/schedule/control.go |
| @@ -12,7 +12,11 @@ import ( |
| "github.com/carlosframework/platform/internal/registry" |
| ) |
| -// TimerStore is the registry subset the control channel needs. |
| +// TimerStore is narrowed to the five methods Mount actually calls, not the |
| +// whole *registry.Registry: *registry.Registry satisfies it directly |
| +// (internal/host/agent.go), and internal/activator — the other side of the |
| +// package boundary this crosses (Backend.Timers) — stubs it with an |
| +// in-memory fake so its wiring tests never touch a real database. |
| type TimerStore interface { |
| UpsertTimer(registry.Timer) error |
| DeleteTimer(host, name string) error |
| @@ -62,10 +66,19 @@ func Mount(mux *http.ServeMux, host string, store TimerStore, token func() (stri |
| At string `json:"at"` |
| Path string `json:"path"` |
| } |
| - if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)).Decode(&body); err != nil { |
| + dec := json.NewDecoder(http.MaxBytesReader(w, r.Body, 4096)) |
| + if err := dec.Decode(&body); err != nil { |
| http.Error(w, "bad request: "+err.Error(), http.StatusBadRequest) |
| return |
| } |
| + // Decode stops at the first JSON value and silently ignores |
| + // whatever follows it; a client that appended a second object (or |
| + // stray bytes) meant something by that, so refuse rather than act |
| + // on the first value alone. |
| + if dec.More() { |
| + http.Error(w, "bad request: trailing data after the JSON body", http.StatusBadRequest) |
| + return |
| + } |
| atT, err := time.Parse(time.RFC3339, body.At) |
| if err != nil { |
| http.Error(w, "at: want RFC3339, e.g. 2026-09-01T08:00:00Z", http.StatusBadRequest) |
diff --git a/internal/schedule/control_test.go b/internal/schedule/control_test.go| index 13bd412b5..1c97afd3b 100644 |
| --- a/internal/schedule/control_test.go |
| +++ b/internal/schedule/control_test.go |
| @@ -110,6 +110,48 @@ func TestControlRefusals(t *testing.T) { |
| } |
| } |
| +// json.Decoder.Decode stops at the first value and silently drops whatever |
| +// follows it, so a naive Decode(&body) would accept a body like |
| +// `{"at":...}{"at":...}` or `{"at":...}garbage` and act on the first |
| +// object alone. The PUT handler must refuse it instead. |
| +func TestControlPutRejectsTrailingJSON(t *testing.T) { |
| + _, mux := controlFixture(t) |
| + for _, body := range []string{ |
| + `{"at":"2026-09-01T08:00:00Z","path":"/p"}{"at":"2026-09-02T08:00:00Z","path":"/p"}`, |
| + `{"at":"2026-09-01T08:00:00Z","path":"/p"} garbage`, |
| + `{"at":"2026-09-01T08:00:00Z","path":"/p"},`, |
| + } { |
| + if rec := do(mux, "PUT", "/schedule/trailing", "Bearer tok", body); rec.Code != http.StatusBadRequest { |
| + t.Errorf("PUT with trailing data %q → %d, want 400 (%s)", body, rec.Code, rec.Body) |
| + } |
| + } |
| + // A clean single object still works. |
| + if rec := do(mux, "PUT", "/schedule/clean", "Bearer tok", `{"at":"2026-09-01T08:00:00Z","path":"/p"}`); rec.Code != http.StatusOK { |
| + t.Fatalf("clean body → %d (%s)", rec.Code, rec.Body) |
| + } |
| +} |
| + |
| +// A member's clock is not UTC; `at` must accept an offset and the row must |
| +// store the UTC instant it names, not the offset's clock-face numbers. |
| +func TestControlPutNormalisesTZOffsetToUTC(t *testing.T) { |
| + reg, mux := controlFixture(t) |
| + // 09:00 +01:00 is 08:00Z. |
| + rec := do(mux, "PUT", "/schedule/tz", "Bearer tok", `{"at":"2026-09-01T09:00:00+01:00","path":"/p"}`) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("put = %d %s", rec.Code, rec.Body) |
| + } |
| + want := at("2026-09-01T08:00:00Z").Unix() |
| + var ts TimerStatus |
| + json.Unmarshal(rec.Body.Bytes(), &ts) |
| + if ts.NextAt != want || ts.Spec != "at "+itoa(want) { |
| + t.Fatalf("response = %+v, want next_at=%d", ts, want) |
| + } |
| + tm, err := reg.GetTimer("a.bab.oncarlos.com", "tz") |
| + if err != nil || tm.NextAt != want || tm.DueAt != want { |
| + t.Fatalf("row = %+v, err=%v, want next_at/due_at=%d", tm, err, want) |
| + } |
| +} |
| + |
| func TestControlDeleteRefusesDeclaredSchedule(t *testing.T) { |
| reg, mux := controlFixture(t) |
| reg.UpsertTimer(registry.Timer{Host: "a.bab.oncarlos.com", Name: "sync", Kind: KindSchedule, Spec: "every 6h0m0s", Path: "/s", NextAt: 1}) |