Skip to content

Duration accepts zero and negative lengths, so Note, Rest, ChordEvent and Progression.Parse accept "C4:-1/4:v64", "R:0/4", "C@-1/4" #353

Closed

Description

@matt-edmondson

What's wrong

  • Duration.Create (Semantics.Music/Duration.cs:42-55) rejects only a zero denominator.
  • Duration.TryParse explicitly allows a leading sign (NumberStyles.AllowLeadingSign, :104-105).
  • The following all accept whatever Duration holds:
    • Note.Create (Note.cs:27)
    • Rest.Create (Rest.cs:19)
    • ChordEvent.Create (ChordEvent.cs:23)
    • their TryParse counterparts
    • the Chord@n/d cell in Progression.Parse (Progression.Parse.cs:149)

So musical events with zero or negative length get through at every entry point.

Reproduction (net10.0)

Call Result
Progression.Parse("4/4 C@-1/4 G / / /") parses; TotalDuration = 3/4 and TotalBars = 0.75 for a chart holding a full bar of G
Progression.Parse("4/4 C@0/1 G / / /") parses with a zero-length C
Note.Parse("C4:-1/4:v64") succeeds
Rest.Parse("R:0/4") succeeds (R:0/1)
ChordEvent.Parse("Am:-3/4") succeeds

Note.Seconds(tempo) and Rest.Seconds(tempo) (Note.cs:39-42, Rest.cs:29-32) then return negative or zero time.

Why it matters

Negative events silently subtract from Progression.TotalDuration and TotalBars, and from anything built on them, such as the bar logic in Form and Section. Related: #305 fixes the same hole, but only for Tempo's beat unit.

Suggested fix / acceptance criteria

  • Pick one of these:
    • Make Duration strictly positive: reject numerator <= 0 in Create, and return false from TryParse.
    • Keep Duration as a general signed rational, and enforce > 0 in Note.Create, Rest.Create, ChordEvent.Create and Progression.Parse.
  • All five inputs above are rejected: TryParse returns false and Create throws ArgumentOutOfRangeException.
  • The zero Duration that seeds the TotalDuration aggregate still works, for example as an internal zero.

Activity

  1. matt-edmondson commented on Sep 30, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

  2. matt-edmondson commented on Oct 29, 2026

    @matt-edmondson
    ContributorAuthor

    Fixed by #487 (commit 47dffbb, now on main). The commit cites this issue inline as "(#353)" rather than with a "Fixes" keyword, so GitHub didn't close it. Closing by hand.


    Generated by Claude Code

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions