craft-midi: say when a lenient read left part of a file out at a limit (#1308)

read_smf_lenient keeps what comes before MAX_EVENTS events or MAX_TRACKS
tracks and only says so in a warning. Those warnings are words, and at
most MAX_WARNINGS of them are kept: when earlier damage has filled them,
nothing tells the caller that the file was cut.

LenientRead gets over_limit, set whenever either limit left something
out, full warnings or not. A caller that refuses over-limit files, as
read_smf does, can now do so after a lenient read. LenientRead is
non_exhaustive, so the new field breaks no caller.

crates/midi/src/smf.rs

@@ -189,13 +189,14 @@ pub const MAX_WARNINGS: usize = 32;
 ///   short is not believed when the track's End of Track says otherwise: the next chunk is looked
 ///   for where the track's events ended;
 /// - bytes that are no chunk are skipped up to the next `MTrk`, and trailing bytes are ignored;
-/// - past [`MAX_TRACKS`] tracks the rest is left out;
+/// - past [`MAX_TRACKS`] tracks the rest is left out; this and the event limit set
+///   [`LenientRead::over_limit`];
 /// - lyrics are counted in a warning (they are not kept); a format-2 file gets a warning that its
 ///   tracks are read side by side.
 pub fn read_smf_lenient(bytes: &[u8]) -> Result<LenientRead, MidiError> {
     let mut warnings = Vec::new();
     let (smf, extra) = read(bytes, Some(&mut warnings))?;
-    Ok(LenientRead { smf, warnings, instruments: extra.instruments, end: extra.end })
+    Ok(LenientRead { smf, warnings, instruments: extra.instruments, end: extra.end, over_limit: extra.over_limit })
 }
 
 /// What [`read_smf_lenient`] makes of a file.
@@ -209,12 +210,18 @@ pub struct LenientRead {
     pub instruments: Vec<String>,
     /// The last tick any track reaches: its End of Track, or where its events stop.
     pub end: i64,
+    /// Whether the file goes past [`MAX_EVENTS`] events or [`MAX_TRACKS`] tracks, so what lies past
+    /// the limit was left out. It is set even when the warnings are already full, so a caller that
+    /// refuses such files (as [`read_smf`] does) can tell without reading the warnings.
+    pub over_limit: bool,
 }
 
-/// What the reader finds beyond [`Smf`]: instrument names per kept track, and the end tick.
+/// What the reader finds beyond [`Smf`]: instrument names per kept track, the end tick, and
+/// whether a limit left part of the file out.
 struct Extra {
     instruments: Vec<String>,
     end: i64,
+    over_limit: bool,
 }
 
 fn warn(warnings: &mut Option<&mut Vec<String>>, what: String) {
@@ -262,6 +269,7 @@ fn read(bytes: &[u8], mut warnings: Option<&mut Vec<String>>) -> Result<(Smf, Ex
     let (mut tracks, mut instruments) = (Vec::new(), Vec::new());
     let mut event_budget = MAX_EVENTS;
     let (mut end, mut seen, mut lyrics) = (0i64, 0usize, 0usize);
+    let mut over_limit = false;
 
     while !cur.is_empty() {
         let chunk_at = cur.at();
@@ -306,6 +314,7 @@ fn read(bytes: &[u8], mut warnings: Option<&mut Vec<String>>) -> Result<(Smf, Ex
                 return Err(MidiError::TooManyTracks(MAX_TRACKS));
             }
             warn(&mut warnings, format!("more than {MAX_TRACKS} tracks: the rest was left out"));
+            over_limit = true;
             break;
         }
         let (body_at, body_start) = (cur.at(), cur.pos);
@@ -319,7 +328,10 @@ fn read(bytes: &[u8], mut warnings: Option<&mut Vec<String>>) -> Result<(Smf, Ex
         end = end.max(read.end);
         match read.stop {
             Some(e) if !lenient => return Err(e),
-            Some(e) => warn(&mut warnings, format!("track {seen} {}: what came before is kept", stop_reason(&e))),
+            Some(e) => {
+                over_limit |= matches!(e, MidiError::TooManyEvents(_));
+                warn(&mut warnings, format!("track {seen} {}: what came before is kept", stop_reason(&e)));
+            }
             None if !read.ended && lenient => warn(&mut warnings, format!("track {seen} has no End of Track: what came before is kept")),
             None => {}
         }
@@ -363,7 +375,7 @@ fn read(bytes: &[u8], mut warnings: Option<&mut Vec<String>>) -> Result<(Smf, Ex
 
     Ok((
         Smf { format, tracks, tempos: globals.tempos, meters: globals.meters, markers: globals.markers, key_sigs: globals.key_sigs },
-        Extra { instruments, end },
+        Extra { instruments, end, over_limit },
     ))
 }
 

crates/midi/tests/it/lenient.rs

@@ -33,8 +33,8 @@ const TWO: &[u8] = &[0x00, 0x91, 62, 100, 0x60, 0x81, 62, 0];
 #[test]
 fn a_sound_file_reads_the_same_either_way_without_warnings() {
     let good = file(1, 96, &[ONE, TWO]);
-    let LenientRead { smf, warnings, end, .. } = read_smf_lenient(&good).unwrap();
-    assert_eq!((keys(&smf), warnings.len()), (vec![vec![60], vec![62]], 0));
+    let LenientRead { smf, warnings, end, over_limit, .. } = read_smf_lenient(&good).unwrap();
+    assert_eq!((keys(&smf), warnings.len(), over_limit), (vec![vec![60], vec![62]], 0, false));
     assert_eq!(smf, read_smf(&good).unwrap());
     assert_eq!(end, 960, "the End of Track tick, at 960 ticks a quarter");
 }
@@ -122,15 +122,34 @@ fn a_file_of_any_size_is_read_into_a_bounded_number_of_events_and_warnings() {
     // Three bytes a note-on under running status: more events than are read.
     let mut body = vec![0x00, 0x90, 60, 100];
     body.extend((0..MAX_EVENTS + 1000).flat_map(|i| [0x00, (i % 128) as u8, 1]));
-    let LenientRead { smf, warnings, .. } = read_smf_lenient(&file(1, 96, &[&body])).unwrap();
+    let LenientRead { smf, warnings, over_limit, .. } = read_smf_lenient(&file(1, 96, &[&body])).unwrap();
     assert_eq!(smf.tracks[0].sequence.notes.len(), MAX_EVENTS);
     assert!(warnings.iter().any(|w| w.contains("more events")), "{warnings:?}");
+    assert!(over_limit, "the event limit left events out");
     // Tracks of nothing, by the thousand: the count stops the reading.
-    let LenientRead { smf, warnings, .. } = read_smf_lenient(&file(1, 96, &vec![&[][..]; MAX_TRACKS + 5])).unwrap();
+    let LenientRead { smf, warnings, over_limit, .. } = read_smf_lenient(&file(1, 96, &vec![&[][..]; MAX_TRACKS + 5])).unwrap();
     assert!(smf.tracks.len() < MAX_TRACKS && warnings.iter().any(|w| w.contains("tracks")));
-    // Every track damaged: never more than MAX_WARNINGS words.
-    let LenientRead { warnings, .. } = read_smf_lenient(&file(1, 96, &vec![&[0x00, 60][..]; 100])).unwrap();
+    assert!(over_limit, "the track limit left tracks out");
+    // Every track damaged: never more than MAX_WARNINGS words, and no limit was reached.
+    let LenientRead { warnings, over_limit, .. } = read_smf_lenient(&file(1, 96, &vec![&[0x00, 60][..]; 100])).unwrap();
     assert_eq!(warnings.len(), MAX_WARNINGS);
+    assert!(!over_limit);
+}
+
+#[test]
+fn a_limit_reached_after_the_warnings_are_full_is_still_reported() {
+    // MAX_WARNINGS damaged tracks fill the warnings; the track after them holds more events than are
+    // read, so its warning is dropped, and over_limit is what says the file was cut.
+    let mut body = vec![0x00, 0x90, 60, 100];
+    body.extend((0..MAX_EVENTS + 1000).flat_map(|i| [0x00, (i % 128) as u8, 1]));
+    let mut tracks = vec![&[0x00, 60][..]; MAX_WARNINGS];
+    tracks.push(&body);
+    let LenientRead { warnings, over_limit, .. } = read_smf_lenient(&file(1, 96, &tracks)).unwrap();
+    assert_eq!(warnings.len(), MAX_WARNINGS);
+    assert!(!warnings.iter().any(|w| w.contains("more events")), "the test needs the limit's warning dropped: {warnings:?}");
+    assert!(over_limit);
+    // The strict reader stops at the first damaged track, before the limit.
+    assert!(!matches!(read_smf(&file(1, 96, &tracks)), Err(MidiError::TooManyEvents(_)) | Ok(_)));
 }
 
 #[test]