diff --git a/changelog/unreleased/pull-3449 b/changelog/unreleased/pull-3449 new file mode 100644 index 000000000..a05094f26 --- /dev/null +++ b/changelog/unreleased/pull-3449 @@ -0,0 +1,8 @@ +Bugfix: Correctly handle download errors during `restore` + +Due to a regression in restic 0.12.0 the `restore` command in some cases +did not retry download errors and only printed a warning. This has been +fixed by retrying incomplete data downloads. + +https://github.com/restic/restic/issues/3439 +https://github.com/restic/restic/pull/3449 diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index d2f71ce8f..2e503c4aa 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -258,7 +258,11 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { if err != nil { return err } - blobData, buf, err = r.loadBlob(bufRd, blobID, blob.length, buf) + buf, err = r.downloadBlob(bufRd, blobID, blob.length, buf) + if err != nil { + return err + } + blobData, err = r.decryptBlob(blobID, buf) if err != nil { for file := range blob.files { if errFile := sanitizeError(file, err); errFile != nil { @@ -309,7 +313,7 @@ func (r *fileRestorer) downloadPack(ctx context.Context, pack *packInfo) error { return nil } -func (r *fileRestorer) loadBlob(rd io.Reader, blobID restic.ID, length int, buf []byte) ([]byte, []byte, error) { +func (r *fileRestorer) downloadBlob(rd io.Reader, blobID restic.ID, length int, buf []byte) ([]byte, error) { // TODO reconcile with Repository#loadBlob implementation if cap(buf) < length { @@ -320,24 +324,29 @@ func (r *fileRestorer) loadBlob(rd io.Reader, blobID restic.ID, length int, buf n, err := io.ReadFull(rd, buf) if err != nil { - return nil, nil, err + return nil, err } if n != length { - return nil, nil, errors.Errorf("error loading blob %v: wrong length returned, want %d, got %d", blobID.Str(), length, n) + return nil, errors.Errorf("error loading blob %v: wrong length returned, want %d, got %d", blobID.Str(), length, n) } + return buf, nil +} + +func (r *fileRestorer) decryptBlob(blobID restic.ID, buf []byte) ([]byte, error) { + // TODO reconcile with Repository#loadBlob implementation // decrypt nonce, ciphertext := buf[:r.key.NonceSize()], buf[r.key.NonceSize():] plaintext, err := r.key.Open(ciphertext[:0], nonce, ciphertext, nil) if err != nil { - return nil, nil, errors.Errorf("decrypting blob %v failed: %v", blobID, err) + return nil, errors.Errorf("decrypting blob %v failed: %v", blobID, err) } // check hash if !restic.Hash(plaintext).Equal(blobID) { - return nil, nil, errors.Errorf("blob %v returned invalid hash", blobID) + return nil, errors.Errorf("blob %v returned invalid hash", blobID) } - return plaintext, buf, nil + return plaintext, nil } diff --git a/internal/restorer/filerestorer_test.go b/internal/restorer/filerestorer_test.go index ac8e371da..333420b70 100644 --- a/internal/restorer/filerestorer_test.go +++ b/internal/restorer/filerestorer_test.go @@ -3,6 +3,7 @@ package restorer import ( "bytes" "context" + "fmt" "io" "io/ioutil" "testing" @@ -163,6 +164,10 @@ func restoreAndVerify(t *testing.T, tempdir string, content []TestFile, files ma err := r.restoreFiles(context.TODO()) rtest.OK(t, err) + verifyRestore(t, r, repo) +} + +func verifyRestore(t *testing.T, r *fileRestorer, repo *TestRepo) { for _, file := range r.files { target := r.targetPath(file.location) data, err := ioutil.ReadFile(target) @@ -264,3 +269,49 @@ func TestErrorRestoreFiles(t *testing.T) { err := r.restoreFiles(context.TODO()) rtest.Equals(t, loadError, err) } + +func TestDownloadError(t *testing.T) { + for i := 0; i < 100; i += 10 { + testPartialDownloadError(t, i) + } +} + +func testPartialDownloadError(t *testing.T, part int) { + tempdir, cleanup := rtest.TempDir(t) + defer cleanup() + content := []TestFile{ + { + name: "file1", + blobs: []TestBlob{ + {"data1-1", "pack1"}, + {"data1-2", "pack1"}, + {"data1-3", "pack1"}, + }, + }} + + repo := newTestRepo(content) + + // loader always returns an error + loader := repo.loader + repo.loader = func(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error { + // only load partial data to execise fault handling in different places + err := loader(ctx, h, length*part/100, offset, fn) + if err == nil { + return nil + } + fmt.Println("Retry after error", err) + return loader(ctx, h, length, offset, fn) + } + + r := newFileRestorer(tempdir, repo.loader, repo.key, repo.Lookup) + r.files = repo.files + r.Error = func(s string, e error) error { + // ignore errors as in the `restore` command + fmt.Println("error during restore", s, e) + return nil + } + + err := r.restoreFiles(context.TODO()) + rtest.OK(t, err) + verifyRestore(t, r, repo) +}