DEV Community

chovy
chovy

Posted on Originally published at dev.profullstack.com

Two movies, forty thousand files

Someone selected two folders on a server in DiskPush, pressed Sync to Local, and watched it start copying forty thousand files. The selection was real. It was on screen, it was highlighted, the pane footer said "2 selected". It just never reached the transfer.

The request was built from the directory path and nothing else:

source: refFor(source, withTrailingSlash(source.path)),
Enter fullscreen mode Exit fullscreen mode

state.selected existed, it was populated, and no code path outside the pane component ever read it. Every transfer had always been the whole folder. Nobody noticed because until you select something, the whole folder is the right answer.

The one underneath it

While fixing that I found a worse bug in the same click. The direction buttons do this:

onDirection('rtl')
onRun()
Enter fullscreen mode Exit fullscreen mode

setDirection does not change the value the current render already closed over. So onRun built its request from the previous direction. Pressing the arrow that was not already armed transferred the opposite way. "Sync to Local" copied local over the server.

With Mirror armed, that deletes on the wrong side. It had been there since the two buttons were added.

The fix is to pass the direction into the run instead of reading it back out of state. State you just set is not state you can read.

The rsync part, which nearly shipped broken

The obvious way to send a selection to rsync is --files-from, a list of names relative to the source. I wrote that, then checked it against the actual binary instead of the flag names:

rsync -a    --files-from=list src/ dst/   ->  cd+++++++++ movieA/
rsync -a -r --files-from=list src/ dst/   ->  cd+++++++++ movieA/
                                              >f+++++++++ movieA/a.mkv
Enter fullscreen mode Exit fullscreen mode

--files-from turns rsync's recursion off, and --archive does not turn it back on. Without an explicit -r, selecting a folder copies an empty folder and exits zero. It would have looked like the bug was fixed, and the two movies would have arrived as two empty directories.

That is a worse failure than the one I was fixing. A tool that copies nothing and says it worked is not a tool anyone can use.

The list is also NUL separated rather than one name per line, because a newline is a legal character in a filename and a line separated list quietly turns one such name into two paths that do not exist.

One good surprise: mirror plus a selection deletes only inside what you selected. I checked rather than assumed, because the other plausible reading of --delete there is "remove everything at the destination that is not in this list", which would empty the folder.

Everything asks now

Both the desktop and the CLI skipped the dry run for a plain sync. The comment in the code said a preview "buys no safety, because nothing is deleted either way".

Forty thousand files is the counterexample. Deleting is not the only way to regret a transfer, and by the time anything appeared on screen it was already copying.

So every manual transfer previews and waits for a yes now. The dialog says whether it is a Sync or a Mirror, and it carries the scope next to the counts: "2 selected items" or "Whole folder". A count on its own tells you how many files. It never tells you how many of what you asked for, which is the number that was wrong here.

The CLI gets --only NAME, repeatable. It only prompts when there is somebody there to answer, so scheduled runs and scripts behave as before. The terminal UI had a cursor and no way to select at all, so it gets marking with space.

Two smaller things fell out. --only did not parse at first: it was missing from the list of flags that take a value, so it silently swallowed its own argument and did nothing. I found that by running the built binary rather than reading the code. And the preview dialog had a "Trust this pair from now on" checkbox that set a piece of state nothing read, and could not have worked if it were wired, because the code that saves a profile hard codes that setting to false on purpose.

DiskPush 0.2.17: https://diskpush.com/download

This post was drafted with AI assistance.

Top comments (0)