From 24c4944b58a070692ca402336422cc22ba5fce74 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 15 Feb 2022 16:04:36 +0100 Subject: [PATCH 1/2] implement if-match check in storageprovider --- changelog/unreleased/storage-if-match.md | 5 ++++ .../storageprovider/storageprovider.go | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 changelog/unreleased/storage-if-match.md diff --git a/changelog/unreleased/storage-if-match.md b/changelog/unreleased/storage-if-match.md new file mode 100644 index 0000000000..9a4ff9e3fb --- /dev/null +++ b/changelog/unreleased/storage-if-match.md @@ -0,0 +1,5 @@ +Enhancement: Add an if-match check to the storage provider + +Implement a check for the if-match value in InitiateFileUpload to prevent overwrites of newer versions. + +https://github.com/cs3org/reva/pull/2547 diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index c7fd5d9a06..cfd826601f 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -291,6 +291,29 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate }, nil } + if ifMatch := req.GetIfMatch(); ifMatch != "" { + sRes, err := s.Stat(ctx, &provider.StatRequest{Ref: req.Ref}) + if err != nil { + return nil, err + } + + switch sRes.Status.Code { + case rpc.Code_CODE_OK: + if sRes.Info.Etag != req.GetIfMatch() { + return &provider.InitiateFileUploadResponse{ + Status: status.NewFailedPrecondition(ctx, errors.New("etag doesn't match"), "etag doesn't match"), + }, nil + } + case rpc.Code_CODE_NOT_FOUND: + // Just continue with a normal upload + default: + return &provider.InitiateFileUploadResponse{ + Status: sRes.Status, + }, nil + } + + } + // FIXME these should be part of the InitiateFileUploadRequest object if req.Opaque != nil { if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { From e25b9b34f20cb1b7d9f64950d76bcd9343a12e69 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Thu, 17 Feb 2022 17:15:12 +0100 Subject: [PATCH 2/2] add if-match check to finish upload --- .../storageprovider/storageprovider.go | 9 ++++--- pkg/rhttp/datatx/manager/simple/simple.go | 2 ++ pkg/rhttp/datatx/manager/spaces/spaces.go | 2 ++ pkg/storage/utils/decomposedfs/upload.go | 26 +++++++++++++++---- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index cfd826601f..f87ccafaf9 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -291,7 +291,9 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate }, nil } - if ifMatch := req.GetIfMatch(); ifMatch != "" { + metadata := map[string]string{} + ifMatch := req.GetIfMatch() + if ifMatch != "" { sRes, err := s.Stat(ctx, &provider.StatRequest{Ref: req.Ref}) if err != nil { return nil, err @@ -299,7 +301,7 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate switch sRes.Status.Code { case rpc.Code_CODE_OK: - if sRes.Info.Etag != req.GetIfMatch() { + if sRes.Info.Etag != ifMatch { return &provider.InitiateFileUploadResponse{ Status: status.NewFailedPrecondition(ctx, errors.New("etag doesn't match"), "etag doesn't match"), }, nil @@ -311,7 +313,7 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate Status: sRes.Status, }, nil } - + metadata["if-match"] = ifMatch } // FIXME these should be part of the InitiateFileUploadRequest object @@ -321,7 +323,6 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate } } - metadata := map[string]string{} var uploadLength int64 if req.Opaque != nil && req.Opaque.Map != nil { if req.Opaque.Map["Upload-Length"] != nil { diff --git a/pkg/rhttp/datatx/manager/simple/simple.go b/pkg/rhttp/datatx/manager/simple/simple.go index 21bfde8244..ed93f6a5a7 100644 --- a/pkg/rhttp/datatx/manager/simple/simple.go +++ b/pkg/rhttp/datatx/manager/simple/simple.go @@ -91,6 +91,8 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { w.WriteHeader(http.StatusUnauthorized) case errtypes.InsufficientStorage: w.WriteHeader(http.StatusInsufficientStorage) + case errtypes.PreconditionFailed: + w.WriteHeader(http.StatusPreconditionFailed) default: sublog.Error().Err(v).Msg("error uploading file") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/rhttp/datatx/manager/spaces/spaces.go b/pkg/rhttp/datatx/manager/spaces/spaces.go index e451889f5e..249e947278 100644 --- a/pkg/rhttp/datatx/manager/spaces/spaces.go +++ b/pkg/rhttp/datatx/manager/spaces/spaces.go @@ -103,6 +103,8 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) { w.WriteHeader(http.StatusUnauthorized) case errtypes.InsufficientStorage: w.WriteHeader(http.StatusInsufficientStorage) + case errtypes.PreconditionFailed: + w.WriteHeader(http.StatusPreconditionFailed) default: sublog.Error().Err(v).Msg("error uploading file") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index 895b590d35..f73342ade6 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -124,24 +124,27 @@ func (fs *Decomposedfs) InitiateUpload(ctx context.Context, ref *provider.Refere } if metadata != nil { - if metadata["mtime"] != "" { - info.MetaData["mtime"] = metadata["mtime"] + if mtime, ok := metadata["mtime"]; ok { + info.MetaData["mtime"] = mtime } if _, ok := metadata["sizedeferred"]; ok { info.SizeIsDeferred = true } - if metadata["checksum"] != "" { - parts := strings.SplitN(metadata["checksum"], " ", 2) + if checksum, ok := metadata["checksum"]; ok { + parts := strings.SplitN(checksum, " ", 2) if len(parts) != 2 { return nil, errtypes.BadRequest("invalid checksum format. must be '[algorithm] [checksum]'") } switch parts[0] { case "sha1", "md5", "adler32": - info.MetaData["checksum"] = metadata["checksum"] + info.MetaData["checksum"] = checksum default: return nil, errtypes.BadRequest("unsupported checksum algorithm: " + parts[0]) } } + if ifMatch, ok := metadata["if-match"]; ok { + info.MetaData["if-match"] = ifMatch + } } log.Debug().Interface("info", info).Interface("node", n).Interface("metadata", metadata).Msg("Decomposedfs: resolved filename") @@ -538,6 +541,19 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) (err error) { // if target exists create new version versionsPath := "" if fi, err = os.Stat(targetPath); err == nil { + // When the if-match header was set we need to check if the + // etag still matches before finishing the upload. + if ifMatch, ok := upload.info.MetaData["if-match"]; ok { + var targetEtag string + targetEtag, err = node.CalculateEtag(n.ID, fi.ModTime()) + if err != nil { + return errtypes.InternalError(err.Error()) + } + if ifMatch != targetEtag { + return errtypes.PreconditionFailed("etag doesn't match") + } + } + // FIXME move versioning to blobs ... no need to copy all the metadata! well ... it does if we want to version metadata... // versions are stored alongside the actual file, so a rename can be efficient and does not cross storage / partition boundaries versionsPath = upload.fs.lu.InternalPath(n.ID + ".REV." + fi.ModTime().UTC().Format(time.RFC3339Nano))