diff --git a/crates/vnidrop/CORE_FLOW.md b/crates/vnidrop/CORE_FLOW.md index 96cf5bd..0d24374 100644 --- a/crates/vnidrop/CORE_FLOW.md +++ b/crates/vnidrop/CORE_FLOW.md @@ -59,9 +59,16 @@ bytes through Kotlin memory. - Persisted shares are restored only when their root collection is complete and readable. Missing or corrupt roots fail closed and emit a recovery event. - Receive destinations use a no-overwrite policy. Rust writes a uniquely named - temporary file in the destination directory, syncs it, and atomically - publishes it with a no-clobber hard link. Failure or cancellation removes the - temporary file. Stale VniDrop temporary files are cleaned on later writes. + temporary file in the destination directory, syncs it, and publishes it with + a no-clobber hard link when the filesystem supports it. On platforms that + reject hard links (notably Android emulated external storage), publication + falls back to an exclusive rename (`renameat2(RENAME_NOREPLACE)` / + `renamex_np(RENAME_EXCL)`). Failure or cancellation removes the temporary + file. Stale VniDrop temporary files are cleaned on later writes. +- Android defaults to the app-specific external Downloads directory + (`getExternalFilesDir`), which is always writable by the process. Shared + system folders require a SAF tree URI via the folder picker; those receives + stream through `ReceiveOutputSink` instead of raw filesystem paths. - Foreign output sinks receive exactly one terminal callback after a successful `start_file`: `finish_file` or `abort_file`. diff --git a/crates/vnidrop/src/filesystem.rs b/crates/vnidrop/src/filesystem.rs index 44d0056..0d3c9a6 100644 --- a/crates/vnidrop/src/filesystem.rs +++ b/crates/vnidrop/src/filesystem.rs @@ -101,18 +101,134 @@ impl AtomicOutputFile { } pub(crate) fn commit(mut self) -> Result<()> { - // Creating a hard link is an atomic no-clobber publication on the same - // filesystem. It fails if another writer created the destination. - std::fs::hard_link(&self.temporary, &self.target) + publish_temp_as_final(&self.temporary, &self.target) .with_context(|| format!("failed to commit {}", self.target.display()))?; self.committed = true; - if let Err(error) = std::fs::remove_file(&self.temporary) { - tracing::warn!(%error, path = %self.temporary.display(), "failed to remove committed temporary file"); - } Ok(()) } } +/// Publish a fully written temporary file as the final destination without +/// clobbering an existing peer. +/// +/// Prefer a same-directory hard link (atomic no-clobber on most Unix volumes). +/// Android's emulated external storage and some FUSE mounts reject hard links +/// even when ordinary create/write/rename work, so fall back to an exclusive +/// rename when the link is unsupported. +fn publish_temp_as_final(temporary: &Path, target: &Path) -> io::Result<()> { + match std::fs::hard_link(temporary, target) { + Ok(()) => { + if let Err(error) = std::fs::remove_file(temporary) { + tracing::warn!( + %error, + path = %temporary.display(), + "failed to remove committed temporary file" + ); + } + Ok(()) + } + Err(error) if is_hard_link_unsupported(&error) => rename_no_replace(temporary, target), + Err(error) => Err(error), + } +} + +fn is_hard_link_unsupported(error: &io::Error) -> bool { + match error.raw_os_error() { + Some(code) + if code == libc::EPERM + || code == libc::EACCES + || code == libc::EOPNOTSUPP + || code == libc::ENOTSUP + || code == libc::EXDEV + || code == libc::EINVAL + || code == libc::ENOSYS => + { + true + } + _ => matches!( + error.kind(), + io::ErrorKind::Unsupported | io::ErrorKind::PermissionDenied + ), + } +} + +fn rename_no_replace(from: &Path, to: &Path) -> io::Result<()> { + #[cfg(any(target_os = "linux", target_os = "android"))] + { + match renameat2_noreplace(from, to) { + Ok(()) => return Ok(()), + Err(error) + if error.raw_os_error() == Some(libc::ENOSYS) + || error.raw_os_error() == Some(libc::EINVAL) => {} + Err(error) => return Err(error), + } + } + + #[cfg(target_os = "macos")] + { + match renamex_np_excl(from, to) { + Ok(()) => return Ok(()), + Err(error) if error.raw_os_error() == Some(libc::ENOTSUP) => {} + Err(error) => return Err(error), + } + } + + // Last resort: refuse an existing destination, then rename. There is a + // small race versus concurrent writers, but this path only runs when the + // platform lacks both hard links and exclusive rename. + if std::fs::symlink_metadata(to).is_ok() { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + format!("destination already exists: {}", to.display()), + )); + } + std::fs::rename(from, to) +} + +#[cfg(any(target_os = "linux", target_os = "android"))] +fn renameat2_noreplace(from: &Path, to: &Path) -> io::Result<()> { + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt; + + let from_c = CString::new(from.as_os_str().as_bytes()) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + let to_c = CString::new(to.as_os_str().as_bytes()) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + // Android defines RENAME_NOREPLACE as c_int while renameat2 takes c_uint. + let flags = libc::RENAME_NOREPLACE as libc::c_uint; + let rc = unsafe { + libc::renameat2( + libc::AT_FDCWD, + from_c.as_ptr(), + libc::AT_FDCWD, + to_c.as_ptr(), + flags, + ) + }; + if rc == 0 { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } +} + +#[cfg(target_os = "macos")] +fn renamex_np_excl(from: &Path, to: &Path) -> io::Result<()> { + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt; + + let from_c = CString::new(from.as_os_str().as_bytes()) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + let to_c = CString::new(to.as_os_str().as_bytes()) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + let rc = unsafe { libc::renamex_np(from_c.as_ptr(), to_c.as_ptr(), libc::RENAME_EXCL) }; + if rc == 0 { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } +} + pub(crate) fn cleanup_stale_temporary_files( directory: &Path, minimum_age: Duration, diff --git a/crates/vnidrop/src/tests/filesystem.rs b/crates/vnidrop/src/tests/filesystem.rs index 38c9937..93f4b5c 100644 --- a/crates/vnidrop/src/tests/filesystem.rs +++ b/crates/vnidrop/src/tests/filesystem.rs @@ -117,6 +117,25 @@ fn atomic_output_commits_without_overwriting() { ); } +#[test] +fn atomic_output_commit_does_not_clobber_existing_peer() { + let output = tempfile::tempdir().unwrap(); + let target = output.path().join("peer.txt"); + std::fs::write(&target, b"original").unwrap(); + + let (pending, mut file) = AtomicOutputFile::create(output.path(), "other.txt").unwrap(); + std::io::Write::write_all(&mut file, b"new").unwrap(); + drop(file); + pending.commit().unwrap(); + + // Existing peer must stay intact while a different file commits. + assert_eq!(std::fs::read(&target).unwrap(), b"original"); + assert_eq!( + std::fs::read(output.path().join("other.txt")).unwrap(), + b"new" + ); +} + #[test] fn dropped_atomic_output_removes_partial_file() { let output = tempfile::tempdir().unwrap(); diff --git a/shared/src/androidMain/kotlin/com/vnidrop/app/core/FileSystemService.android.kt b/shared/src/androidMain/kotlin/com/vnidrop/app/core/FileSystemService.android.kt index 5f3d8ee..5818c7a 100644 --- a/shared/src/androidMain/kotlin/com/vnidrop/app/core/FileSystemService.android.kt +++ b/shared/src/androidMain/kotlin/com/vnidrop/app/core/FileSystemService.android.kt @@ -22,14 +22,17 @@ private class AndroidFileSystemService( private val context: Context, ) : FileSystemService { override fun defaultReceiveFolder(): ReceiveFolder { + // App-specific external storage is always writable without SAF or + // legacy storage permissions. It is NOT the shared system Downloads + // gallery — that still requires "Choose folder" (tree URI). val path = context .getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS) ?.absolutePath - ?: (System.getProperty("java.io.tmpdir") ?: "/data/local/tmp/vnidrop-receive") + ?: context.filesDir.resolve("Downloads").apply { mkdirs() }.absolutePath return ReceiveFolder( kind = ReceiveFolderKind.FileSystemPath, value = path, - displayName = "Downloads", + displayName = "App downloads", ) } @@ -64,11 +67,27 @@ private class AndroidFileSystemService( } } + /** + * Probe a real create/write/delete instead of [java.io.File.canWrite]. + * + * Scoped storage often reports public directories as writable even when + * the process cannot create files there. A probe matches what receive needs. + */ private fun validatePath(path: String): FolderAccessStatus = runCatching { val directory = java.io.File(path) - if (!directory.exists()) directory.mkdirs() - if (directory.isDirectory && directory.canWrite()) FolderAccessStatus.Writable else FolderAccessStatus.Unavailable + if (!directory.exists() && !directory.mkdirs()) { + return FolderAccessStatus.Unavailable + } + if (!directory.isDirectory) return FolderAccessStatus.Unavailable + val probe = java.io.File(directory, ".vnidrop-write-test-${UUID.randomUUID()}") + try { + probe.outputStream().use { stream -> stream.write(1) } + if (!probe.exists()) return FolderAccessStatus.Unavailable + FolderAccessStatus.Writable + } finally { + probe.delete() + } }.getOrDefault(FolderAccessStatus.Unavailable) private fun validateTreeUri(value: String): FolderAccessStatus {