From b46465ca423a1cacbb6c3b03263bf5023f5e6a4c Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Fri, 23 Jun 2023 13:51:04 +0200 Subject: [PATCH] fix mtime if 0 size file uploaded --- .drone.env | 2 +- changelog/unreleased/fix-mtime.md | 6 ++++ .../storageprovider/storageprovider.go | 4 ++- internal/http/services/owncloud/ocdav/put.go | 36 +++++++------------ pkg/storage/fs/cephfs/cephfs.go | 2 +- pkg/storage/fs/nextcloud/nextcloud.go | 2 +- pkg/storage/fs/owncloudsql/owncloudsql.go | 18 +++++++--- pkg/storage/fs/s3/s3.go | 2 +- pkg/storage/storage.go | 2 +- .../utils/decomposedfs/decomposedfs.go | 10 ++++-- pkg/storage/utils/decomposedfs/mocks/Tree.go | 10 +++--- pkg/storage/utils/decomposedfs/tree/tree.go | 7 +++- .../utils/decomposedfs/tree/tree_test.go | 2 +- pkg/storage/utils/eosfs/eosfs.go | 5 ++- pkg/storage/utils/localfs/localfs.go | 2 +- 15 files changed, 63 insertions(+), 47 deletions(-) create mode 100644 changelog/unreleased/fix-mtime.md diff --git a/.drone.env b/.drone.env index 4747c6fcd96..3d3fbb52e2e 100644 --- a/.drone.env +++ b/.drone.env @@ -1,4 +1,4 @@ # The test runner source for API tests -APITESTS_COMMITID=2ea3b8c400688b14ef328786b3af445c2edbbe18 +APITESTS_COMMITID=c8f8bc55e22597bc3d53b63b2f38f345b4814b07 APITESTS_BRANCH=master APITESTS_REPO_GIT_URL=https://github.com/owncloud/ocis.git diff --git a/changelog/unreleased/fix-mtime.md b/changelog/unreleased/fix-mtime.md new file mode 100644 index 00000000000..552cc5f133e --- /dev/null +++ b/changelog/unreleased/fix-mtime.md @@ -0,0 +1,6 @@ +Bugfix: fix mtime if 0 size file uploaded + +Fix mtime if 0 size file uploaded + +https://github.com/cs3org/reva/pull/4012 +https://github.com/owncloud/ocis/issues/1248 diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index e08aff6bdb8..8877cd114ac 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -662,13 +662,15 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta func (s *service) TouchFile(ctx context.Context, req *provider.TouchFileRequest) (*provider.TouchFileResponse, error) { // FIXME these should be part of the TouchFileRequest object + var mtime string if req.Opaque != nil { if e, ok := req.Opaque.Map["lockid"]; ok && e.Decoder == "plain" { ctx = ctxpkg.ContextSetLockID(ctx, string(e.Value)) } + mtime = utils.ReadPlainFromOpaque(req.Opaque, "X-OC-Mtime") } - err := s.storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing")) + err := s.storage.TouchFile(ctx, req.Ref, utils.ExistsInOpaque(req.Opaque, "markprocessing"), mtime) return &provider.TouchFileResponse{ Status: status.NewStatusFromErrType(ctx, "touch file", err), diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index c76aafeb652..3013be4672e 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -154,9 +154,17 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ w.WriteHeader(http.StatusInternalServerError) return } + opaque := &typespb.Opaque{} + if mtime := r.Header.Get(net.HeaderOCMtime); mtime != "" { + utils.AppendPlainToOpaque(opaque, net.HeaderOCMtime, mtime) + + // TODO: find a way to check if the storage really accepted the value + w.Header().Set(net.HeaderOCMtime, "accepted") + } if length == 0 { tfRes, err := client.TouchFile(ctx, &provider.TouchFileRequest{ - Ref: ref, + Opaque: opaque, + Ref: ref, }) if err != nil { log.Error().Err(err).Msg("error sending grpc touch file request") @@ -191,22 +199,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ return } - opaqueMap := map[string]*typespb.OpaqueEntry{ - net.HeaderUploadLength: { - Decoder: "plain", - Value: []byte(strconv.FormatInt(length, 10)), - }, - } - - if mtime := r.Header.Get(net.HeaderOCMtime); mtime != "" { - opaqueMap[net.HeaderOCMtime] = &typespb.OpaqueEntry{ - Decoder: "plain", - Value: []byte(mtime), - } - - // TODO: find a way to check if the storage really accepted the value - w.Header().Set(net.HeaderOCMtime, "accepted") - } + utils.AppendPlainToOpaque(opaque, net.HeaderUploadLength, strconv.FormatInt(length, 10)) // curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef' @@ -231,16 +224,13 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ // we do not check the algorithm here, because it might depend on the storage if len(cparts) == 2 { // Translate into TUS style Upload-Checksum header - opaqueMap[net.HeaderUploadChecksum] = &typespb.OpaqueEntry{ - Decoder: "plain", - // algorithm is always lowercase, checksum is separated by space - Value: []byte(strings.ToLower(cparts[0]) + " " + cparts[1]), - } + // algorithm is always lowercase, checksum is separated by space + utils.AppendPlainToOpaque(opaque, net.HeaderUploadChecksum, strings.ToLower(cparts[0])+" "+cparts[1]) } uReq := &provider.InitiateFileUploadRequest{ Ref: ref, - Opaque: &typespb.Opaque{Map: opaqueMap}, + Opaque: opaque, } if ifMatch := r.Header.Get(net.HeaderIfMatch); ifMatch != "" { uReq.Options = &provider.InitiateFileUploadRequest_IfMatch{IfMatch: ifMatch} diff --git a/pkg/storage/fs/cephfs/cephfs.go b/pkg/storage/fs/cephfs/cephfs.go index 12bbfe175ab..84b43aafe8a 100644 --- a/pkg/storage/fs/cephfs/cephfs.go +++ b/pkg/storage/fs/cephfs/cephfs.go @@ -165,7 +165,7 @@ func (fs *cephfs) CreateDir(ctx context.Context, ref *provider.Reference) error } // TouchFile as defined in the storage.FS interface -func (fs *cephfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error { +func (fs *cephfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error { return fmt.Errorf("unimplemented: TouchFile") } diff --git a/pkg/storage/fs/nextcloud/nextcloud.go b/pkg/storage/fs/nextcloud/nextcloud.go index e843cb19805..6ed8112645a 100644 --- a/pkg/storage/fs/nextcloud/nextcloud.go +++ b/pkg/storage/fs/nextcloud/nextcloud.go @@ -266,7 +266,7 @@ func (nc *StorageDriver) CreateDir(ctx context.Context, ref *provider.Reference) } // TouchFile as defined in the storage.FS interface -func (nc *StorageDriver) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error { +func (nc *StorageDriver) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error { return fmt.Errorf("unimplemented: TouchFile") } diff --git a/pkg/storage/fs/owncloudsql/owncloudsql.go b/pkg/storage/fs/owncloudsql/owncloudsql.go index eb1243a95ba..7da1e11e98a 100644 --- a/pkg/storage/fs/owncloudsql/owncloudsql.go +++ b/pkg/storage/fs/owncloudsql/owncloudsql.go @@ -794,7 +794,7 @@ func (fs *owncloudsqlfs) CreateDir(ctx context.Context, ref *provider.Reference) } // TouchFile as defined in the storage.FS interface -func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error { +func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error { ip, err := fs.resolve(ctx, ref) if err != nil { return err @@ -830,15 +830,25 @@ func (fs *owncloudsqlfs) TouchFile(ctx context.Context, ref *provider.Reference, if err != nil { return err } - mtime := time.Now().Unix() + storageMtime := time.Now().Unix() + mt := storageMtime + if mtime != "" { + t, err := strconv.Atoi(mtime) + if err != nil { + log.Info(). + Str("owncloudsql", ip). + Msg("error mtime conversion. mtine set to system time") + } + mt = time.Unix(int64(t), 0).Unix() + } data := map[string]interface{}{ "path": fs.toDatabasePath(ip), "etag": calcEtag(ctx, fi), "mimetype": mime.Detect(false, ip), "permissions": int(conversions.RoleFromResourcePermissions(parentPerms, false).OCSPermissions()), // inherit permissions of parent - "mtime": mtime, - "storage_mtime": mtime, + "mtime": mt, + "storage_mtime": storageMtime, } storageID, err := fs.getStorage(ctx, ip) if err != nil { diff --git a/pkg/storage/fs/s3/s3.go b/pkg/storage/fs/s3/s3.go index 52dbb2268c8..3cb8380dff2 100644 --- a/pkg/storage/fs/s3/s3.go +++ b/pkg/storage/fs/s3/s3.go @@ -348,7 +348,7 @@ func (fs *s3FS) CreateDir(ctx context.Context, ref *provider.Reference) error { } // TouchFile as defined in the storage.FS interface -func (fs *s3FS) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error { +func (fs *s3FS) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error { return fmt.Errorf("unimplemented: TouchFile") } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 93d1c8ba5a7..668b37f369c 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -38,7 +38,7 @@ type FS interface { GetHome(ctx context.Context) (string, error) CreateHome(ctx context.Context) error CreateDir(ctx context.Context, ref *provider.Reference) error - TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error + TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error Delete(ctx context.Context, ref *provider.Reference) error Move(ctx context.Context, oldRef, newRef *provider.Reference) error GetMD(ctx context.Context, ref *provider.Reference, mdKeys, fieldMask []string) (*provider.ResourceInfo, error) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index d105bea917d..7e342b58d49 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -48,6 +48,7 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/utils/chunking" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/lookup" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata" + "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/migrator" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/mtimesyncedcache" "github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node" @@ -76,7 +77,7 @@ type Tree interface { ListFolder(ctx context.Context, node *node.Node) ([]*node.Node, error) // CreateHome(owner *userpb.UserId) (n *node.Node, err error) CreateDir(ctx context.Context, node *node.Node) (err error) - TouchFile(ctx context.Context, node *node.Node, markprocessing bool) error + TouchFile(ctx context.Context, node *node.Node, markprocessing bool, mtime string) error // CreateReference(ctx context.Context, node *node.Node, targetURI *url.URL) error Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) (err error) Delete(ctx context.Context, node *node.Node) (err error) @@ -616,7 +617,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference) } // TouchFile as defined in the storage.FS interface -func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool) error { +func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, markprocessing bool, mtime string) error { parentRef := &provider.Reference{ ResourceId: ref.ResourceId, Path: path.Dir(ref.Path), @@ -655,7 +656,10 @@ func (fs *Decomposedfs) TouchFile(ctx context.Context, ref *provider.Reference, if err := n.CheckLock(ctx); err != nil { return err } - return fs.tp.TouchFile(ctx, n, markprocessing) + if err = n.SetXattrString(prefixes.MetadataPrefix+mtime, mtime); err != nil { + return err + } + return fs.tp.TouchFile(ctx, n, markprocessing, mtime) } // CreateReference creates a reference as a node folder with the target stored in extended attributes diff --git a/pkg/storage/utils/decomposedfs/mocks/Tree.go b/pkg/storage/utils/decomposedfs/mocks/Tree.go index 86316455bb3..848f13e4f36 100644 --- a/pkg/storage/utils/decomposedfs/mocks/Tree.go +++ b/pkg/storage/utils/decomposedfs/mocks/Tree.go @@ -278,13 +278,13 @@ func (_m *Tree) Setup() error { return r0 } -// TouchFile provides a mock function with given fields: ctx, _a1, markprocessing -func (_m *Tree) TouchFile(ctx context.Context, _a1 *node.Node, markprocessing bool) error { - ret := _m.Called(ctx, _a1, markprocessing) +// TouchFile provides a mock function with given fields: ctx, _a1, markprocessing, mtime +func (_m *Tree) TouchFile(ctx context.Context, _a1 *node.Node, markprocessing bool, mtime string) error { + ret := _m.Called(ctx, _a1, markprocessing, mtime) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *node.Node, bool) error); ok { - r0 = rf(ctx, _a1, markprocessing) + if rf, ok := ret.Get(0).(func(context.Context, *node.Node, bool, string) error); ok { + r0 = rf(ctx, _a1, markprocessing, mtime) } else { r0 = ret.Error(0) } diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index c8891f16dd0..4fa87cac1c0 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -128,7 +128,7 @@ func (t *Tree) GetMD(ctx context.Context, n *node.Node) (os.FileInfo, error) { } // TouchFile creates a new empty file -func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool) error { +func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool, mtime string) error { if n.Exists { if markprocessing { return n.SetXattr(prefixes.StatusPrefix, []byte(node.ProcessingStatus)) @@ -155,6 +155,11 @@ func (t *Tree) TouchFile(ctx context.Context, n *node.Node, markprocessing bool) if markprocessing { attributes[prefixes.StatusPrefix] = []byte(node.ProcessingStatus) } + if mtime != "" { + if err := n.SetMtimeString(mtime); err != nil { + return errors.Wrap(err, "Decomposedfs: could not set mtime") + } + } err = n.SetXattrs(attributes, true) if err != nil { return err diff --git a/pkg/storage/utils/decomposedfs/tree/tree_test.go b/pkg/storage/utils/decomposedfs/tree/tree_test.go index 2eebabf3786..8c7c9324e25 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree_test.go +++ b/pkg/storage/utils/decomposedfs/tree/tree_test.go @@ -246,7 +246,7 @@ var _ = Describe("Tree", func() { Expect(err).ToNot(HaveOccurred()) Expect(fileToBeCreated.Exists).To(BeFalse()) - err = t.TouchFile(env.Ctx, fileToBeCreated, false) + err = t.TouchFile(env.Ctx, fileToBeCreated, false, "") Expect(err).ToNot(HaveOccurred()) existingFile, err := env.Lookup.NodeFromResource(env.Ctx, ref) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 5aeb1ceded9..270524f38d4 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -21,6 +21,7 @@ package eosfs import ( "context" "database/sql" + b64 "encoding/base64" "encoding/json" "fmt" "io" @@ -33,8 +34,6 @@ import ( "strings" "time" - b64 "encoding/base64" - "github.com/bluele/gcache" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -1673,7 +1672,7 @@ func (fs *eosfs) CreateDir(ctx context.Context, ref *provider.Reference) error { } // TouchFile as defined in the storage.FS interface -func (fs *eosfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool) error { +func (fs *eosfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool, _ string) error { log := appctx.GetLogger(ctx) fn, auth, err := fs.resolveRefAndGetAuth(ctx, ref) diff --git a/pkg/storage/utils/localfs/localfs.go b/pkg/storage/utils/localfs/localfs.go index 5fc8862ac42..a37c084f180 100644 --- a/pkg/storage/utils/localfs/localfs.go +++ b/pkg/storage/utils/localfs/localfs.go @@ -806,7 +806,7 @@ func (fs *localfs) CreateDir(ctx context.Context, ref *provider.Reference) error } // TouchFile as defined in the storage.FS interface -func (fs *localfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool) error { +func (fs *localfs) TouchFile(ctx context.Context, ref *provider.Reference, _ bool, _ string) error { return fmt.Errorf("unimplemented: TouchFile") }