Skip to main content

max / audiofiles

Audit Run 19 remediations: updater, OAuth, classify, export tests Fixes surfaced by the 2026-07-06 audit that live in single-concern files: - H2 updater: replace UpdateStatus.dismissed bool with dismissed_version so a newer release re-surfaces its banner instead of being suppressed until restart. - H4 auth: read the OAuth callback to end-of-request-line (chunked, 8 KiB cap, 5s per-read timeout) rather than one 2048-byte read that could truncate the code/state and hang login; drop the dead set_nonblocking pair; extract read_request_head + parse_oauth_callback (+7 tests). - H5a export/runner: add tests for resolve_collision, write_atomic tmp cleanup, and run_export cancel/missing-source paths (Test F -> covered, +8 tests). - H6 classify: extract the 40+ decision-tree cutoffs into a documented thresholds submodule; behavior unchanged (unit + golden validation tests). - H8 spectral: correct the stale "hop size = window/4" doc (code uses no overlap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-06 15:13 UTC
Commit: 3e902f1ad1e3fcf09e4d9e3f77fca43618c031dd
Parent: 3366b8a
5 files changed, +506 insertions, -86 deletions
M crates/audiofiles-app/src/updater.rs +56 -13
@@ -36,7 +36,11 @@ pub struct UpdateStatus {
36 36 pub version: String,
37 37 pub notes: String,
38 38 pub download_url: String,
39 - pub dismissed: bool,
39 + /// The version the user dismissed the banner for, if any. Stored as the
40 + /// version string rather than a bool so that dismissing v1.1.0 suppresses
41 + /// only that release — a later v1.2.0 re-surfaces the banner instead of
42 + /// being silently swallowed until app restart.
43 + pub dismissed_version: Option<String>,
40 44 }
41 45
42 46 /// Handle to the update checker. Clone-cheap (Arc-wrapped).
@@ -118,15 +122,20 @@ impl UpdateChecker {
118 122 }
119 123 }
120 124
121 - /// Dismiss the update notification (user clicked dismiss).
125 + /// Dismiss the update notification (user clicked dismiss). Pins the
126 + /// dismissal to the currently-advertised version so a later, strictly
127 + /// newer release still shows its own banner.
122 128 pub fn dismiss(&self) {
123 - self.status.lock().dismissed = true;
129 + let mut s = self.status.lock();
130 + s.dismissed_version = Some(s.version.clone());
124 131 }
125 132
126 - /// Whether to show the update banner.
133 + /// Whether to show the update banner. Shown when an update is available and
134 + /// the user has not dismissed *this* version (dismissing an older version
135 + /// does not suppress a newer one).
127 136 pub fn should_show(&self) -> bool {
128 137 let s = self.status.lock();
129 - s.available && !s.dismissed
138 + s.available && s.dismissed_version.as_deref() != Some(s.version.as_str())
130 139 }
131 140 }
132 141
@@ -219,7 +228,7 @@ mod tests {
219 228 fn update_status_default_is_inactive() {
220 229 let s = UpdateStatus::default();
221 230 assert!(!s.available);
222 - assert!(!s.dismissed);
231 + assert!(s.dismissed_version.is_none());
223 232 assert!(s.version.is_empty());
224 233 assert!(s.notes.is_empty());
225 234 assert!(s.download_url.is_empty());
@@ -233,18 +242,24 @@ mod tests {
233 242 }
234 243
235 244 #[test]
236 - fn dismiss_sets_flag() {
237 - let checker = UpdateChecker::inert();
238 - assert!(!checker.status.lock().dismissed);
245 + fn dismiss_pins_current_version() {
246 + let checker = checker_with_status(UpdateStatus {
247 + available: true,
248 + version: "1.1.0".to_string(),
249 + ..Default::default()
250 + });
251 + assert!(checker.status.lock().dismissed_version.is_none());
239 252 checker.dismiss();
240 - assert!(checker.status.lock().dismissed);
253 + assert_eq!(
254 + checker.status.lock().dismissed_version.as_deref(),
255 + Some("1.1.0")
256 + );
241 257 }
242 258
243 259 #[test]
244 260 fn should_show_when_available_and_not_dismissed() {
245 261 let checker = checker_with_status(UpdateStatus {
246 262 available: true,
247 - dismissed: false,
248 263 version: "1.0.0".to_string(),
249 264 ..Default::default()
250 265 });
@@ -258,10 +273,11 @@ mod tests {
258 273 }
259 274
260 275 #[test]
261 - fn should_not_show_when_dismissed() {
276 + fn should_not_show_when_this_version_dismissed() {
262 277 let checker = checker_with_status(UpdateStatus {
263 278 available: true,
264 - dismissed: true,
279 + version: "1.1.0".to_string(),
280 + dismissed_version: Some("1.1.0".to_string()),
265 281 ..Default::default()
266 282 });
267 283 assert!(!checker.should_show());
@@ -271,6 +287,7 @@ mod tests {
271 287 fn dismiss_then_should_show_returns_false() {
272 288 let checker = checker_with_status(UpdateStatus {
273 289 available: true,
290 + version: "1.1.0".to_string(),
274 291 ..Default::default()
275 292 });
276 293 assert!(checker.should_show());
@@ -279,6 +296,32 @@ mod tests {
279 296 }
280 297
281 298 #[test]
299 + fn newer_version_resurfaces_after_dismiss() {
300 + // Regression: dismissing v1.1.0 must not suppress a later v1.2.0.
301 + let checker = checker_with_status(UpdateStatus {
302 + available: true,
303 + version: "1.1.0".to_string(),
304 + ..Default::default()
305 + });
306 + checker.dismiss();
307 + assert!(!checker.should_show());
308 +
309 + // A subsequent check surfaces a strictly newer release.
310 + {
311 + let mut s = checker.status.lock();
312 + s.version = "1.2.0".to_string();
313 + }
314 + assert!(
315 + checker.should_show(),
316 + "a newer version than the dismissed one must re-show the banner"
317 + );
318 +
319 + // Dismissing again pins the new version and re-hides.
320 + checker.dismiss();
321 + assert!(!checker.should_show());
322 + }
323 +
324 + #[test]
282 325 fn set_enabled_on_inert_is_noop() {
283 326 let checker = UpdateChecker::inert();
284 327 // No panic, no observable side-effect — there's no task to gate.
M crates/audiofiles-core/src/analysis/classify.rs +105 -16
@@ -334,6 +334,91 @@ impl SampleClass {
334 334
335 335 // ── Legacy functions ──
336 336
337 + /// Decision-tree thresholds for [`classify_full`], named so the taxonomy is
338 + /// legible and tunable in one place rather than scattered as bare literals.
339 + ///
340 + /// Units: durations in seconds; `centroid`/`bandwidth` in Hz; `flatness`, `zcr`
341 + /// dimensionless in `0..1`; `crest` a linear peak/RMS ratio; `attack` in seconds;
342 + /// `centroid_variance` in Hz². Names read `<CLASS>_<FEATURE>_<MIN|MAX>`, where
343 + /// `MIN`/`MAX` is the inclusive-side bound the rule tests (`> MIN`, `< MAX`).
344 + mod thresholds {
345 + /// Upper duration bound shared by the drum/percussion phase — a "short" hit.
346 + pub const SHORT_SECS: f64 = 2.0;
347 +
348 + /// Rule 1 — Noise: near-uniform spectral energy.
349 + pub const NOISE_FLATNESS_MIN: f64 = 0.7;
350 +
351 + // Rule 2 — Kick: short, low-frequency dominant.
352 + pub const KICK_CENTROID_MAX: f64 = 1200.0;
353 + pub const KICK_FLATNESS_MAX: f64 = 0.35;
354 +
355 + // Rule 3 — HiHat: short, bright, metallic (relatively tonal).
356 + pub const HIHAT_CENTROID_MIN: f64 = 3500.0;
357 + pub const HIHAT_ZCR_MIN: f64 = 0.1;
358 + pub const HIHAT_FLATNESS_MAX: f64 = 0.3;
359 +
360 + // Rule 4 — Cymbal: bright, noisy sustain (crashes, rides).
361 + pub const CYMBAL_DUR_MIN: f64 = 0.5;
362 + pub const CYMBAL_DUR_MAX: f64 = 10.0;
363 + pub const CYMBAL_CENTROID_MIN: f64 = 3000.0;
364 + pub const CYMBAL_FLATNESS_MIN: f64 = 0.25;
365 +
366 + // Rule 5 — Snare: short, mid/high, broadband noise from the snare wires.
367 + pub const SNARE_CENTROID_MIN: f64 = 800.0;
368 + pub const SNARE_FLATNESS_MIN: f64 = 0.2;
369 + pub const SNARE_BANDWIDTH_MIN: f64 = 2000.0;
370 +
371 + // Rule 6 — Percussion: short percussive catch-all (sharp attack OR high crest).
372 + pub const PERC_ATTACK_MAX: f64 = 0.05;
373 + pub const PERC_CREST_MIN: f64 = 3.0;
374 +
375 + // Rule 7 — Impact: very sharp non-drum transient.
376 + pub const IMPACT_DUR_MAX: f64 = 3.0;
377 + pub const IMPACT_CREST_MIN: f64 = 10.0;
378 + pub const IMPACT_ATTACK_MAX: f64 = 0.005;
379 +
380 + // Rule 8 — Ambience: long, spectrally static, moderate noise.
381 + pub const AMBIENCE_DUR_MIN: f64 = 5.0;
382 + pub const AMBIENCE_CV_MAX: f64 = 100_000.0;
383 + pub const AMBIENCE_FLATNESS_MIN: f64 = 0.15;
384 + pub const AMBIENCE_FLATNESS_MAX: f64 = 0.5;
385 +
386 + // Rule 9 — Bass: low-frequency, tonal.
387 + pub const BASS_CENTROID_MAX: f64 = 400.0;
388 + pub const BASS_FLATNESS_MAX: f64 = 0.15;
389 +
390 + // Rule 10 — Vocal: mid-range, tonal, smooth waveform, sustained.
391 + pub const VOCAL_DUR_MIN: f64 = 0.5;
392 + pub const VOCAL_CENTROID_MIN: f64 = 300.0;
393 + pub const VOCAL_CENTROID_MAX: f64 = 3000.0;
394 + pub const VOCAL_FLATNESS_MAX: f64 = 0.2;
395 + pub const VOCAL_ZCR_MAX: f64 = 0.08;
396 +
397 + // Rule 11 — Pad: long, tonal, mid-range.
398 + pub const PAD_DUR_MIN: f64 = 2.0;
399 + pub const PAD_FLATNESS_MAX: f64 = 0.2;
400 + pub const PAD_CENTROID_MIN: f64 = 200.0;
401 + pub const PAD_CENTROID_MAX: f64 = 2000.0;
402 +
403 + // Rule 12 — Texture: long, spectrally evolving.
404 + pub const TEXTURE_DUR_MIN: f64 = 2.0;
405 + pub const TEXTURE_CV_MIN: f64 = 500_000.0;
406 +
407 + // Rule 13 — Foley: broadband, moderate noise.
408 + pub const FOLEY_DUR_MIN: f64 = 0.1;
409 + pub const FOLEY_BANDWIDTH_MIN: f64 = 2000.0;
410 + pub const FOLEY_FLATNESS_MIN: f64 = 0.1;
411 + pub const FOLEY_FLATNESS_MAX: f64 = 0.5;
412 +
413 + // Rule 14 — Synth: tonal, mid/high centroid, smooth waveform.
414 + pub const SYNTH_CENTROID_MIN: f64 = 500.0;
415 + pub const SYNTH_FLATNESS_MAX: f64 = 0.3;
416 + pub const SYNTH_ZCR_MAX: f64 = 0.1;
417 +
418 + /// Rule 16 — Music: long catch-all once no drum/tonal rule matched.
419 + pub const MUSIC_DUR_MIN: f64 = 3.0;
420 + }
421 +
337 422 /// Classify a sample using the full feature set (16 classes, priority-ordered decision tree).
338 423 ///
339 424 /// Uses threshold-based rules — no ML. Rules are evaluated in priority order. The tree is
@@ -344,8 +429,12 @@ impl SampleClass {
344 429 /// The expanded feature set (crest factor, attack time, bandwidth, centroid variance)
345 430 /// enables game audio categories (ambience, impact, foley, texture) that the original
346 431 /// 4-feature classifier collapsed into Misc/Percussion/Music.
432 + ///
433 + /// All cutoffs live in [`thresholds`].
347 434 #[instrument(skip_all)]
348 435 pub fn classify_full(input: &ClassifyInput) -> SampleClass {
436 + use thresholds as t;
437 +
349 438 let d = input.duration;
350 439 let c = input.centroid;
351 440 let flat = input.flatness;
@@ -357,86 +446,86 @@ pub fn classify_full(input: &ClassifyInput) -> SampleClass {
357 446 let attack = input.attack_time;
358 447
359 448 // 1. Noise: energy spread nearly uniformly across all frequencies
360 - if flat > 0.7 {
449 + if flat > t::NOISE_FLATNESS_MIN {
361 450 return SampleClass::Noise;
362 451 }
363 452
364 453 // --- Phase 1: Drum/percussion classification (short sounds first) ---
365 454
366 455 // 2. Kick: short, low-frequency dominant
367 - if d < 2.0 && c < 1200.0 && flat < 0.35 {
456 + if d < t::SHORT_SECS && c < t::KICK_CENTROID_MAX && flat < t::KICK_FLATNESS_MAX {
368 457 return SampleClass::Kick;
369 458 }
370 459
371 460 // 3. HiHat: short, bright, metallic (relatively tonal)
372 - if d < 2.0 && c > 3500.0 && zcr > 0.1 && flat < 0.3 {
461 + if d < t::SHORT_SECS && c > t::HIHAT_CENTROID_MIN && zcr > t::HIHAT_ZCR_MIN && flat < t::HIHAT_FLATNESS_MAX {
373 462 return SampleClass::HiHat;
374 463 }
375 464
376 465 // 4. Cymbal: bright, noisy sustain (crashes, rides)
377 - if d > 0.5 && d < 10.0 && c > 3000.0 && flat > 0.25 {
466 + if d > t::CYMBAL_DUR_MIN && d < t::CYMBAL_DUR_MAX && c > t::CYMBAL_CENTROID_MIN && flat > t::CYMBAL_FLATNESS_MIN {
378 467 return SampleClass::Cymbal;
379 468 }
380 469
381 470 // 5. Snare: short, mid-to-high frequency, broadband noise from snare wires
382 - if d < 2.0 && c > 800.0 && flat > 0.2 && bw > 2000.0 {
471 + if d < t::SHORT_SECS && c > t::SNARE_CENTROID_MIN && flat > t::SNARE_FLATNESS_MIN && bw > t::SNARE_BANDWIDTH_MIN {
383 472 return SampleClass::Snare;
384 473 }
385 474
386 475 // 6. Percussion: short percussive catch-all
387 - if d < 2.0 && (attack < 0.05 || crest > 3.0) {
476 + if d < t::SHORT_SECS && (attack < t::PERC_ATTACK_MAX || crest > t::PERC_CREST_MIN) {
388 477 return SampleClass::Percussion;
389 478 }
390 479
391 480 // --- Phase 2: Non-drum classification ---
392 481
393 482 // 7. Impact: very sharp non-drum transient
394 - if d < 3.0 && crest > 10.0 && attack < 0.005 {
483 + if d < t::IMPACT_DUR_MAX && crest > t::IMPACT_CREST_MIN && attack < t::IMPACT_ATTACK_MAX {
395 484 return SampleClass::Impact;
396 485 }
397 486
398 487 // 8. Ambience: long, spectrally static, moderate noise
399 - if d > 5.0 && cv < 100_000.0 && flat > 0.15 && flat < 0.5 {
488 + if d > t::AMBIENCE_DUR_MIN && cv < t::AMBIENCE_CV_MAX && flat > t::AMBIENCE_FLATNESS_MIN && flat < t::AMBIENCE_FLATNESS_MAX {
400 489 return SampleClass::Ambience;
401 490 }
402 491
403 492 // 9. Bass: low-frequency, tonal
404 - if c < 400.0 && flat < 0.15 {
493 + if c < t::BASS_CENTROID_MAX && flat < t::BASS_FLATNESS_MAX {
405 494 return SampleClass::Bass;
406 495 }
407 496
408 497 // 10. Vocal: mid-range, tonal, smooth waveform, sustained
409 - if d > 0.5 && c > 300.0 && c < 3000.0 && flat < 0.2 && zcr < 0.08 {
498 + if d > t::VOCAL_DUR_MIN && c > t::VOCAL_CENTROID_MIN && c < t::VOCAL_CENTROID_MAX && flat < t::VOCAL_FLATNESS_MAX && zcr < t::VOCAL_ZCR_MAX {
410 499 return SampleClass::Vocal;
411 500 }
412 501
413 502 // 11. Pad: long, tonal, mid-range
414 - if d > 2.0 && flat < 0.2 && c > 200.0 && c < 2000.0 {
503 + if d > t::PAD_DUR_MIN && flat < t::PAD_FLATNESS_MAX && c > t::PAD_CENTROID_MIN && c < t::PAD_CENTROID_MAX {
415 504 return SampleClass::Pad;
416 505 }
417 506
418 507 // 12. Texture: long, spectrally evolving
419 - if d > 2.0 && cv > 500_000.0 {
508 + if d > t::TEXTURE_DUR_MIN && cv > t::TEXTURE_CV_MIN {
420 509 return SampleClass::Texture;
421 510 }
422 511
423 512 // 13. Foley: broadband, moderate noise
424 - if d > 0.1 && bw > 2000.0 && flat > 0.1 && flat < 0.5 {
513 + if d > t::FOLEY_DUR_MIN && bw > t::FOLEY_BANDWIDTH_MIN && flat > t::FOLEY_FLATNESS_MIN && flat < t::FOLEY_FLATNESS_MAX {
425 514 return SampleClass::Foley;
426 515 }
427 516
428 517 // 14. Synth: tonal, mid-to-high centroid, smooth waveform
429 - if c > 500.0 && flat < 0.3 && zcr < 0.1 {
518 + if c > t::SYNTH_CENTROID_MIN && flat < t::SYNTH_FLATNESS_MAX && zcr < t::SYNTH_ZCR_MAX {
430 519 return SampleClass::Synth;
431 520 }
432 521
433 522 // 15. Percussion: short catch-all
434 - if d < 2.0 {
523 + if d < t::SHORT_SECS {
435 524 return SampleClass::Percussion;
436 525 }
437 526
438 527 // 16. Music: long catch-all
439 - if d > 3.0 {
528 + if d > t::MUSIC_DUR_MIN {
440 529 return SampleClass::Music;
441 530 }
442 531
M crates/audiofiles-core/src/analysis/spectral.rs +2 -1
@@ -31,7 +31,8 @@ pub struct SpectralFeatures {
31 31
32 32 /// Compute spectral features from mono audio samples.
33 33 ///
34 - /// Uses STFT with Hann window, hop size = window/4.
34 + /// Uses STFT with a Hann window and no frame overlap (hop size = window): this
35 + /// classifier needs stable spectral averages, not frame-level temporal detail.
35 36 /// All features are averaged across frames.
36 37 ///
37 38 /// Requires the `analysis` feature (pulls in `realfft`).
M crates/audiofiles-core/src/export/runner.rs +163
@@ -288,3 +288,166 @@ fn export_single_item(
288 288 }
289 289 }
290 290 }
291 +
292 + #[cfg(test)]
293 + mod tests {
294 + use super::*;
295 + use crate::export::{ExportChannels, ExportConfig, ExportFormat, ExportItem};
296 +
297 + fn write_source(dir: &Path, name: &str, bytes: &[u8]) -> PathBuf {
298 + let p = dir.join(name);
299 + fs::write(&p, bytes).unwrap();
300 + p
301 + }
302 +
303 + /// An `Original`-format item that copies directly from `source` (bypassing the
304 + /// store), so the copied bytes don't need to be valid audio.
305 + fn item_from(source: &Path, name: &str) -> ExportItem {
306 + ExportItem {
307 + hash: crate::SampleHash::from_trusted("a".repeat(64)),
308 + ext: "wav".to_string(),
309 + relative_path: PathBuf::from(name),
310 + name: name.to_string(),
311 + bpm: Some(120.0),
312 + musical_key: None,
313 + classification: Some("kick".to_string()),
314 + duration: Some(1.5),
315 + tags: vec!["drum".to_string()],
316 + source_path: Some(source.to_path_buf()),
317 + }
318 + }
319 +
320 + fn original_config(dest: &Path, sidecar: bool) -> ExportConfig {
321 + ExportConfig {
322 + format: ExportFormat::Original,
323 + sample_rate: None,
324 + bit_depth: None,
325 + channels: ExportChannels::Original,
326 + naming_pattern: None,
327 + flatten: true,
328 + metadata_sidecar: sidecar,
329 + destination: dest.to_path_buf(),
330 + device_profile: None,
331 + naming_rules: None,
332 + max_file_size_bytes: None,
333 + name_overrides: None,
334 + }
335 + }
336 +
337 + #[test]
338 + fn resolve_collision_appends_free_suffix() {
339 + let dir = tempfile::tempdir().unwrap();
340 + let base = dir.path().join("kick.wav");
341 + // No collision: returns the path unchanged.
342 + assert_eq!(resolve_collision(&base), base);
343 + // First collision → _1, second → _2.
344 + fs::write(&base, b"x").unwrap();
345 + assert_eq!(resolve_collision(&base), dir.path().join("kick_1.wav"));
346 + fs::write(dir.path().join("kick_1.wav"), b"x").unwrap();
347 + assert_eq!(resolve_collision(&base), dir.path().join("kick_2.wav"));
348 + }
349 +
350 + #[test]
351 + fn tmp_path_for_appends_marker() {
352 + assert_eq!(
353 + tmp_path_for(Path::new("/out/kick.wav")),
354 + PathBuf::from("/out/kick.wav.audiofiles_tmp")
355 + );
356 + }
357 +
358 + #[test]
359 + fn write_atomic_success_renames_and_leaves_no_tmp() {
360 + let dir = tempfile::tempdir().unwrap();
361 + let dest = dir.path().join("out.bin");
362 + write_atomic(&dest, |tmp| fs::write(tmp, b"hello").map_err(|e| io_err(tmp, e))).unwrap();
363 + assert_eq!(fs::read(&dest).unwrap(), b"hello");
364 + assert!(!tmp_path_for(&dest).exists(), "tmp must be gone after success");
365 + }
366 +
367 + #[test]
368 + fn write_atomic_failure_removes_tmp_and_leaves_no_dest() {
369 + let dir = tempfile::tempdir().unwrap();
370 + let dest = dir.path().join("out.bin");
371 + let res = write_atomic(&dest, |tmp| {
372 + fs::write(tmp, b"partial").unwrap();
373 + Err(crate::error::CoreError::Cancelled)
374 + });
375 + assert!(res.is_err());
376 + assert!(!dest.exists(), "no dest is written on failure");
377 + assert!(!tmp_path_for(&dest).exists(), "partial tmp is cleaned up on failure");
378 + }
379 +
380 + #[test]
381 + fn run_export_original_copies_and_writes_sidecar() {
382 + let dir = tempfile::tempdir().unwrap();
383 + let src = write_source(dir.path(), "src.wav", b"RIFFxxxx");
384 + let dest = dir.path().join("out");
385 + let store = SampleStore::new(dir.path().join("store")).unwrap();
386 + let items = vec![item_from(&src, "kick.wav")];
387 + let cancel = AtomicBool::new(false);
388 +
389 + let summary =
390 + run_export(&items, &original_config(&dest, true), &store, &cancel, |_, _, _| true)
391 + .unwrap();
392 + assert_eq!(summary.total, 1);
393 + assert!(summary.errors.is_empty(), "unexpected errors: {:?}", summary.errors);
394 + assert_eq!(fs::read(dest.join("kick.wav")).unwrap(), b"RIFFxxxx");
395 + assert!(dest.join("kick.wav.audiofiles.json").exists(), "sidecar written");
396 + }
397 +
398 + #[test]
399 + fn run_export_precancelled_flag_produces_nothing() {
400 + let dir = tempfile::tempdir().unwrap();
401 + let src = write_source(dir.path(), "src.wav", b"data");
402 + let dest = dir.path().join("out");
403 + let store = SampleStore::new(dir.path().join("store")).unwrap();
404 + let items = vec![item_from(&src, "kick.wav")];
405 + // Cancel flag already set before the first item is reached.
406 + let cancel = AtomicBool::new(true);
407 +
408 + let summary =
409 + run_export(&items, &original_config(&dest, false), &store, &cancel, |_, _, _| true)
410 + .unwrap();
411 + assert_eq!(summary.total, 1);
412 + assert!(summary.errors.is_empty());
413 + assert!(!dest.join("kick.wav").exists(), "a pre-set cancel writes nothing");
414 + }
415 +
416 + #[test]
417 + fn run_export_callback_cancel_stops_batch() {
418 + let dir = tempfile::tempdir().unwrap();
419 + let src = write_source(dir.path(), "src.wav", b"data");
420 + let dest = dir.path().join("out");
421 + let store = SampleStore::new(dir.path().join("store")).unwrap();
422 + let items = vec![item_from(&src, "a.wav"), item_from(&src, "b.wav")];
423 + let cancel = AtomicBool::new(false);
424 +
425 + // Callback returns false at the second item (i == 1): only the first exports.
426 + let summary =
427 + run_export(&items, &original_config(&dest, false), &store, &cancel, |i, _, _| i < 1)
428 + .unwrap();
429 + assert_eq!(summary.total, 2);
430 + assert!(dest.join("a.wav").exists(), "first item exported");
431 + assert!(!dest.join("b.wav").exists(), "batch stops before the second item");
432 + }
433 +
434 + #[test]
435 + fn run_export_missing_source_records_error_and_continues() {
436 + let dir = tempfile::tempdir().unwrap();
437 + let good = write_source(dir.path(), "good.wav", b"ok");
438 + let dest = dir.path().join("out");
439 + let store = SampleStore::new(dir.path().join("store")).unwrap();
440 + // First item points at a non-existent source (and the store lacks the blob);
441 + // second is valid. The batch must report the error yet still export the good one.
442 + let mut missing = item_from(&good, "missing.wav");
443 + missing.source_path = Some(dir.path().join("nope.wav"));
444 + let items = vec![missing, item_from(&good, "good.wav")];
445 + let cancel = AtomicBool::new(false);
446 +
447 + let summary =
448 + run_export(&items, &original_config(&dest, false), &store, &cancel, |_, _, _| true)
449 + .unwrap();
450 + assert_eq!(summary.errors.len(), 1, "one per-item error, not a batch abort");
451 + assert!(dest.join("good.wav").exists(), "valid item still exported after the error");
452 + }
453 + }
M crates/audiofiles-sync/src/auth.rs +180 -56
@@ -71,14 +71,10 @@ pub fn start_auth(client: &SyncKitClient) -> Result<AuthSession> {
71 71 // logs and the oneshot sender drops, so the awaiting caller sees a cancelled
72 72 // channel rather than a silently dead listener).
73 73 let _ = audiofiles_core::worker_runtime::spawn_detached("oauth-listener", move || {
74 - listener
75 - .set_nonblocking(false)
76 - .ok();
77 -
78 - // 5-minute timeout
79 - listener
80 - .set_nonblocking(true)
81 - .ok();
74 + // Non-blocking accept so the loop can poll the 5-minute deadline between
75 + // connection attempts; each accepted stream then gets a bounded blocking
76 + // read (see `read_request_head`).
77 + listener.set_nonblocking(true).ok();
82 78
83 79 let deadline = std::time::Instant::now() + std::time::Duration::from_secs(300);
84 80
@@ -90,63 +86,38 @@ pub fn start_auth(client: &SyncKitClient) -> Result<AuthSession> {
90 86
91 87 match listener.accept() {
92 88 Ok((mut stream, _)) => {
93 - use std::io::{Read, Write};
89 + use std::io::Write;
94 90
95 - let mut buf = [0u8; 2048];
96 - let n = stream.read(&mut buf).unwrap_or(0);
97 - let request = String::from_utf8_lossy(&buf[..n]);
91 + // Read the whole HTTP request line rather than a single 2048-byte
92 + // chunk: a segmented or slow-start request can split the line
93 + // across TCP reads, which would truncate `code`/`state` and hang
94 + // auth to the 5-minute timeout. Bounded by an 8 KiB cap and a
95 + // per-read timeout so a stalled client can't wedge the listener.
96 + let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5)));
97 + let request = read_request_head(&mut stream, 8192);
98 98
99 99 // Parse GET /callback?code=X&state=Y or GET /?code=X&state=Y
100 - if let Some(query_start) = request.find('?') {
101 - let after_q = query_start + 1;
102 - let query_end = request[query_start..]
103 - .find(' ')
104 - .unwrap_or(request.len().saturating_sub(query_start));
105 - let end = query_start + query_end;
106 -
107 - let query = match request.get(after_q..end) {
108 - Some(q) => q,
109 - None => {
110 - tracing::warn!("Malformed OAuth callback request, could not parse query string");
111 - break;
112 - }
113 - };
114 -
115 - let mut code = None;
116 - let mut cb_state = None;
117 -
118 - for param in query.split('&') {
119 - if let Some((key, value)) = param.split_once('=') {
120 - match key {
121 - "code" => code = Some(percent_decode(value)),
122 - "state" => cb_state = Some(percent_decode(value)),
123 - _ => {}
124 - }
125 - }
126 - }
127 -
128 - if let (Some(code), Some(state)) = (code, cb_state) {
129 - if state != thread_expected_state {
130 - tracing::warn!("OAuth callback state mismatch — rejecting");
131 - let body = "<html><body><h1>Authentication failed</h1><p>CSRF state mismatch. Please try again.</p></body></html>";
132 - let response = format!(
133 - "HTTP/1.1 403 Forbidden\r\nContent-Type: text/html\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n{}",
134 - body.len(),
135 - body
136 - );
137 - let _ = stream.write_all(response.as_bytes());
138 - break;
139 - }
140 -
141 - let body = "<html><body><h1>Authentication successful</h1><p>You can close this tab and return to audiofiles.</p></body></html>";
100 + if let Some((code, cb_state)) = parse_oauth_callback(&request) {
101 + if cb_state != thread_expected_state {
102 + tracing::warn!("OAuth callback state mismatch — rejecting");
103 + let body = "<html><body><h1>Authentication failed</h1><p>CSRF state mismatch. Please try again.</p></body></html>";
142 104 let response = format!(
143 - "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n{}",
105 + "HTTP/1.1 403 Forbidden\r\nContent-Type: text/html\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n{}",
144 106 body.len(),
145 107 body
146 108 );
147 109 let _ = stream.write_all(response.as_bytes());
148 - let _ = tx.send(CallbackResult { code, state });
110 + break;
149 111 }
112 +
113 + let body = "<html><body><h1>Authentication successful</h1><p>You can close this tab and return to audiofiles.</p></body></html>";
114 + let response = format!(
115 + "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n{}",
116 + body.len(),
117 + body
118 + );
119 + let _ = stream.write_all(response.as_bytes());
120 + let _ = tx.send(CallbackResult { code, state: cb_state });
150 121 }
151 122 break;
152 123 }
@@ -168,6 +139,62 @@ pub fn start_auth(client: &SyncKitClient) -> Result<AuthSession> {
168 139 })
169 140 }
170 141
142 + /// Read an HTTP request head from `stream` until the end of the request line
143 + /// (the first `\n`), EOF, a read error/timeout, or `cap` bytes — whichever comes
144 + /// first. Reads in chunks so a request delivered across several TCP segments is
145 + /// assembled into one string rather than truncated by a single fixed-size read.
146 + fn read_request_head(stream: &mut impl std::io::Read, cap: usize) -> String {
147 + let mut data: Vec<u8> = Vec::with_capacity(2048);
148 + let mut chunk = [0u8; 1024];
149 + loop {
150 + match stream.read(&mut chunk) {
151 + Ok(0) => break,
152 + Ok(n) => {
153 + data.extend_from_slice(&chunk[..n]);
154 + // The request line ends at the first newline; once we have it we
155 + // have everything the callback parser needs.
156 + if data.contains(&b'\n') || data.len() >= cap {
157 + break;
158 + }
159 + }
160 + Err(_) => break,
161 + }
162 + }
163 + if data.len() > cap {
164 + data.truncate(cap);
165 + }
166 + String::from_utf8_lossy(&data).into_owned()
167 + }
168 +
169 + /// Extract `(code, state)` from an HTTP callback request, percent-decoding both.
170 + /// Returns `None` unless both parameters are present. Parses only the query of
171 + /// the request line (`GET /callback?code=X&state=Y HTTP/1.1`), so a `?` appearing
172 + /// later in a header cannot be mistaken for the query.
173 + fn parse_oauth_callback(request: &str) -> Option<(String, String)> {
174 + let query_start = request.find('?')?;
175 + let after_q = query_start + 1;
176 + // The query ends at the first space after `?` (the ` HTTP/1.1` delimiter) or
177 + // at end-of-input if the line was truncated before that space.
178 + let end = request[query_start..]
179 + .find(' ')
180 + .map(|rel| query_start + rel)
181 + .unwrap_or(request.len());
182 + let query = request.get(after_q..end)?;
183 +
184 + let mut code = None;
185 + let mut cb_state = None;
186 + for param in query.split('&') {
187 + if let Some((key, value)) = param.split_once('=') {
188 + match key {
189 + "code" => code = Some(percent_decode(value)),
190 + "state" => cb_state = Some(percent_decode(value)),
191 + _ => {}
192 + }
193 + }
194 + }
195 + Some((code?, cb_state?))
196 + }
197 +
171 198 /// Decode percent-encoded UTF-8 strings (RFC 3986).
172 199 /// Passes through invalid sequences unchanged.
173 200 ///
@@ -244,4 +271,101 @@ mod tests {
244 271 assert_eq!(percent_decode("abc%"), "abc%");
245 272 assert_eq!(percent_decode("abc%4"), "abc%4");
246 273 }
274 +
275 + #[test]
276 + fn parse_oauth_callback_extracts_code_and_state() {
277 + let req = "GET /callback?code=abc123&state=xyz HTTP/1.1\r\nHost: localhost\r\n\r\n";
278 + assert_eq!(
279 + parse_oauth_callback(req),
280 + Some(("abc123".to_string(), "xyz".to_string()))
281 + );
282 + }
283 +
284 + #[test]
285 + fn parse_oauth_callback_percent_decodes() {
286 + let req = "GET /?code=a%2Fb&state=s%20t HTTP/1.1\r\n";
287 + assert_eq!(
288 + parse_oauth_callback(req),
289 + Some(("a/b".to_string(), "s t".to_string()))
290 + );
291 + }
292 +
293 + #[test]
294 + fn parse_oauth_callback_requires_both_params() {
295 + assert_eq!(parse_oauth_callback("GET /?code=abc HTTP/1.1\r\n"), None);
296 + assert_eq!(parse_oauth_callback("GET /?state=xyz HTTP/1.1\r\n"), None);
297 + assert_eq!(parse_oauth_callback("GET / HTTP/1.1\r\n"), None);
298 + }
299 +
300 + #[test]
301 + fn parse_oauth_callback_ignores_later_question_mark() {
302 + // A `?` in a header must not be mistaken for the query start.
303 + let req = "GET /callback?code=c&state=s HTTP/1.1\r\nReferer: http://x/?q=1\r\n";
304 + assert_eq!(
305 + parse_oauth_callback(req),
306 + Some(("c".to_string(), "s".to_string()))
307 + );
308 + }
309 +
310 + /// A `Read` that hands out at most `chunk` bytes per call, to simulate a
311 + /// request delivered across several TCP segments.
312 + struct ChunkedReader {
313 + data: Vec<u8>,
314 + pos: usize,
315 + chunk: usize,
316 + }
317 +
318 + impl std::io::Read for ChunkedReader {
319 + fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
320 + let remaining = &self.data[self.pos..];
321 + let n = remaining.len().min(self.chunk).min(buf.len());
322 + buf[..n].copy_from_slice(&remaining[..n]);
323 + self.pos += n;
324 + Ok(n)
325 + }
326 + }
327 +
328 + #[test]
329 + fn read_request_head_assembles_segmented_input() {
330 + // The full request line split into 4-byte segments — a single 2048-byte
331 + // read would have grabbed only the first chunk. The assembled string must
332 + // parse cleanly, proving the truncation bug is fixed.
333 + let full = "GET /callback?code=abc123&state=xyz HTTP/1.1\r\n\r\n";
334 + let mut reader = ChunkedReader {
335 + data: full.as_bytes().to_vec(),
336 + pos: 0,
337 + chunk: 4,
338 + };
339 + let got = read_request_head(&mut reader, 8192);
340 + assert!(got.starts_with("GET /callback?code=abc123&state=xyz"));
341 + assert_eq!(
342 + parse_oauth_callback(&got),
343 + Some(("abc123".to_string(), "xyz".to_string()))
344 + );
345 + }
346 +
347 + #[test]
348 + fn read_request_head_respects_cap() {
349 + let mut reader = ChunkedReader {
350 + data: vec![b'a'; 10_000],
351 + pos: 0,
352 + chunk: 1024,
353 + };
354 + let got = read_request_head(&mut reader, 8192);
355 + assert!(got.len() <= 8192, "must not read past the cap");
356 + }
357 +
358 + #[test]
359 + fn read_request_head_stops_at_first_newline() {
360 + // Everything after the first `\n` is irrelevant to the parser; the reader
361 + // should stop once the request line is complete rather than draining body.
362 + let full = "GET /?code=c&state=s HTTP/1.1\r\nHost: x\r\n\r\nBODYBODYBODY";
363 + let mut reader = ChunkedReader {
364 + data: full.as_bytes().to_vec(),
365 + pos: 0,
366 + chunk: 8,
367 + };
368 + let got = read_request_head(&mut reader, 8192);
369 + assert!(!got.contains("BODYBODYBODY"), "should not drain past request line");
370 + }
247 371 }