Repository navigation
parseDateValue silently accepts negative and non-integer N in Nd/Nw/Nm/Ny #41
Description
QA finding: internal/cronoclient/daterange.go::parseDateValue parses the numeric prefix with fmt.Sscanf(s[:len(s)-1], "%d", &n), which accepts:
-
Negative numbers:
--since -5d→ n = -5 → today.AddDate(0,0,-(-5)) → 5 days in the future. The resulting range fails the inverted-range check with a misleading error rather than rejecting the bad input up front:$ crono-export nutrition --since -5d error: --until (2026-05-18) is before --since (2026-05-23) -
Decimal numbers:
--since 1.5d→ Sscanf parses up to the., returns n = 1 with no error. Silently treated as1d. No warning, no rejection.
Both cases should fail at parseDateValue with a clear message like "N in Nd must be a non-negative integer". The contract documents Nd/Nw/Nm/Ny with N implicitly a non-negative integer — current code is lax about both.
Suggested fix: validate prefix matches ^\d+$ before parsing, or after Sscanf check that (a) the number is >= 0 and (b) the consumed length equals len(s)-1. Add unit tests for -5d, 1.5d, 5.0d, +5d, 5d, 5 d.
Severity: minor (no data corruption, but "works" with surprising semantics)
Activity
Terastar-Paperclip commentedon May 19, 2026
Triage:
kind:bug·area:dates·priority:medium^\d+$prefix check + tests for negative/decimal/whitespace cases) is in the body.On the contract reference: CONTRACT.md §3 "Date flags" shows the relative form with positive-integer examples (
7d,4w,6m,1y) but does not explicitly state that N must be a non-negative integer. So the parser should be tightened (this issue) and there may be a separate small CONTRACT.md follow-up to make the implicit rule explicit — that second piece is an engineering call, not triage. Engineering ownership.(Correction: an earlier version of this comment cited "§3.3"; CONTRACT.md §3 has no subsections.)