diff --git a/CHANGELOG.md b/CHANGELOG.md index 68d0e08b..516d7906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - **ISO completion diagnostics** — strict per-packet validation now records a bounded snapshot of packet lengths, transfer slots, and the selected wire format on the first mismatch, without accepting packets more loosely. This is diagnostic groundwork for #354, not a compatibility workaround. ### Network & SMB +- **SMB account formats and interrupted syncs** — usernames in `DOMAIN\user` or `user@domain` form now reach NTLM split into the account and domain a Windows client would send. If a library walk is interrupted, already-listed tracks are imported, stale rows are not purged, and the failure still surfaces so the next sync can finish the job. - **SMB sync resilience** — an unreadable or vanished child folder no longer aborts the whole library scan; directory loops are depth-capped and duplicate paths are skipped. Folder art larger than the server's single-READ limit now loads through the pipelined reader. - **SMB playback robustness** — a failing progress callback can no longer cancel a shared transfer or start a duplicate download, cache accounting failures no longer discard a completed file, a disappearing cache entry is treated as a miss, a changed remote file invalidates its cached bytes on resync, and a failed next-track download reports cleanly instead of throwing in the background. - **Atomic SMB downloads** — playback, next-track prefetch, and gapless preparation for the same file share one transfer. Downloads publish only after the native completion event, a consistent byte count, and a matching on-disk length; short, cancelled, and incomplete transfers delete their staging file and stay retryable. diff --git a/docs/SMB_VALIDATION.md b/docs/SMB_VALIDATION.md index 141d23f8..c5ced209 100644 --- a/docs/SMB_VALIDATION.md +++ b/docs/SMB_VALIDATION.md @@ -10,10 +10,15 @@ permission for every file or successful decoding/output. Library sync treats the configured root as mandatory but skips an unreadable or vanished child directory (common on Windows shares: `System Volume Information`, per-user folders, or a folder deleted mid-scan) instead of -aborting the whole sync. Directory identities are de-duplicated and recursion is -depth-capped so junction/DFS loops cannot run forever. Cover-art reads use the -pipelined reader, so images larger than the server's single-READ limit still -load. +aborting the whole sync. If the walk is interrupted before it finishes, the +entries already listed are imported, but stale rows are not purged and the scan +still reports failure so the next attempt can complete it. Directory identities +are de-duplicated and recursion is depth-capped so junction/DFS loops cannot +run forever. Cover-art reads use the pipelined reader, so images larger than the +server's single-READ limit still load. + +Username fields accept `DOMAIN\user` and `user@domain`; the native client +receives the account and domain separately, matching a Windows client. Downloads use a same-directory hidden staging file, verify the server's byte count, then close and atomically rename it. Dart also requires the completion @@ -70,6 +75,9 @@ posting addresses, share names or credentials. 4. Interrupt Wi-Fi mid-download, restore it and retry **without clearing app data**. No partial file should play. Repeat with next-track prefetch running while manually selecting that track; both callers should use one transfer. + Also interrupt a library sync and confirm already-listed tracks stay while + the UI still reports the failure; a later successful sync must purge stale + rows normally. 5. Upgrade from a build with a failed SMB transfer. The old cache file must not be reused. Check subsequent cache hits and cache-cap eviction. 6. Fail file decoding/output deliberately; the playback banner should distinguish diff --git a/lib/services/sources/smb_service.dart b/lib/services/sources/smb_service.dart index 41d8b4ed..f5b74fc5 100644 --- a/lib/services/sources/smb_service.dart +++ b/lib/services/sources/smb_service.dart @@ -420,28 +420,40 @@ class SmbService implements NetworkSourceService { // cover/audio classification), yielding progress as the walk streams in. final byDir = >{}; var totalEntries = 0; - await for (final entry in smbListShare( - host: s.host, - port: s.port, - share: s.share, - user: user, - pass: pass, - rootPath: s.rootPath, - )) { - totalEntries++; - if (!entry.isDir) { - final parent = _parentDir(entry.path); - (byDir[parent] ??= []).add(entry); - } - if (totalEntries % 500 == 0) { - yield ScanProgress( - songsFound: 0, - totalFiles: totalEntries, - filesProcessed: totalEntries, - phase: 'Syncing ${server.label}', - currentFile: entry.name, - ); + Object? walkError; + StackTrace? walkStackTrace; + try { + await for (final entry in smbListShare( + host: s.host, + port: s.port, + share: s.share, + user: user, + pass: pass, + rootPath: s.rootPath, + )) { + totalEntries++; + if (!entry.isDir) { + final parent = _parentDir(entry.path); + (byDir[parent] ??= []).add(entry); + } + if (totalEntries % 500 == 0) { + yield ScanProgress( + songsFound: 0, + totalFiles: totalEntries, + filesProcessed: totalEntries, + phase: 'Syncing ${server.label}', + currentFile: entry.name, + ); + } } + } catch (e, stackTrace) { + // A connection drop halfway through the tree must not throw away the + // entries already collected. Import them below, but never purge rows we + // simply did not reach, then surface the incomplete scan to the caller. + walkError = e; + walkStackTrace = stackTrace; + final failure = SmbFailure.from(e); + AppLog.instance.add('$failure result=partial entries=$totalEntries'); } final syncedRemoteIds = {}; @@ -492,6 +504,12 @@ class SmbService implements NetworkSourceService { songsFound += entities.length; } + if (walkError != null) { + // Imported rows stay; no purge or lastSyncedAt stamp for an incomplete + // walk, because unseen files are not stale. + Error.throwWithStackTrace(walkError, walkStackTrace!); + } + await purgeAndStampNetworkSync(server, syncedRemoteIds); yield ScanProgress( diff --git a/rust/src/api/smb_api.rs b/rust/src/api/smb_api.rs index 7ac42dda..acb83c3b 100644 --- a/rust/src/api/smb_api.rs +++ b/rust/src/api/smb_api.rs @@ -51,14 +51,34 @@ fn failure(stage: &str, error: smb2::Error) -> anyhow::Error { anyhow::anyhow!("SMB stage={} code={}", stage, error_code(&error)) } +/// `smb2` takes the account name and domain separately. Users commonly type +/// `DOMAIN\user` or `user@domain` into the single username field; split those +/// forms so NTLM receives the pieces a Windows client would send. Plain local +/// accounts pass through unchanged. +fn split_account(user: &str) -> (&str, &str) { + if let Some((domain, account)) = user.split_once('\\') { + if !domain.is_empty() && !account.is_empty() { + return (account, domain); + } + } + if let Some((account, domain)) = user.split_once('@') { + if !account.is_empty() && !domain.is_empty() { + return (account, domain); + } + } + (user, "") +} + /// Shared client configuration. `smb2::connect`'s shorthand is a five-second, /// no-reconnect client; a share on a waking NAS, or a Wi-Fi roam between /// tracks, should recover in place instead of failing the whole transfer. fn client_config(host: &str, port: u16, user: &str, pass: &str) -> smb2::ClientConfig { + let (account, domain) = split_account(user); smb2::ClientConfig { addr: addr(host, port), - username: user.to_string(), + username: account.to_string(), password: pass.to_string(), + domain: domain.to_string(), timeout: Duration::from_secs(15), auto_reconnect: true, ..Default::default() @@ -334,7 +354,7 @@ pub async fn smb_download_file( mod tests { use super::{ addr, client_config, failure, should_skip_directory_error, skippable_subdirectory_error, - PendingDownload, + split_account, PendingDownload, }; use std::io::Write; use std::path::PathBuf; @@ -466,7 +486,25 @@ mod tests { assert_eq!(config.addr, "nas.local:1445"); assert_eq!(config.username, "user"); assert_eq!(config.password, "pass"); + assert_eq!(config.domain, ""); assert!(config.auto_reconnect); assert_eq!(config.timeout, Duration::from_secs(15)); } + + #[test] + fn account_formats_split_domain_and_user() { + assert_eq!(split_account("alice"), ("alice", "")); + assert_eq!(split_account(r"CORP\alice"), ("alice", "CORP")); + assert_eq!( + split_account("alice@corp.example"), + ("alice", "corp.example") + ); + assert_eq!(split_account(""), ("", "")); + assert_eq!(split_account(r"DOMAIN\"), (r"DOMAIN\", "")); + assert_eq!(split_account(r"\alice"), (r"\alice", "")); + + let config = client_config("nas.local", 445, r"CORP\alice", "pass"); + assert_eq!(config.username, "alice"); + assert_eq!(config.domain, "CORP"); + } }