feat: reopen file instead of deleting and creating

packages/core/src/http/server/common/save.rs

@@ -242,6 +242,9 @@ fn spawn_file_writer(
 ///
 /// Fails if the total number of written bytes does not match `expected_size`
 /// (e.g. the sender disconnected mid-transfer).
+///
+/// The file is truncated to the written size, so that a target that pointed at
+/// a longer, pre-existing file cannot keep a tail of the old content.
 async fn write_file_from_receiver(
     open: impl Future<Output = Result<tokio::fs::File, String>>,
     expected_size: u64,
@@ -276,5 +279,18 @@ async fn write_file_from_receiver(
             "Expected {expected_size} bytes, received {written}"
         ));
     }
+
+    // Drops content beyond the file that was just written, in case the target
+    // pointed at a longer, pre-existing file: opening truncates for paths and
+    // for descriptors opened with the SAF "wt" mode, but a document provider is
+    // free to ignore that mode. Retries of the same upload are covered by the
+    // exact size check above either way.
+    //
+    // Best-effort: a provider may back the descriptor by something that cannot
+    // be truncated (e.g. a pipe), which must not fail the completed transfer.
+    if let Err(e) = file.set_len(written).await {
+        tracing::warn!("Could not truncate file to {written} bytes: {e}");
+    }
+
     Ok(())
 }

packages/core/tests/v2_server.rs

@@ -498,6 +498,70 @@ async fn test_upload_retry_after_mismatched_sha256() {
     assert!(matches!(session_ends[0].1, SessionEndReasonV2::Finished));
 }
 
