Pitch.TryParse("C1073741823") succeeds as C-1: octave arithmetic wraps, and "C 4 " is accepted too

[matt-edmondson]

What's wrong

Pitch computes the MIDI number as ((octave + 1) * 12) + letter + accidental in unchecked int, so a huge octave wraps around into the 0..127 range and passes the range check.

Where

• Semantics.Music/Pitch.cs:81-86 (TryParse):

  if (!int.TryParse(name[index..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int octave)) return false;
  int midi = ((octave + 1) * 12) + (int)letter + accidental;
  if (midi is < 0 or > 127) return false;

• Semantics.Music/Pitch.cs:46 (Create(NoteLetter, Accidental, int octave)) uses the same expression.

Failure scenarios

These were reproduced against the built library on .NET 10:

• (1073741823 + 1) * 12 wraps to 0, so:
  • Pitch.TryParse("C1073741823", out var p) returns true with p = C-1 (MIDI 0).
  • Pitch.Create(NoteLetter.C, Accidental.Natural, 1073741823) returns C-1 instead of throwing the documented ArgumentOutOfRangeException.
• NumberStyles.Integer allows leading and trailing whitespace and a leading sign, so "C 4 " and "C+4" parse. They then print back as "C4", which breaks the round-trip/canonical-text contract that the type-safe-factories work set up for Music types.

Suggested fix

• Range-check the octave before the multiplication. Any octave outside roughly −2..10 cannot produce MIDI 0..127 even with accidentals, so reject it early. Alternatively, compute in long.
• Parse the octave with NumberStyles.AllowLeadingSign only.
• Add tests for "C1073741823", "C 4", "C+4", and the typed factory with int.MaxValue.

[matt-edmondson]

Triage

• Category: Bug
• Priority: Low. Absurd octaves wrap into range, and whitespace or + is accepted in the octave. Both break the canonical-text contract, but only on malformed input.
• Area / suggested assignee: Semantics.Music/Pitch.cs (TryParse and Create). @matt-edmondson.
• Duplicates: None. This is the same Music-parser input-validation cluster as Tempo.TryParse("NaNbpm@1/4") throws instead of returning false, and zero/negative/infinite beats are accepted, so Seconds() returns Infinity or negative time #305 (Tempo.TryParse) and Duration.Add/Multiply/Dotted overflow silently, so Progression.TotalDuration of ten tuplet durations comes out negative #306 (Duration overflow). One PR that hardens Pitch, Tempo and Duration parsing and arithmetic together would be efficient.
• In progress: No open PR covers it.
• Notes: Range-check the octave early and parse with NumberStyles.AllowLeadingSign.

---

Generated by Claude Code