fix(android): make default receive path actually writable

Android emulated storage rejects hard-link commit even when canWrite()
reports success. Fall back to exclusive rename for publish, probe real
writes during folder validation, and label the default app downloads dir
clearly so it is not confused with shared system Downloads.
This commit is contained in:
2026-07-12 04:40:10 +02:00
parent 4050a7c011
commit d5d4e37d1c
4 changed files with 174 additions and 13 deletions

View File

@@ -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`.

View File

@@ -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,

View File

@@ -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();

View File

@@ -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 {