Skip to content

fix(restore): return errors from restoring file contents instead of panicking - #603

Open
AlphaJack wants to merge 1 commit into
rustic-rs:mainfrom
AlphaJack:fix/restore-no-panic
Open

AlphaJack wants to merge 1 commit into
rustic-rs:mainfrom
AlphaJack:fix/restore-no-panic

Conversation

@AlphaJack

Copy link
Copy Markdown

Fixes #593

Problem

restore_contents unwraps every fallible call in its worker tasks: reading the pack, decrypting a blob, reading from an existing file, setting a file's length, writing data. Any failure panics a worker. With panic = "abort" in rustic's release profile, the whole process aborts (exit code 134) with a panic message instead of an error, and the metadata of the restored files isn't set.

Two example ways to get there with rustic 0.11.4:

  • a read-only file in the destination: EACCES panics at restore.rs:658 (set_length)
  • a pack whose bytes changed but kept their length: the MAC check fails and panics at restore.rs:643 (read_encrypted_from_partial)

Fix

The tasks record the first error in a shared slot. Once it is set, the remaining tasks skip their work, and restore_contents returns the error after the thread pool has finished. Errors of the destination become ErrorKind::InputOutput with the file's path as context, like the existing error for setting the length of empty files a few lines above. The // TODO: error handling! comment is gone.

The restore now stops at the first error: tasks that are already running finish, the others do nothing.
If you'd rather continue with the other files and report all errors at the end, that's a small change to how the slot is used.

sizes.lock().unwrap() stays: the mutex can only be poisoned by a panic, and there's none left.

Tests

  • test_restore_damaged_pack_returns_error: flips a byte in a file's data blob, so the pack keeps its length) and restores.
  • test_restore_over_read_only_file_returns_error (Unix only): restores over a read-only file in a read-only directory. Skipped when running as root, where mode bits aren't enforced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

restore panics instead of returning an error when a file cannot be written or a pack read fails

1 participant