From bbe92e1ce8da19144dc09ce1bad7762f86b6a79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 8 Jan 2024 09:04:57 +0100 Subject: [PATCH 01/12] Set the upload length headers in the upload request opaque map --- pkg/storage/utils/metadata/cs3.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/storage/utils/metadata/cs3.go b/pkg/storage/utils/metadata/cs3.go index 2f0fed087e..c39cae9e0e 100644 --- a/pkg/storage/utils/metadata/cs3.go +++ b/pkg/storage/utils/metadata/cs3.go @@ -181,6 +181,8 @@ func (cs3 *CS3) Upload(ctx context.Context, req UploadRequest) (*UploadResponse, ifuReq.Opaque = utils.AppendPlainToOpaque(ifuReq.Opaque, "X-OC-Mtime", strconv.Itoa(int(req.MTime.Unix()))+"."+strconv.Itoa(req.MTime.Nanosecond())) } + ifuReq.Opaque = utils.AppendPlainToOpaque(ifuReq.Opaque, net.HeaderUploadLength, strconv.FormatInt(int64(len(req.Content)), 10)) + res, err := client.InitiateFileUpload(ctx, ifuReq) if err != nil { return nil, err From c90c0b03bec0cce84eb778a116e921077f632a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 8 Jan 2024 09:25:36 +0100 Subject: [PATCH 02/12] Directly finish 0-byte uploads when initiating The TUS handler will not finish the upload for us, that's usually already done in the TUS POST request already, which we bypass by doing the InitiateFileUpload flow. --- internal/http/services/owncloud/ocdav/tus.go | 88 ++++++++++---------- pkg/storage/utils/decomposedfs/upload.go | 8 ++ 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index d3592f1ab7..0ac9b7044d 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -252,56 +252,57 @@ 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) - if err != nil { - log.Debug().Err(err).Msg("wrong request") - w.WriteHeader(http.StatusInternalServerError) - return - } - Propagator.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) - - httpReq.Header.Set(net.HeaderContentType, r.Header.Get(net.HeaderContentType)) - httpReq.Header.Set(net.HeaderContentLength, r.Header.Get(net.HeaderContentLength)) - if r.Header.Get(net.HeaderUploadOffset) != "" { - httpReq.Header.Set(net.HeaderUploadOffset, r.Header.Get(net.HeaderUploadOffset)) - } else { - httpReq.Header.Set(net.HeaderUploadOffset, "0") - } - httpReq.Header.Set(net.HeaderTusResumable, r.Header.Get(net.HeaderTusResumable)) + finishUpload := true + if uploadLength > 0 { + var httpRes *http.Response - httpRes, err = s.client.Do(httpReq) - if err != nil { - log.Error().Err(err).Msg("error doing PATCH request to data gateway") - w.WriteHeader(http.StatusInternalServerError) - return - } - defer httpRes.Body.Close() + httpReq, err := rhttp.NewRequest(ctx, http.MethodPatch, ep, r.Body) + if err != nil { + log.Debug().Err(err).Msg("wrong request") + w.WriteHeader(http.StatusInternalServerError) + return + } + Propagator.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) + + httpReq.Header.Set(net.HeaderContentType, r.Header.Get(net.HeaderContentType)) + httpReq.Header.Set(net.HeaderContentLength, r.Header.Get(net.HeaderContentLength)) + if r.Header.Get(net.HeaderUploadOffset) != "" { + httpReq.Header.Set(net.HeaderUploadOffset, r.Header.Get(net.HeaderUploadOffset)) + } else { + httpReq.Header.Set(net.HeaderUploadOffset, "0") + } + httpReq.Header.Set(net.HeaderTusResumable, r.Header.Get(net.HeaderTusResumable)) - w.Header().Set(net.HeaderUploadOffset, httpRes.Header.Get(net.HeaderUploadOffset)) - w.Header().Set(net.HeaderTusResumable, httpRes.Header.Get(net.HeaderTusResumable)) - w.Header().Set(net.HeaderTusUploadExpires, httpRes.Header.Get(net.HeaderTusUploadExpires)) - if httpRes.StatusCode != http.StatusNoContent { - w.WriteHeader(httpRes.StatusCode) - return - } + httpRes, err = s.client.Do(httpReq) + if err != nil { + log.Error().Err(err).Msg("error doing PATCH request to data gateway") + w.WriteHeader(http.StatusInternalServerError) + return + } + defer httpRes.Body.Close() - // check if upload was fully completed - if length == 0 || httpRes.Header.Get(net.HeaderUploadOffset) == r.Header.Get(net.HeaderUploadLength) { - // get uploaded file metadata + w.Header().Set(net.HeaderUploadOffset, httpRes.Header.Get(net.HeaderUploadOffset)) + w.Header().Set(net.HeaderTusResumable, httpRes.Header.Get(net.HeaderTusResumable)) + w.Header().Set(net.HeaderTusUploadExpires, httpRes.Header.Get(net.HeaderTusUploadExpires)) + if httpRes.StatusCode != http.StatusNoContent { + w.WriteHeader(httpRes.StatusCode) + return + } if resid, err := storagespace.ParseID(httpRes.Header.Get(net.HeaderOCFileID)); err == nil { sReq.Ref = &provider.Reference{ ResourceId: &resid, } } + if httpRes != nil && httpRes.Header != nil && httpRes.Header.Get(net.HeaderOCMtime) != "" { + w.Header().Set(net.HeaderOCMtime, httpRes.Header.Get(net.HeaderOCMtime)) + } + finishUpload = httpRes.Header.Get(net.HeaderUploadOffset) == r.Header.Get(net.HeaderUploadLength) + } + + // check if upload was fully completed + if uploadLength == 0 || finishUpload { + // get uploaded file metadata sRes, err := client.Stat(ctx, sReq) if err != nil { @@ -311,7 +312,6 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. } if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - if sRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED { // the token expired during upload, so the stat failed // and we can't do anything about it. @@ -330,10 +330,6 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. w.WriteHeader(http.StatusInternalServerError) return } - if httpRes != nil && httpRes.Header != nil && httpRes.Header.Get(net.HeaderOCMtime) != "" { - // set the "accepted" value if returned in the upload response headers - w.Header().Set(net.HeaderOCMtime, httpRes.Header.Get(net.HeaderOCMtime)) - } // get WebDav permissions for file isPublic := false diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index 73dfd5496a..d3af49b90a 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -299,6 +299,14 @@ func (fs *Decomposedfs) InitiateUpload(ctx context.Context, ref *provider.Refere metrics.UploadSessionsInitiated.Inc() + if uploadLength == 0 { + // Directly finish this upload + err = session.FinishUpload(ctx) + if err != nil { + return nil, err + } + } + return map[string]string{ "simple": session.ID(), "tus": session.ID(), From 7c346ff0ce04a3e735ca358f58582140b29ba627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 8 Jan 2024 11:11:38 +0100 Subject: [PATCH 03/12] Add changelog --- changelog/unreleased/fix-truncating-files.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/fix-truncating-files.md diff --git a/changelog/unreleased/fix-truncating-files.md b/changelog/unreleased/fix-truncating-files.md new file mode 100644 index 0000000000..036ab80000 --- /dev/null +++ b/changelog/unreleased/fix-truncating-files.md @@ -0,0 +1,5 @@ +Bugfix: Fix truncating existing files + +We fixed a problem where existing files kept their content when being overwritten by a 0-byte file. + +https://github.com/cs3org/reva/pull/4448 From a5f9fa77e0175760a5964723c2e5db7130a1b4c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 8 Jan 2024 11:17:46 +0100 Subject: [PATCH 04/12] Fix linter issues --- internal/http/services/owncloud/ocdav/tus.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 0ac9b7044d..c32f4022a6 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -274,29 +274,30 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. httpReq.Header.Set(net.HeaderTusResumable, r.Header.Get(net.HeaderTusResumable)) httpRes, err = s.client.Do(httpReq) - if err != nil { + if err != nil || httpRes == nil { log.Error().Err(err).Msg("error doing PATCH request to data gateway") w.WriteHeader(http.StatusInternalServerError) return } defer httpRes.Body.Close() - w.Header().Set(net.HeaderUploadOffset, httpRes.Header.Get(net.HeaderUploadOffset)) - w.Header().Set(net.HeaderTusResumable, httpRes.Header.Get(net.HeaderTusResumable)) - w.Header().Set(net.HeaderTusUploadExpires, httpRes.Header.Get(net.HeaderTusUploadExpires)) if httpRes.StatusCode != http.StatusNoContent { w.WriteHeader(httpRes.StatusCode) return } + w.Header().Set(net.HeaderUploadOffset, httpRes.Header.Get(net.HeaderUploadOffset)) + w.Header().Set(net.HeaderTusResumable, httpRes.Header.Get(net.HeaderTusResumable)) + w.Header().Set(net.HeaderTusUploadExpires, httpRes.Header.Get(net.HeaderTusUploadExpires)) + if httpRes.Header.Get(net.HeaderOCMtime) != "" { + w.Header().Set(net.HeaderOCMtime, httpRes.Header.Get(net.HeaderOCMtime)) + } + if resid, err := storagespace.ParseID(httpRes.Header.Get(net.HeaderOCFileID)); err == nil { sReq.Ref = &provider.Reference{ ResourceId: &resid, } } - if httpRes != nil && httpRes.Header != nil && httpRes.Header.Get(net.HeaderOCMtime) != "" { - w.Header().Set(net.HeaderOCMtime, httpRes.Header.Get(net.HeaderOCMtime)) - } finishUpload = httpRes.Header.Get(net.HeaderUploadOffset) == r.Header.Get(net.HeaderUploadLength) } From 717c1c6ff2d55e25311e7671143c397aec9942ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 8 Jan 2024 15:00:58 +0100 Subject: [PATCH 05/12] Fix tests --- pkg/storage/utils/decomposedfs/upload_test.go | 45 ++----------------- 1 file changed, 3 insertions(+), 42 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/upload_test.go b/pkg/storage/utils/decomposedfs/upload_test.go index 44d4dd78c5..f2d72ccaf5 100644 --- a/pkg/storage/utils/decomposedfs/upload_test.go +++ b/pkg/storage/utils/decomposedfs/upload_test.go @@ -230,6 +230,8 @@ var _ = Describe("File uploads", func() { When("the user initiates a zero byte file upload", func() { It("succeeds", func() { + bs.On("Upload", mock.AnythingOfType("*node.Node"), mock.AnythingOfType("string"), mock.Anything). + Return(nil) uploadIds, err := fs.InitiateUpload(ctx, ref, 0, map[string]string{}) Expect(err).ToNot(HaveOccurred()) @@ -239,7 +241,7 @@ var _ = Describe("File uploads", func() { resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) Expect(err).ToNot(HaveOccurred()) - Expect(len(resources)).To(Equal(0)) + Expect(len(resources)).To(Equal(1)) }) }) @@ -284,47 +286,6 @@ var _ = Describe("File uploads", func() { }) }) - When("the user uploads a zero byte file", func() { - It("succeeds", func() { - var ( - fileContent = []byte("") - ) - - uploadIds, err := fs.InitiateUpload(ctx, ref, 0, map[string]string{}) - - Expect(err).ToNot(HaveOccurred()) - Expect(len(uploadIds)).To(Equal(2)) - Expect(uploadIds["simple"]).ToNot(BeEmpty()) - Expect(uploadIds["tus"]).ToNot(BeEmpty()) - - uploadRef := &provider.Reference{Path: "/" + uploadIds["simple"]} - - bs.On("Upload", mock.AnythingOfType("*node.Node"), mock.AnythingOfType("string"), mock.Anything). - Return(nil). - Run(func(args mock.Arguments) { - data, err := os.ReadFile(args.Get(1).(string)) - - Expect(err).ToNot(HaveOccurred()) - Expect(data).To(Equal([]byte(""))) - }) - - _, err = fs.Upload(ctx, storage.UploadRequest{ - Ref: uploadRef, - Body: io.NopCloser(bytes.NewReader(fileContent)), - Length: int64(len(fileContent)), - }, nil) - - Expect(err).ToNot(HaveOccurred()) - bs.AssertCalled(GinkgoT(), "Upload", mock.Anything, mock.Anything, mock.Anything) - - resources, err := fs.ListFolder(ctx, rootRef, []string{}, []string{}) - - Expect(err).ToNot(HaveOccurred()) - Expect(len(resources)).To(Equal(1)) - Expect(resources[0].Path).To(Equal(ref.Path)) - }) - }) - When("the user tries to upload a file without intialising the upload", func() { It("fails", func() { var ( From a2908c4ff8ebbb271c5d0ba2edf55442cd1510c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 9 Jan 2024 09:12:58 +0100 Subject: [PATCH 06/12] Do not try to upload to the datagateway if there's nothing to upload --- internal/http/services/owncloud/ocdav/put.go | 88 ++++++++++---------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index cb52ea695a..0c0b322b92 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -292,58 +292,58 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ } // ony send actual PUT request if file has bytes. Otherwise the initiate file upload request creates the file - // if length != 0 { // FIXME bring back 0 byte file upload handling, see https://github.com/owncloud/ocis/issues/2609 - - var ep, token string - for _, p := range uRes.Protocols { - if p.Protocol == "simple" { - ep, token = p.UploadEndpoint, p.Token + if length != 0 { // FIXME bring back 0 byte file upload handling, see https://github.com/owncloud/ocis/issues/2609 + var ep, token string + for _, p := range uRes.Protocols { + if p.Protocol == "simple" { + ep, token = p.UploadEndpoint, p.Token + } } - } - httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, ep, r.Body) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - Propagator.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) - httpReq.Header.Set(datagateway.TokenTransportHeader, token) + httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, ep, r.Body) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return + } + Propagator.Inject(ctx, propagation.HeaderCarrier(httpReq.Header)) + httpReq.Header.Set(datagateway.TokenTransportHeader, token) - httpRes, err := s.client.Do(httpReq) - if err != nil { - log.Error().Err(err).Msg("error doing PUT request to data service") - w.WriteHeader(http.StatusInternalServerError) - return - } - defer httpRes.Body.Close() - if httpRes.StatusCode != http.StatusOK { - if httpRes.StatusCode == http.StatusPartialContent { - w.WriteHeader(http.StatusPartialContent) + httpRes, err := s.client.Do(httpReq) + if err != nil { + log.Error().Err(err).Msg("error doing PUT request to data service") + w.WriteHeader(http.StatusInternalServerError) return } - if httpRes.StatusCode == errtypes.StatusChecksumMismatch { - w.WriteHeader(http.StatusBadRequest) - b, err := errors.Marshal(http.StatusBadRequest, "The computed checksum does not match the one received from the client.", "") - errors.HandleWebdavError(&log, w, b, err) + defer httpRes.Body.Close() + if httpRes.StatusCode != http.StatusOK { + if httpRes.StatusCode == http.StatusPartialContent { + w.WriteHeader(http.StatusPartialContent) + return + } + if httpRes.StatusCode == errtypes.StatusChecksumMismatch { + w.WriteHeader(http.StatusBadRequest) + b, err := errors.Marshal(http.StatusBadRequest, "The computed checksum does not match the one received from the client.", "") + errors.HandleWebdavError(&log, w, b, err) + return + } + log.Error().Err(err).Msg("PUT request to data server failed") + w.WriteHeader(httpRes.StatusCode) return } - log.Error().Err(err).Msg("PUT request to data server failed") - w.WriteHeader(httpRes.StatusCode) - return - } - // copy headers if they are present - if httpRes.Header.Get(net.HeaderETag) != "" { - w.Header().Set(net.HeaderETag, httpRes.Header.Get(net.HeaderETag)) - } - if httpRes.Header.Get(net.HeaderOCETag) != "" { - w.Header().Set(net.HeaderOCETag, httpRes.Header.Get(net.HeaderOCETag)) - } - if httpRes.Header.Get(net.HeaderOCFileID) != "" { - w.Header().Set(net.HeaderOCFileID, httpRes.Header.Get(net.HeaderOCFileID)) - } - if httpRes.Header.Get(net.HeaderLastModified) != "" { - w.Header().Set(net.HeaderLastModified, httpRes.Header.Get(net.HeaderLastModified)) + // copy headers if they are present + if httpRes.Header.Get(net.HeaderETag) != "" { + w.Header().Set(net.HeaderETag, httpRes.Header.Get(net.HeaderETag)) + } + if httpRes.Header.Get(net.HeaderOCETag) != "" { + w.Header().Set(net.HeaderOCETag, httpRes.Header.Get(net.HeaderOCETag)) + } + if httpRes.Header.Get(net.HeaderOCFileID) != "" { + w.Header().Set(net.HeaderOCFileID, httpRes.Header.Get(net.HeaderOCFileID)) + } + if httpRes.Header.Get(net.HeaderLastModified) != "" { + w.Header().Set(net.HeaderLastModified, httpRes.Header.Get(net.HeaderLastModified)) + } } // file was new From c560fa997f5fbe60357eb77601a4e339ee04fc47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 10 Jan 2024 08:26:06 +0100 Subject: [PATCH 07/12] Do not try to upload if there's nothing to upload --- tests/helpers/helpers.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/helpers/helpers.go b/tests/helpers/helpers.go index e80f16fff4..2d391b4b7c 100644 --- a/tests/helpers/helpers.go +++ b/tests/helpers/helpers.go @@ -94,20 +94,23 @@ func TempJSONFile(c any) (string, error) { // Upload can be used to initiate an upload and do the upload to a storage.FS in one step func Upload(ctx context.Context, fs storage.FS, ref *provider.Reference, content []byte) error { - uploadIds, err := fs.InitiateUpload(ctx, ref, 0, map[string]string{}) + length := int64(len(content)) + uploadIds, err := fs.InitiateUpload(ctx, ref, length, map[string]string{}) if err != nil { return err } - uploadID, ok := uploadIds["simple"] - if !ok { - return errors.New("simple upload method not available") + if length > 0 { + uploadID, ok := uploadIds["simple"] + if !ok { + return errors.New("simple upload method not available") + } + uploadRef := &provider.Reference{Path: "/" + uploadID} + _, err = fs.Upload(ctx, storage.UploadRequest{ + Ref: uploadRef, + Body: io.NopCloser(bytes.NewReader(content)), + Length: int64(len(content)), + }, nil) } - uploadRef := &provider.Reference{Path: "/" + uploadID} - _, err = fs.Upload(ctx, storage.UploadRequest{ - Ref: uploadRef, - Body: io.NopCloser(bytes.NewReader(content)), - Length: int64(len(content)), - }, nil) return err } From b8abd72dc51e7ba2416439da2ac148b52e541ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 10 Jan 2024 10:15:28 +0100 Subject: [PATCH 08/12] Fix tests --- .../fs/nextcloud/nextcloud_server_mock.go | 3 +- tests/helpers/helpers.go | 47 ++++++++++++------- .../cdd6db6c-331b-470e-8984-ea17caf555a4 | 0 .../cdd6db6c-331b-470e-8984-ea17caf555a4.info | 1 + 4 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4 create mode 100644 tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4.info diff --git a/pkg/storage/fs/nextcloud/nextcloud_server_mock.go b/pkg/storage/fs/nextcloud/nextcloud_server_mock.go index 3bb95e02a9..a23b5533f2 100644 --- a/pkg/storage/fs/nextcloud/nextcloud_server_mock.go +++ b/pkg/storage/fs/nextcloud/nextcloud_server_mock.go @@ -113,7 +113,8 @@ var responses = map[string]Response{ `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/GetMD {"ref":{"path":"/file"},"mdKeys":null}`: {404, ``, serverStateEmpty}, `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/InitiateUpload {"ref":{"path":"/file"},"uploadLength":0,"metadata":{"providerID":""}}`: {200, `{"simple": "yes","tus": "yes"}`, serverStateEmpty}, - `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/InitiateUpload {"ref":{"resource_id":{"storage_id":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"},"path":"/versionedFile"},"uploadLength":0,"metadata":{}}`: {200, `{"simple": "yes","tus": "yes"}`, serverStateEmpty}, + `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/InitiateUpload {"ref":{"resource_id":{"storage_id":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"},"path":"/versionedFile"},"uploadLength":1,"metadata":{}}`: {200, `{"simple": "yes","tus": "yes"}`, serverStateEmpty}, + `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/InitiateUpload {"ref":{"resource_id":{"storage_id":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"},"path":"/versionedFile"},"uploadLength":2,"metadata":{}}`: {200, `{"simple": "yes","tus": "yes"}`, serverStateEmpty}, `POST /apps/sciencemesh/~f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c/api/storage/GetMD {"ref":{"path":"/yes"},"mdKeys":[]}`: {200, `{"opaque":{},"type":1,"id":{"opaque_id":"fileid-/yes"},"checksum":{},"etag":"deadbeef","mime_type":"text/plain","mtime":{"seconds":1234567890},"path":"/yes","permission_set":{},"size":1,"canonical_metadata":{},"arbitrary_metadata":{}}`, serverStateEmpty}, diff --git a/tests/helpers/helpers.go b/tests/helpers/helpers.go index 2d391b4b7c..d46fe18abf 100644 --- a/tests/helpers/helpers.go +++ b/tests/helpers/helpers.go @@ -28,17 +28,21 @@ import ( "os" "path/filepath" "runtime" + "strconv" + "github.com/owncloud/ocis/v2/services/webdav/pkg/net" "github.com/pkg/errors" "github.com/studio-b12/gowebdav" gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/datagateway" "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/rhttp" "github.com/cs3org/reva/v2/pkg/storage" + "github.com/cs3org/reva/v2/pkg/utils" ) // TempDir creates a temporary directory in tmp/ and returns its path @@ -238,31 +242,38 @@ func CreateStructure(ctx context.Context, gw gatewayv1beta1.GatewayAPIClient, ro // CreateFile creates a file in the given path with an initial content. func CreateFile(ctx context.Context, gw gatewayv1beta1.GatewayAPIClient, ref *provider.Reference, content []byte) error { - initRes, err := gw.InitiateFileUpload(ctx, &provider.InitiateFileUploadRequest{Ref: ref}) + length := int64(len(content)) + initRes, err := gw.InitiateFileUpload(ctx, &provider.InitiateFileUploadRequest{ + Opaque: utils.AppendPlainToOpaque(&typespb.Opaque{}, net.HeaderUploadLength, strconv.FormatInt(length, 10)), + Ref: ref, + }) if err != nil { return err } - var token, endpoint string - for _, p := range initRes.Protocols { - if p.Protocol == "simple" { - token, endpoint = p.Token, p.UploadEndpoint + + if length > 0 { + var token, endpoint string + for _, p := range initRes.Protocols { + if p.Protocol == "simple" { + token, endpoint = p.Token, p.UploadEndpoint + } + } + httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, endpoint, bytes.NewReader(content)) + if err != nil { + return err } - } - httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, endpoint, bytes.NewReader(content)) - if err != nil { - return err - } - httpReq.Header.Set(datagateway.TokenTransportHeader, token) + httpReq.Header.Set(datagateway.TokenTransportHeader, token) - httpRes, err := http.DefaultClient.Do(httpReq) - if err != nil { - return err - } - if httpRes.StatusCode != http.StatusOK { - return errors.New(httpRes.Status) + httpRes, err := http.DefaultClient.Do(httpReq) + if err != nil { + return err + } + if httpRes.StatusCode != http.StatusOK { + return errors.New(httpRes.Status) + } + defer httpRes.Body.Close() } - defer httpRes.Body.Close() return nil } diff --git a/tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4 b/tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4 new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4.info b/tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4.info new file mode 100644 index 0000000000..80a4a8c13e --- /dev/null +++ b/tests/integration/grpc/uploads/cdd6db6c-331b-470e-8984-ea17caf555a4.info @@ -0,0 +1 @@ +{"ID":"cdd6db6c-331b-470e-8984-ea17caf555a4","Size":0,"SizeIsDeferred":false,"Offset":0,"MetaData":{"dir":"/","filename":"6b8f7ecf-e0c2-4ce5-9f7d-3f074aec8a8a","idp":"cesnet.cz","user":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"},"IsPartial":false,"IsFinal":false,"PartialUploads":null,"Storage":{"Path":"uploads","Type":"OCM"}} \ No newline at end of file From e1b3eabdfe4b88a656d6e3b6f547f2c9d0c85a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 10 Jan 2024 15:05:14 +0100 Subject: [PATCH 09/12] Use latest cs3api-validator to fix upload test failures --- .drone.star | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.star b/.drone.star index 089d6270c0..d7c67dca6b 100644 --- a/.drone.star +++ b/.drone.star @@ -5,7 +5,7 @@ OSIXIA_OPEN_LDAP = "osixia/openldap:1.3.0" REDIS = "redis:6-alpine" OC_CI_PHP = "cs3org/behat:latest" OC_LITMUS = "owncloud/litmus:latest" -OC_CS3_API_VALIDATOR = "owncloud/cs3api-validator:0.2.0" +OC_CS3_API_VALIDATOR = "owncloud/cs3api-validator:latest" OC_CI_BAZEL_BUILDIFIER = "owncloudci/bazel-buildifier:latest" # Shared step definitions From 9d4259905ffb36387c69b2ae63c814c50612b934 Mon Sep 17 00:00:00 2001 From: Andre Duffeck Date: Wed, 10 Jan 2024 17:17:27 +0100 Subject: [PATCH 10/12] Update internal/http/services/owncloud/ocdav/put.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/put.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 0c0b322b92..7a6324ce38 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -292,7 +292,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ } // ony send actual PUT request if file has bytes. Otherwise the initiate file upload request creates the file - if length != 0 { // FIXME bring back 0 byte file upload handling, see https://github.com/owncloud/ocis/issues/2609 + if length != 0 { var ep, token string for _, p := range uRes.Protocols { if p.Protocol == "simple" { From 275e6a12ca7e1c5774bb2ddbfd635050d4f73fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 10 Jan 2024 17:05:36 +0100 Subject: [PATCH 11/12] Add test --- pkg/storage/utils/decomposedfs/upload_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/storage/utils/decomposedfs/upload_test.go b/pkg/storage/utils/decomposedfs/upload_test.go index f2d72ccaf5..6388ce321a 100644 --- a/pkg/storage/utils/decomposedfs/upload_test.go +++ b/pkg/storage/utils/decomposedfs/upload_test.go @@ -243,6 +243,26 @@ var _ = Describe("File uploads", func() { Expect(err).ToNot(HaveOccurred()) Expect(len(resources)).To(Equal(1)) }) + + It("fails when trying to upload empty data. 0-byte uploads are finished during initialization already", func() { + bs.On("Upload", mock.AnythingOfType("*node.Node"), mock.AnythingOfType("string"), mock.Anything). + Return(nil) + uploadIds, err := fs.InitiateUpload(ctx, ref, 0, map[string]string{}) + + Expect(err).ToNot(HaveOccurred()) + Expect(len(uploadIds)).To(Equal(2)) + Expect(uploadIds["simple"]).ToNot(BeEmpty()) + + uploadRef := &provider.Reference{Path: "/" + uploadIds["simple"]} + + _, err = fs.Upload(ctx, storage.UploadRequest{ + Ref: uploadRef, + Body: io.NopCloser(bytes.NewReader([]byte(""))), + Length: 0, + }, nil) + + Expect(err).To(HaveOccurred()) + }) }) When("the user uploads a non zero byte file", func() { From 51cb2d79ae8da601d57e617d3cc4e2ae9d0b7d56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 10 Jan 2024 17:15:54 +0100 Subject: [PATCH 12/12] Use tagged version of the cs3api-validator image --- .drone.star | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.star b/.drone.star index d7c67dca6b..a8238e1d3d 100644 --- a/.drone.star +++ b/.drone.star @@ -5,7 +5,7 @@ OSIXIA_OPEN_LDAP = "osixia/openldap:1.3.0" REDIS = "redis:6-alpine" OC_CI_PHP = "cs3org/behat:latest" OC_LITMUS = "owncloud/litmus:latest" -OC_CS3_API_VALIDATOR = "owncloud/cs3api-validator:latest" +OC_CS3_API_VALIDATOR = "owncloud/cs3api-validator:0.2.1" OC_CI_BAZEL_BUILDIFIER = "owncloudci/bazel-buildifier:latest" # Shared step definitions