+/// A retry writes to the same path as the failed attempt, so the receiver ends
+/// up with exactly one file holding the correct content.
+#[tokio::test]
+async fn test_upload_retry_reuses_the_same_path() {
+    let save_dir = std::env::temp_dir().join(format!("localsend-test-{}", uuid::Uuid::new_v4()));
+    tokio::fs::create_dir_all(&save_dir).await.unwrap();
+
+    let server = start_test_server(None, true, Some(save_dir.clone())).await;
+    let client = LsHttpClientV2::try_new_without_cert().unwrap();
+
+    let bytes: Vec<u8> = (0..50_000u32).map(|i| i as u8).collect();
+    let mut corrupted = bytes.clone();
+    *corrupted.last_mut().unwrap() ^= 0xff;
+    let mut file = file_dto("file-a", "a.bin", bytes.len() as u64);
+    file.sha256 = Some(sha256_hex(&bytes));
+
+    let response = client
+        .prepare_upload(
+            ProtocolType::Http,
+            "127.0.0.1",
+            server.port,
+            None,
+            prepare_upload_request(&[file]),
+            None,
+        )
+        .await
+        .unwrap()
+        .response
+        .unwrap();
+
+    let result = upload_bytes(
+        &client,
+        server.port,
+        &response.session_id,
+        "file-a",
+        &response.files["file-a"],
+        &corrupted,
+    )
+    .await;
+    assert_status(result, 422);
+
+    upload_bytes(
+        &client,
+        server.port,
+        &response.session_id,
+        "file-a",
+        &response.files["file-a"],
+        &bytes,
+    )
+    .await
+    .unwrap();
+
+    // The corrupted attempt has been overwritten, not kept as a second file.
+    let mut entries = tokio::fs::read_dir(&save_dir).await.unwrap();
+    let mut file_names = Vec::new();
+    while let Some(entry) = entries.next_entry().await.unwrap() {
+        file_names.push(entry.file_name().to_string_lossy().to_string());
+    }
+    assert_eq!(file_names, vec!["file-a".to_string()]);
+    assert_eq!(tokio::fs::read(save_dir.join("file-a")).await.unwrap(), bytes);
+
+    tokio::fs::remove_dir_all(&save_dir).await.unwrap();
+}
+
 #[tokio::test]
 async fn test_upload_mismatched_sha256_attempts_exhausted() {
     let server = start_test_server(None, true, None).await;

packages/localsend_isolates/lib/src/isolate/child/server_isolate.dart

@@ -340,9 +340,15 @@ class _ReceiveSession {
   /// One queue per file ID, so that uploads of the same file do not overlap.
   ///
   /// A sender may upload the same file again after it was rejected because of
-  /// a checksum mismatch.
+  /// a checksum mismatch. Both attempts write to the same [targets] entry.
   final Map<String, FutureQueue> uploads = {};
 
+  /// The destination of each file of this session, by file ID.
+  ///
+  /// Remembered so that another attempt at the same file overwrites it instead
+  /// of being saved next to it under a numbered name.
+  final Map<String, FileSaveTarget> targets = {};
+
   _ReceiveSession(this.config);
 }
 
@@ -618,15 +624,21 @@ Future<void> _handleFileUpload({
 
   final FileSaveTarget target;
   try {
-    target = await prepareFileSaveTarget(
-      destinationDirectory: config.destinationDirectory,
-      cacheDirectory: config.cacheDirectory,
-      fileName: desiredName,
-      saveToGallery: shouldSaveToGallery,
-      isImage: isImage,
-      createdDirectories: session.createdDirectories,
-      androidSdkInt: config.androidSdkInt,
-    );
+    // A previous attempt at this file already picked a destination, which this
+    // attempt overwrites instead of creating a numbered version.
+    final previous = session.targets[fileId];
+    target = previous != null
+        ? await reopenFileSaveTarget(previous)
+        : await prepareFileSaveTarget(
+            destinationDirectory: config.destinationDirectory,
+            cacheDirectory: config.cacheDirectory,
+            fileName: desiredName,
+            saveToGallery: shouldSaveToGallery,
+            isImage: isImage,
+            createdDirectories: session.createdDirectories,
+            androidSdkInt: config.androidSdkInt,
+          );
+    session.targets[fileId] = target;
   } catch (e, st) {
     _logger.severe('Failed to prepare save target', e, st);
 
@@ -663,18 +675,13 @@ Future<void> _handleFileUpload({
       );
     }
   } catch (e, st) {
+    // The incomplete file is kept: a retry of this file overwrites it, and
+    // otherwise it stays behind as the partial file of a failed transfer.
     _logger.severe('Failed to save file', e, st);
-
-    // Delete the partial (or checksum-mismatched) file so a retried upload
-    // gets the same file name again instead of a renamed one.
-    await deleteFileSaveTarget(target);
-
     emitFailed(e);
     return;
   }
 
-  // The file is fully received and the sender was already told success,
-  // so failures from here on must not delete the file.
   try {
     await applyFileTimestamps(
       target: target,

packages/localsend_isolates/lib/src/task/server/file_saver.dart

@@ -93,30 +93,26 @@ Future<FileSaveTarget> prepareFileSaveTarget({
   );
 }
 
-/// Deletes the file behind [target].
+/// Prepares [target] for another attempt at the same file, e.g. after the
+/// previous attempt was rejected because of a checksum mismatch.
 ///
-/// Used when a file could not be received completely, so that a retry of the
-/// same file gets the original file name again instead of a numbered one.
-///
-/// Failures are logged and swallowed: a leftover file is not worth failing
-/// the transfer for.
-Future<void> deleteFileSaveTarget(FileSaveTarget target) async {
-  // SAF targets are written through a file descriptor and must be deleted
-  // through the Storage Access Framework, addressed by their document URI.
-  final uri = target.path ?? target.displayPath;
-  try {
-    if (uri.startsWith('content://')) {
-      await android_channel.deleteFileAndroid(uri: uri);
-      return;
-    }
-
-    final file = File(uri);
-    if (await file.exists()) {
-      await file.delete();
-    }
-  } catch (e) {
-    _logger.warning('Could not delete file at $uri', e);
+/// The destination is kept, so the file is overwritten instead of being
+/// created a second time under a numbered name.
+Future<FileSaveTarget> reopenFileSaveTarget(FileSaveTarget target) async {
+  final path = target.path;
+  if (path != null) {
+    // The server opens (and truncates) the path itself.
+    return target;
   }
+
+  // The descriptor of the previous attempt was consumed by it, so the SAF
+  // document has to be opened again.
+  _logger.info('Reopening ${target.displayPath}');
+  return FileSaveTarget(
+    path: null,
+    fileDescriptor: await android_channel.openFileForWritingAndroid(uri: target.displayPath),
+    displayPath: target.displayPath,
+  );
 }
 
 /// Applies the file timestamps after the file has been written to a plain path.

packages/localsend_isolates/lib/util/android_channel.dart

@@ -85,13 +85,17 @@ Future<CreatedFileAndroid> createFileAndroid({
   );
 }
 
-/// Deletes the document at [uri], e.g. a file created by [createFileAndroid].
+/// Opens an existing document created by [createFileAndroid] for writing and
+/// discards its current content.
 ///
-/// Returns whether the document has been deleted. A document that does not
-/// exist (anymore) returns `false` instead of throwing.
-Future<bool> deleteFileAndroid({required String uri}) async {
-  final deleted = await _methodChannel.invokeMethod<bool>('deleteFile', {
+/// The descriptor stays open after this call and must be closed by the native
+/// consumer it is passed to.
+Future<int> openFileForWritingAndroid({required String uri}) async {
+  final fileDescriptor = await _methodChannel.invokeMethod<int>('openFileForWriting', {
     'uri': uri,
   });
-  return deleted ?? false;
+  if (fileDescriptor == null) {
+    throw StateError('Android returned no file descriptor for $uri');
+  }
+  return fileDescriptor;
 }