From b66575456021d4555a9928b443ea8a1f4025e1af Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 22 Apr 2026 09:07:55 +0100 Subject: [PATCH] fix(cr-2026-04-22): read_wav surfaces sample decode errors MAJOR from the 2026-04-22 review (wav.rs:135-145): read_wav used filter_map(|s| s.ok()) on both integer and float sample iterators, so any per-sample decode error (truncated payload, corrupted format descriptor after a partial write) was silently discarded. Callers received Ok with a short samples vec, losing audio without any signal to investigate. Switches to collect::>>() with a map that converts hound's per-sample errors into KonError::AudioDecodeFailed. First error aborts the read rather than returning a partial vector. Test fabricates the regression by writing a valid WAV and chopping the last 10 bytes; the previous filter_map would have returned Ok with a shortened vec, the new code returns Err. --- crates/audio/src/wav.rs | 59 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/crates/audio/src/wav.rs b/crates/audio/src/wav.rs index df3caea..1872f69 100644 --- a/crates/audio/src/wav.rs +++ b/crates/audio/src/wav.rs @@ -123,7 +123,13 @@ pub fn write_wav(path: &Path, audio: &AudioSamples) -> Result<()> { Ok(()) } -/// Read a WAV file to f32 PCM AudioSamples. +/// Read a WAV file to f32 PCM `AudioSamples`. +/// +/// Any per-sample decode error is surfaced as `KonError::AudioDecodeFailed` +/// rather than silently dropped. A previous implementation used +/// `filter_map(|s| s.ok())`, so a truncated or corrupt payload returned +/// a short, silently-partial `AudioSamples` — callers got `Ok` while +/// losing audio (flagged by the 2026-04-22 review). pub fn read_wav(path: &Path) -> Result { let reader = hound::WavReader::open(path) .map_err(|e| KonError::AudioDecodeFailed(format!("WAV open failed: {e}")))?; @@ -131,17 +137,27 @@ pub fn read_wav(path: &Path) -> Result { let spec = reader.spec(); let sample_rate = spec.sample_rate; let channels = spec.channels; + let bits_per_sample = spec.bits_per_sample; let samples: Vec = match spec.sample_format { hound::SampleFormat::Int => reader .into_samples::() - .filter_map(|s| s.ok()) - .map(|s| s as f32 / (1 << (spec.bits_per_sample - 1)) as f32) - .collect(), + .map(|sample| { + sample + .map(|s| s as f32 / (1 << (bits_per_sample - 1)) as f32) + .map_err(|e| { + KonError::AudioDecodeFailed(format!("WAV sample decode failed: {e}")) + }) + }) + .collect::>>()?, hound::SampleFormat::Float => reader .into_samples::() - .filter_map(|s| s.ok()) - .collect(), + .map(|sample| { + sample.map_err(|e| { + KonError::AudioDecodeFailed(format!("WAV sample decode failed: {e}")) + }) + }) + .collect::>>()?, }; Ok(AudioSamples::new(samples, sample_rate, channels)) @@ -217,6 +233,37 @@ mod tests { let _ = std::fs::remove_file(&path); } + #[test] + fn read_wav_surfaces_truncated_sample_stream_errors() { + // Regression for the 2026-04-22 review: filter_map(|s| s.ok()) + // previously swallowed decode errors on corrupt input, so a + // truncated WAV returned Ok with a short samples vec. The + // new code must propagate the error. + let temp_dir = std::env::temp_dir(); + let path = temp_dir.join("kon_test_truncated_wav.wav"); + let _ = std::fs::remove_file(&path); + + // Write 100 samples (200 bytes at 16-bit). + let original = + AudioSamples::mono_16khz((0..100).map(|i| (i as f32) / 100.0).collect()); + write_wav(&path, &original).unwrap(); + + // Drop the last 10 bytes — 5 samples' worth. hound's iterator + // should surface an UnexpectedEof on the final read once its + // internal data-chunk accounting runs out of bytes. + let content = std::fs::read(&path).unwrap(); + let truncated = &content[..content.len() - 10]; + std::fs::write(&path, truncated).unwrap(); + + let result = read_wav(&path); + assert!( + result.is_err(), + "truncated WAV must surface an AudioDecodeFailed error, got: {result:?}" + ); + + let _ = std::fs::remove_file(&path); + } + #[test] fn wav_roundtrip() { let temp_dir = std::env::temp_dir();