diff --git a/changelog/unreleased/fix-0-byte-uploads.md b/changelog/unreleased/fix-0-byte-uploads.md new file mode 100644 index 0000000000..74dcda6077 --- /dev/null +++ b/changelog/unreleased/fix-0-byte-uploads.md @@ -0,0 +1,5 @@ +Bugfix: Fix 0-byte-uploads + +We fixed a problem with 0-byte uploads by using TouchFile instead of going through TUS (decomposedfs and owncloudsql storage drivers only for now). + +https://github.com/cs3org/reva/pull/2998 diff --git a/internal/grpc/interceptors/eventsmiddleware/conversion.go b/internal/grpc/interceptors/eventsmiddleware/conversion.go index df095b402d..4d5ca49dbe 100644 --- a/internal/grpc/interceptors/eventsmiddleware/conversion.go +++ b/internal/grpc/interceptors/eventsmiddleware/conversion.go @@ -165,6 +165,14 @@ func LinkRemoved(r *link.RemovePublicShareResponse, req *link.RemovePublicShareR } } +// FileTouched converts the response to an event +func FileTouched(r *provider.TouchFileResponse, req *provider.TouchFileRequest, executant *user.UserId) events.FileTouched { + return events.FileTouched{ + Executant: executant, + Ref: req.Ref, + } +} + // FileUploaded converts the response to an event func FileUploaded(r *provider.InitiateFileUploadResponse, req *provider.InitiateFileUploadRequest, executant *user.UserId) events.FileUploaded { return events.FileUploaded{ diff --git a/internal/grpc/interceptors/eventsmiddleware/events.go b/internal/grpc/interceptors/eventsmiddleware/events.go index 05f5fdd29e..c53608de63 100644 --- a/internal/grpc/interceptors/eventsmiddleware/events.go +++ b/internal/grpc/interceptors/eventsmiddleware/events.go @@ -156,6 +156,10 @@ func NewUnary(m map[string]interface{}) (grpc.UnaryServerInterceptor, int, error ev = SpaceDisabled(v, r, executantID) } } + case *provider.TouchFileResponse: + if isSuccess(v) { + ev = FileTouched(v, req.(*provider.TouchFileRequest), executantID) + } } if ev != nil { diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index b72db362a4..1b6b3b8dcb 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -626,7 +626,9 @@ func (s *svc) CreateContainer(ctx context.Context, req *provider.CreateContainer } func (s *svc) TouchFile(ctx context.Context, req *provider.TouchFileRequest) (*provider.TouchFileResponse, error) { - c, _, err := s.find(ctx, req.Ref) + var c provider.ProviderAPIClient + var err error + c, _, req.Ref, err = s.findAndUnwrap(ctx, req.Ref) if err != nil { return &provider.TouchFileResponse{ Status: status.NewStatusFromErrType(ctx, "TouchFile ref="+req.Ref.String(), err), diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index be29b79ca9..8fa0f22264 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -168,6 +168,28 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. } } + uploadLength, err := strconv.ParseInt(r.Header.Get(net.HeaderUploadLength), 10, 64) + if err != nil { + log.Debug().Err(err).Msg("wrong request") + w.WriteHeader(http.StatusBadRequest) + return + } + if uploadLength == 0 { + tfRes, err := client.TouchFile(ctx, &provider.TouchFileRequest{ + Ref: ref, + }) + if err != nil { + log.Error().Err(err).Msg("error sending grpc stat request") + w.WriteHeader(http.StatusInternalServerError) + return + } + if tfRes.Status.Code != rpc.Code_CODE_OK { + log.Error().Interface("status", tfRes.Status).Msg("error touching file") + w.WriteHeader(http.StatusInternalServerError) + return + } + } + opaqueMap := map[string]*typespb.OpaqueEntry{ net.HeaderUploadLength: { Decoder: "plain", @@ -228,14 +250,12 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. // for creation-with-upload extension forward bytes to dataprovider // TODO check this really streams if r.Header.Get(net.HeaderContentType) == "application/offset+octet-stream" { - length, err := strconv.ParseInt(r.Header.Get(net.HeaderContentLength), 10, 64) if err != nil { log.Debug().Err(err).Msg("wrong request") w.WriteHeader(http.StatusBadRequest) return } - var httpRes *http.Response httpReq, err := rhttp.NewRequest(ctx, http.MethodPatch, ep, r.Body) diff --git a/pkg/events/files.go b/pkg/events/files.go index c1722d3917..051d6237cf 100644 --- a/pkg/events/files.go +++ b/pkg/events/files.go @@ -53,6 +53,19 @@ func (FileUploaded) Unmarshal(v []byte) (interface{}, error) { return e, err } +// FileTouched is emitted when a file is uploaded +type FileTouched struct { + Executant *user.UserId + Ref *provider.Reference +} + +// Unmarshal to fulfill umarshaller interface +func (FileTouched) Unmarshal(v []byte) (interface{}, error) { + e := FileTouched{} + err := json.Unmarshal(v, &e) + return e, err +} + // FileDownloaded is emitted when a file is downloaded type FileDownloaded struct { Executant *user.UserId diff --git a/pkg/storage/fs/owncloudsql/owncloudsql.go b/pkg/storage/fs/owncloudsql/owncloudsql.go index b5b4afc5c6..addae9ccd8 100644 --- a/pkg/storage/fs/owncloudsql/owncloudsql.go +++ b/pkg/storage/fs/owncloudsql/owncloudsql.go @@ -790,7 +790,61 @@ 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) error { - return fmt.Errorf("unimplemented: TouchFile") + ip, err := fs.resolve(ctx, ref) + if err != nil { + return err + } + + // check permissions of parent dir + parentPerms, err := fs.readPermissions(ctx, filepath.Dir(ip)) + if err == nil { + if !parentPerms.InitiateFileUpload { + return errtypes.PermissionDenied("") + } + } else { + if isNotFound(err) { + return errtypes.NotFound(ref.Path) + } + return errors.Wrap(err, "owncloudsql: error reading permissions") + } + + _, err = os.Create(ip) + if err != nil { + if os.IsNotExist(err) { + return errtypes.NotFound(ref.Path) + } + // FIXME we also need already exists error, webdav expects 405 MethodNotAllowed + return errors.Wrap(err, "owncloudsql: error creating file "+fs.toStoragePath(ctx, filepath.Dir(ip))) + } + + if err = os.Chmod(ip, 0700); err != nil { + return errors.Wrap(err, "owncloudsql: error setting file permissions on "+fs.toStoragePath(ctx, filepath.Dir(ip))) + } + + fi, err := os.Stat(ip) + if err != nil { + return err + } + mtime := time.Now().Unix() + + data := map[string]interface{}{ + "path": fs.toDatabasePath(ip), + "etag": calcEtag(ctx, fi), + "mimetype": mime.Detect(false, ip), + "permissions": int(conversions.RoleFromResourcePermissions(parentPerms).OCSPermissions()), // inherit permissions of parent + "mtime": mtime, + "storage_mtime": mtime, + } + storageID, err := fs.getStorage(ctx, ip) + if err != nil { + return err + } + _, err = fs.filecache.InsertOrUpdate(ctx, storageID, data, false) + if err != nil { + return err + } + + return fs.propagate(ctx, filepath.Dir(ip)) } func (fs *owncloudsqlfs) CreateReference(ctx context.Context, sp string, targetURI *url.URL) error { diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 09d8f52e0b..7897696eab 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -24,7 +24,6 @@ package decomposedfs import ( "context" - "fmt" "io" "net/url" "os" @@ -78,6 +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) 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) @@ -337,7 +337,39 @@ 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) error { - return fmt.Errorf("unimplemented: TouchFile") + parentRef := &provider.Reference{ + ResourceId: ref.ResourceId, + Path: path.Dir(ref.Path), + } + + // verify parent exists + parent, err := fs.lu.NodeFromResource(ctx, parentRef) + if err != nil { + return errtypes.InternalError(err.Error()) + } + if !parent.Exists { + return errtypes.NotFound(parentRef.Path) + } + + n, err := fs.lu.NodeFromResource(ctx, ref) + if err != nil { + return errtypes.InternalError(err.Error()) + } + ok, err := fs.p.HasPermission(ctx, n, func(rp *provider.ResourcePermissions) bool { + return rp.InitiateFileUpload + }) + switch { + case err != nil: + return errtypes.InternalError(err.Error()) + case !ok: + return errtypes.PermissionDenied(filepath.Join(n.ParentID, n.Name)) + } + + // check lock + if err := n.CheckLock(ctx); err != nil { + return err + } + return fs.tp.TouchFile(ctx, n) } // 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 d71ac7345a..f41f8c2a83 100644 --- a/pkg/storage/utils/decomposedfs/mocks/Tree.go +++ b/pkg/storage/utils/decomposedfs/mocks/Tree.go @@ -265,6 +265,20 @@ func (_m *Tree) Setup() error { return r0 } +// TouchFile provides a mock function with given fields: ctx, _a1 +func (_m *Tree) TouchFile(ctx context.Context, _a1 *node.Node) error { + ret := _m.Called(ctx, _a1) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *node.Node) error); ok { + r0 = rf(ctx, _a1) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // WriteBlob provides a mock function with given fields: _a0, reader func (_m *Tree) WriteBlob(_a0 *node.Node, reader io.Reader) error { ret := _m.Called(_a0, reader) diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index e3461d78d8..2e21b77d06 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "io/fs" + iofs "io/fs" "os" "path/filepath" "strconv" @@ -228,6 +229,49 @@ func (t *Tree) GetMD(ctx context.Context, n *node.Node) (os.FileInfo, error) { return md, nil } +// TouchFile creates a new empty file +func (t *Tree) TouchFile(ctx context.Context, n *node.Node) error { + if n.Exists { + return errtypes.AlreadyExists(n.ID) + } + + if n.ID == "" { + n.ID = uuid.New().String() + } + + nodePath := n.InternalPath() + if err := os.MkdirAll(filepath.Dir(nodePath), 0700); err != nil { + return errors.Wrap(err, "Decomposedfs: error creating node") + } + _, err := os.Create(nodePath) + if err != nil { + return errors.Wrap(err, "Decomposedfs: error creating node") + } + + err = n.WriteAllNodeMetadata() + if err != nil { + return err + } + + // link child name to parent if it is new + childNameLink := filepath.Join(n.ParentInternalPath(), n.Name) + var link string + link, err = os.Readlink(childNameLink) + if err == nil && link != "../"+n.ID { + if err = os.Remove(childNameLink); err != nil { + return errors.Wrap(err, "Decomposedfs: could not remove symlink child entry") + } + } + if errors.Is(err, iofs.ErrNotExist) || link != "../"+n.ID { + relativeNodePath := filepath.Join("../../../../../", lookup.Pathify(n.ID, 4, 2)) + if err = os.Symlink(relativeNodePath, childNameLink); err != nil { + return errors.Wrap(err, "Decomposedfs: could not symlink child entry") + } + } + + return t.Propagate(ctx, n) +} + // CreateDir creates a new directory entry in the tree func (t *Tree) CreateDir(ctx context.Context, n *node.Node) (err error) { diff --git a/pkg/storage/utils/decomposedfs/tree/tree_test.go b/pkg/storage/utils/decomposedfs/tree/tree_test.go index d590025bd1..4b85656d23 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree_test.go +++ b/pkg/storage/utils/decomposedfs/tree/tree_test.go @@ -233,6 +233,25 @@ var _ = Describe("Tree", func() { Expect(err).ToNot(HaveOccurred()) }) + Describe("TouchFile", func() { + It("creates a file inside", func() { + ref := &provider.Reference{ + ResourceId: env.SpaceRootRes, + Path: "emptydir/newFile", + } + fileToBeCreated, err := env.Lookup.NodeFromResource(env.Ctx, ref) + Expect(err).ToNot(HaveOccurred()) + Expect(fileToBeCreated.Exists).To(BeFalse()) + + err = t.TouchFile(env.Ctx, fileToBeCreated) + Expect(err).ToNot(HaveOccurred()) + + existingFile, err := env.Lookup.NodeFromResource(env.Ctx, ref) + Expect(err).ToNot(HaveOccurred()) + Expect(existingFile.Exists).To(BeTrue()) + }) + }) + Context("that was deleted", func() { var ( trashPath string