From 822422ef03880db40b6fbd3e24d59e763537ec87 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 19 Sep 2021 19:58:47 +0200 Subject: [PATCH] retry key loading on hash mismatch --- internal/backend/utils.go | 15 ++++++++++++ internal/backend/utils_test.go | 42 ++++++++++++++++++++++++++++++++++ internal/cache/backend_test.go | 1 - 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/internal/backend/utils.go b/internal/backend/utils.go index be1c2a9e0..d2ac44670 100644 --- a/internal/backend/utils.go +++ b/internal/backend/utils.go @@ -6,6 +6,8 @@ import ( "fmt" "io" + "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" ) @@ -13,6 +15,7 @@ import ( // buffer, which is truncated. If the buffer is not large enough or nil, a new // one is allocated. func LoadAll(ctx context.Context, buf []byte, be restic.Backend, h restic.Handle) ([]byte, error) { + retriedInvalidData := false err := be.Load(ctx, h, 0, 0, func(rd io.Reader) error { // make sure this is idempotent, in case an error occurs this function may be called multiple times! wr := bytes.NewBuffer(buf[:0]) @@ -21,6 +24,18 @@ func LoadAll(ctx context.Context, buf []byte, be restic.Backend, h restic.Handle return cerr } buf = wr.Bytes() + + // retry loading damaged data only once. If a file fails to download correctly + // the second time, then it is likely corrupted at the backend. Return the data + // to the caller in that case to let it decide what to do with the data. + if !retriedInvalidData && h.Type != restic.ConfigFile { + id, err := restic.ParseID(h.Name) + if err == nil && !restic.Hash(buf).Equal(id) { + debug.Log("retry loading broken blob %v", h) + retriedInvalidData = true + return errors.Errorf("loadAll(%v): invalid data returned", h) + } + } return nil }) diff --git a/internal/backend/utils_test.go b/internal/backend/utils_test.go index 2e77fa9bd..3fb570eba 100644 --- a/internal/backend/utils_test.go +++ b/internal/backend/utils_test.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io" + "io/ioutil" "math/rand" "testing" @@ -56,6 +57,47 @@ func save(t testing.TB, be restic.Backend, buf []byte) restic.Handle { return h } +type quickRetryBackend struct { + restic.Backend +} + +func (be *quickRetryBackend) Load(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error { + err := be.Backend.Load(ctx, h, length, offset, fn) + if err != nil { + // retry + err = be.Backend.Load(ctx, h, length, offset, fn) + } + return err +} + +func TestLoadAllBroken(t *testing.T) { + b := mock.NewBackend() + + data := rtest.Random(23, rand.Intn(MiB)+500*KiB) + id := restic.Hash(data) + // damage buffer + data[0] ^= 0xff + + b.OpenReaderFn = func(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewReader(data)), nil + } + + // must fail on first try + _, err := backend.LoadAll(context.TODO(), nil, b, restic.Handle{Type: restic.PackFile, Name: id.String()}) + if err == nil { + t.Fatalf("missing expected error") + } + + // must return the broken data after a retry + be := &quickRetryBackend{Backend: b} + buf, err := backend.LoadAll(context.TODO(), nil, be, restic.Handle{Type: restic.PackFile, Name: id.String()}) + rtest.OK(t, err) + + if !bytes.Equal(buf, data) { + t.Fatalf("wrong data returned") + } +} + func TestLoadAllAppend(t *testing.T) { b := mem.New() diff --git a/internal/cache/backend_test.go b/internal/cache/backend_test.go index 79b838eb2..e9f0c4d2b 100644 --- a/internal/cache/backend_test.go +++ b/internal/cache/backend_test.go @@ -48,7 +48,6 @@ func remove(t testing.TB, be restic.Backend, h restic.Handle) { func randomData(n int) (restic.Handle, []byte) { data := test.Random(rand.Int(), n) id := restic.Hash(data) - copy(id[:], data) h := restic.Handle{ Type: restic.IndexFile, Name: id.String(),