OCFS+EOS properly process empty file upload (cs3org#734)

Whenever an upload of an empty file has started, finish it directly instead of waiting for further bytes. This fixes an issue when uploading empty files with PUT or POST. Co-authored-by: Jörn Friedrich Dreyer <[email protected]> Co-authored-by: Jörn Friedrich Dreyer <[email protected]>
owncloud · May 15, 2020 · 2a47afc · 2a47afc
1 parent aecfee9
commit 2a47afc
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 10 deletions.
diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go
@@ -254,6 +254,7 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) {
 
 	tusc, err := tus.NewClient(dataServerURL, c)
 	if err != nil {
+		log.Error().Err(err).Msg("Could not get TUS client")
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
@@ -273,10 +274,12 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) {
 	// start the uploading process.
 	err = uploader.Upload()
 	if err != nil {
+		log.Error().Err(err).Msg("Could not start TUS upload")
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 
+	// stat again to check the new file's metadata
 	sRes, err = client.Stat(ctx, sReq)
 	if err != nil {
 		log.Error().Err(err).Msg("error sending grpc stat request")
@@ -285,6 +288,7 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) {
 	}
 
 	if sRes.Status.Code != rpc.Code_CODE_OK {
+		log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes.Status.Code)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}

diff --git a/pkg/storage/fs/eos/upload.go b/pkg/storage/fs/eos/upload.go
@@ -148,6 +148,16 @@ func (fs *eosfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd
 		fs:       fs,
 	}
 
+	if !info.SizeIsDeferred && info.Size == 0 {
+		log.Debug().Interface("info", info).Msg("eos: finishing upload for empty file")
+		// no need to create info file and finish directly
+		err := u.FinishUpload(ctx)
+		if err != nil {
+			return nil, err
+		}
+		return u, nil
+	}
+
 	// writeInfo creates the file by itself if necessary
 	err = u.writeInfo()
 	if err != nil {
@@ -285,12 +295,16 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) error {
 		// cleanup in the background, delete might take a while and we don't need to wait for it to finish
 		go func() {
 			if err := os.Remove(upload.infoPath); err != nil {
-				log := appctx.GetLogger(ctx)
-				log.Err(err).Interface("info", upload.info).Msg("eos: could not delete upload info")
+				if !os.IsNotExist(err) {
+					log := appctx.GetLogger(ctx)
+					log.Err(err).Interface("info", upload.info).Msg("eos: could not delete upload info")
+				}
 			}
 			if err := os.Remove(upload.binPath); err != nil {
-				log := appctx.GetLogger(ctx)
-				log.Err(err).Interface("info", upload.info).Msg("eos: could not delete upload binary")
+				if !os.IsNotExist(err) {
+					log := appctx.GetLogger(ctx)
+					log.Err(err).Interface("info", upload.info).Msg("eos: could not delete upload binary")
+				}
 			}
 		}()
 	}
@@ -310,10 +324,14 @@ func (fs *eosfs) AsTerminatableUpload(upload tusd.Upload) tusd.TerminatableUploa
 // Terminate terminates the upload
 func (upload *fileUpload) Terminate(ctx context.Context) error {
 	if err := os.Remove(upload.infoPath); err != nil {
-		return err
+		if !os.IsNotExist(err) {
+			return err
+		}
 	}
 	if err := os.Remove(upload.binPath); err != nil {
-		return err
+		if !os.IsNotExist(err) {
+			return err
+		}
 	}
 	return nil
 }
diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go
@@ -159,6 +159,7 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd.
 		"LogLevel": log.GetLevel().String(),
 	}
 	// Create binary file in the upload folder with no content
+	log.Debug().Interface("info", info).Msg("ocfs: built storage info")
 	file, err := os.OpenFile(binPath, os.O_CREATE|os.O_WRONLY, defaultFilePerm)
 	if err != nil {
 		return nil, err
@@ -170,6 +171,17 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd.
 		binPath:  binPath,
 		infoPath: filepath.Join(fs.c.UploadInfoDir, info.ID+".info"),
 		fs:       fs,
+		ctx:      ctx,
+	}
+
+	if !info.SizeIsDeferred && info.Size == 0 {
+		log.Debug().Interface("info", info).Msg("ocfs: finishing upload for empty file")
+		// no need to create info file and finish directly
+		err := u.FinishUpload(ctx)
+		if err != nil {
+			return nil, err
+		}
+		return u, nil
 	}
 
 	// writeInfo creates the file by itself if necessary
@@ -340,10 +352,12 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) error {
 
 	// only delete the upload if it was successfully written to eos
 	if err := os.Remove(upload.infoPath); err != nil {
-		log.Err(err).Interface("info", upload.info).Msg("ocfs: could not delete upload info")
+		if !os.IsNotExist(err) {
+			log.Err(err).Interface("info", upload.info).Msg("ocfs: could not delete upload info")
+			return err
+		}
 	}
 
-	// FIXME metadata propagation is left to the storage implementation
 	return upload.fs.propagate(upload.ctx, np)
 }
 
@@ -359,10 +373,14 @@ func (fs *ocfs) AsTerminatableUpload(upload tusd.Upload) tusd.TerminatableUpload
 // Terminate terminates the upload
 func (upload *fileUpload) Terminate(ctx context.Context) error {
 	if err := os.Remove(upload.infoPath); err != nil {
-		return err
+		if !os.IsNotExist(err) {
+			return err
+		}
 	}
 	if err := os.Remove(upload.binPath); err != nil {
-		return err
+		if !os.IsNotExist(err) {
+			return err
+		}
 	}
 	return nil
 }