Skip to content

parseDateValue silently accepts negative and non-integer N in Nd/Nw/Nm/Ny #41

Description

@DTTerastar

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 as 1d. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:dates--since / --until parsing and windowing.kind:bugObserved behavior diverges from documented behavior.priority:mediumReproducible bug or gap with a workaround, or a well-scoped enhancement clearly in charter.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions