Problem: The sender records a file for removal whenever the send returns success (rc==0). But --existing, --ignore-existing, and --update are enforced server-side at write time and simply return true (skip). The client never learns the file was skipped, so after the final handshake it unlinks the source file anyway.
Impact: Silent data loss. Verified: client --remove-source-files --existing src dest where a file exists only in src -> file exists in neither src nor dest. Also reproduces with --ignore-existing and --update. Reference rsync leaves the file in src.
Fix: Only remove files the receiver actually wrote: server must report a per-file skipped/transferred outcome and the sender must not record skipped files for removal.
Regression test required (integration).
Found by code review of `dev` (9f8b588).
**Where:** src/client/client_send.c:133-166 `remove_transferred_sources`, :517-530, :870-871; skip paths src/shared/file_receive.c:73-103.
**Problem:** The sender records a file for removal whenever the *send* returns success (rc==0). But `--existing`, `--ignore-existing`, and `--update` are enforced server-side at write time and simply `return true` (skip). The client never learns the file was skipped, so after the final handshake it unlinks the source file anyway.
**Impact:** Silent data loss. Verified: `client --remove-source-files --existing src dest` where a file exists only in `src` -> file exists in *neither* src nor dest. Also reproduces with `--ignore-existing` and `--update`. Reference rsync leaves the file in src.
**Fix:** Only remove files the receiver actually wrote: server must report a per-file skipped/transferred outcome and the sender must not record skipped files for removal.
Regression test required (integration).
Fixed on dev via merge of fix/receiver-correctness (14c064a): server now reports per-file outcomes after STATUS_FINISHED; --remove-source-files only deletes sources the receiver actually wrote. Integration tests added (TestRemoveSourceFilesSkips + -m variant).
CI run #461: all jobs green (lint, build-and-test, sanitizers address+undefined, fuzz-build, coverage, valgrind). Unit 25/25, integration 83 passed / 10 skipped / 1 xpassed. Closing.
**Resolved on `dev`** (HEAD f7c6c91).
Fixed on dev via merge of fix/receiver-correctness (14c064a): server now reports per-file outcomes after STATUS_FINISHED; --remove-source-files only deletes sources the receiver actually wrote. Integration tests added (TestRemoveSourceFilesSkips + -m variant).
CI run #461: all jobs green (lint, build-and-test, sanitizers address+undefined, fuzz-build, coverage, valgrind). Unit 25/25, integration 83 passed / 10 skipped / 1 xpassed. Closing.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found by code review of
dev(9f8b588).Where: src/client/client_send.c:133-166
remove_transferred_sources, :517-530, :870-871; skip paths src/shared/file_receive.c:73-103.Problem: The sender records a file for removal whenever the send returns success (rc==0). But
--existing,--ignore-existing, and--updateare enforced server-side at write time and simplyreturn true(skip). The client never learns the file was skipped, so after the final handshake it unlinks the source file anyway.Impact: Silent data loss. Verified:
client --remove-source-files --existing src destwhere a file exists only insrc-> file exists in neither src nor dest. Also reproduces with--ignore-existingand--update. Reference rsync leaves the file in src.Fix: Only remove files the receiver actually wrote: server must report a per-file skipped/transferred outcome and the sender must not record skipped files for removal.
Regression test required (integration).
Resolved on
dev(HEADf7c6c91).Fixed on dev via merge of fix/receiver-correctness (
14c064a): server now reports per-file outcomes after STATUS_FINISHED; --remove-source-files only deletes sources the receiver actually wrote. Integration tests added (TestRemoveSourceFilesSkips + -m variant).CI run #461: all jobs green (lint, build-and-test, sanitizers address+undefined, fuzz-build, coverage, valgrind). Unit 25/25, integration 83 passed / 10 skipped / 1 xpassed. Closing.