From a30d90c96dd05456baa92271040cc3e63b9d0f95 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 11 May 2020 17:11:02 +0200 Subject: [PATCH] OCFS+EOS properly process empty file upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/http/services/owncloud/ocdav/put.go | 4 +++ pkg/storage/fs/eos/upload.go | 30 ++++++++++++++++---- pkg/storage/fs/owncloud/upload.go | 26 ++++++++++++++--- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 7c621002ef..a5f318b003 100644 --- 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 index 0a024727a5..f7fed5b781 100644 --- 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 index 58dcc81958..c40a6ab6d3 100644 --- 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 }