From 04fcddeff03e8b0b1c06281c7bb7f7bb74402562 Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Sat, 19 Sep 2020 11:47:57 -0700 Subject: [PATCH] webdav: prevent webdav client race condition (#626) The race is described in https://github.com/studio-b12/gowebdav/issues/36 We need this workaround until the fix is merged upstream, to avoid maintaining a webdav client fork. Fixes #624 --- repo/blob/rclone/rclone_storage_test.go | 23 +++++++++++++++++++++++ repo/blob/webdav/webdav_storage.go | 14 ++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/repo/blob/rclone/rclone_storage_test.go b/repo/blob/rclone/rclone_storage_test.go index 242d37bcb..f0d316f84 100644 --- a/repo/blob/rclone/rclone_storage_test.go +++ b/repo/blob/rclone/rclone_storage_test.go @@ -6,8 +6,13 @@ "os/exec" "testing" + "github.com/google/uuid" + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" + "github.com/kopia/kopia/internal/blobtesting" "github.com/kopia/kopia/internal/testlogging" + "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/blob/rclone" ) @@ -36,6 +41,24 @@ func TestRCloneStorage(t *testing.T) { defer st.Close(ctx) + var eg errgroup.Group + + // trigger multiple parallel reads to ensure we're properly preventing race + // described in https://github.com/kopia/kopia/issues/624 + for i := 0; i < 100; i++ { + eg.Go(func() error { + if _, err := st.GetBlob(ctx, blob.ID(uuid.New().String()), 0, -1); !errors.Is(err, blob.ErrBlobNotFound) { + return errors.Errorf("unexpected error when downloading non-existent blob: %v", err) + } + + return nil + }) + } + + if err := eg.Wait(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + blobtesting.VerifyStorage(ctx, t, st) blobtesting.AssertConnectionInfoRoundTrips(ctx, t, st) } diff --git a/repo/blob/webdav/webdav_storage.go b/repo/blob/webdav/webdav_storage.go index 17c0f581b..57d7f20fd 100644 --- a/repo/blob/webdav/webdav_storage.go +++ b/repo/blob/webdav/webdav_storage.go @@ -12,6 +12,7 @@ "strings" "time" + "github.com/google/uuid" "github.com/pkg/errors" "github.com/studio-b12/gowebdav" @@ -201,7 +202,7 @@ func New(ctx context.Context, opts *Options) (blob.Storage, error) { cli.SetTransport(tlsutil.TransportTrustingSingleCertificate(opts.TrustedServerCertificateFingerprint)) } - return &davStorage{ + s := &davStorage{ sharded.Storage{ Impl: &davStorageImpl{ Options: *opts, @@ -211,7 +212,16 @@ func New(ctx context.Context, opts *Options) (blob.Storage, error) { Suffix: fsStorageChunkSuffix, Shards: opts.shards(), }, - }, nil + } + + // temporary workaround to a race condition problem in https://github.com/studio-b12/gowebdav/issues/36 + // the race condition is only during first request to the server, so to fix it we force the first request + // to read a non-existent blob, which sets the authentication method. + if _, err := s.GetBlob(ctx, blob.ID(uuid.New().String()), 0, -1); !errors.Is(err, blob.ErrBlobNotFound) { + return nil, errors.Errorf("unexpected error when initializing webdav: %v", err) + } + + return s, nil } func init() {