Fix truncating existing files

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

internal/http/services/owncloud/ocdav/tus.go

@@ -252,56 +252,58 @@ 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
+		finishUpload := true
+		if uploadLength > 0 {
+			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))
+			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))
 
-		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()
+			httpRes, err = s.client.Do(httpReq)
+			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
-		}
+			if httpRes.StatusCode != http.StatusNoContent {
+				w.WriteHeader(httpRes.StatusCode)
+				return
+			}
 
-		// 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.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,
 				}
 			}
+			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 +313,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 +331,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

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(),

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 (

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