From 8f793172c278c7894b91e0350dcf47d9308f7183 Mon Sep 17 00:00:00 2001 From: Anass Bouassaba Date: Thu, 11 Jul 2024 02:30:27 +0200 Subject: [PATCH] fix: temporary files not deleted, correctly check if files exist (#158) --- api/router/file_router.go | 18 ++++++------- conversion/pipeline/audio_video_pipeline.go | 10 ++++---- conversion/pipeline/glb_pipeline.go | 19 +++++++------- conversion/pipeline/image_pipeline.go | 28 ++++++++++----------- conversion/pipeline/insights_pipeline.go | 27 +++++++++----------- conversion/pipeline/mosaic_pipeline.go | 9 +++---- conversion/pipeline/office_pipeline.go | 10 ++++---- conversion/pipeline/pdf_pipeline.go | 19 +++++++------- conversion/pipeline/zip_pipeline.go | 19 +++++++------- conversion/processor/pdf_processor.go | 3 +-- conversion/processor/video_processor.go | 10 ++++---- webdav/handler/method_put.go | 24 +++++++++--------- webdav/main.go | 2 +- 13 files changed, 93 insertions(+), 105 deletions(-) diff --git a/api/router/file_router.go b/api/router/file_router.go index 667da7051..9c3544280 100644 --- a/api/router/file_router.go +++ b/api/router/file_router.go @@ -154,11 +154,10 @@ func (r *FileRouter) Create(c *fiber.Ctx) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - log.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + log.GetLogger().Error(err) } }(tmpPath) file, err = r.fileSvc.Store(file.ID, service.StoreOptions{Path: &tmpPath}, userID) @@ -221,11 +220,10 @@ func (r *FileRouter) Patch(c *fiber.Ctx) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - log.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + log.GetLogger().Error(err) } }(tmpPath) file, err = r.fileSvc.Store(file.ID, service.StoreOptions{Path: &tmpPath}, userID) diff --git a/conversion/pipeline/audio_video_pipeline.go b/conversion/pipeline/audio_video_pipeline.go index 774c6f474..695accc6e 100644 --- a/conversion/pipeline/audio_video_pipeline.go +++ b/conversion/pipeline/audio_video_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -48,11 +49,10 @@ func (p *audioVideoPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ diff --git a/conversion/pipeline/glb_pipeline.go b/conversion/pipeline/glb_pipeline.go index bf32dc61b..05cd91219 100644 --- a/conversion/pipeline/glb_pipeline.go +++ b/conversion/pipeline/glb_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -48,11 +49,10 @@ func (p *glbPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ @@ -85,11 +85,10 @@ func (p *glbPipeline) Run(opts client.PipelineRunOptions) error { func (p *glbPipeline) createThumbnail(inputPath string, opts client.PipelineRunOptions) error { tmpPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".jpeg") defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(tmpPath) if err := p.glbProc.Thumbnail(inputPath, p.config.Limits.ImagePreviewMaxWidth, p.config.Limits.ImagePreviewMaxHeight, "rgb(255,255,255)", tmpPath); err != nil { diff --git a/conversion/pipeline/image_pipeline.go b/conversion/pipeline/image_pipeline.go index 6859c7d4b..758f29627 100644 --- a/conversion/pipeline/image_pipeline.go +++ b/conversion/pipeline/image_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -49,11 +50,10 @@ func (p *imagePipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ @@ -136,11 +136,10 @@ func (p *imagePipeline) createThumbnail(inputPath string, opts client.PipelineRu } if *isAvailable { defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(tmpPath) } else { @@ -179,11 +178,10 @@ func (p *imagePipeline) convertTIFFToJPEG(inputPath string, imageProps client.Im return nil, err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(jpegPath) stat, err := os.Stat(jpegPath) diff --git a/conversion/pipeline/insights_pipeline.go b/conversion/pipeline/insights_pipeline.go index 3385c3eee..7ab0d1b2b 100644 --- a/conversion/pipeline/insights_pipeline.go +++ b/conversion/pipeline/insights_pipeline.go @@ -57,11 +57,10 @@ func (p *insightsPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ @@ -108,11 +107,10 @@ func (p *insightsPipeline) createText(inputPath string, opts client.PipelineRunO return nil, err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(noAlphaImagePath) /* Convert to PDF/A */ @@ -121,11 +119,10 @@ func (p *insightsPipeline) createText(inputPath string, opts client.PipelineRunO return nil, err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(pdfPath) /* Set OCR S3 object */ diff --git a/conversion/pipeline/mosaic_pipeline.go b/conversion/pipeline/mosaic_pipeline.go index a82ad28ac..3d4a2fa3d 100644 --- a/conversion/pipeline/mosaic_pipeline.go +++ b/conversion/pipeline/mosaic_pipeline.go @@ -49,11 +49,10 @@ func (p *mosaicPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ diff --git a/conversion/pipeline/office_pipeline.go b/conversion/pipeline/office_pipeline.go index a28117294..5bcc9a588 100644 --- a/conversion/pipeline/office_pipeline.go +++ b/conversion/pipeline/office_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -84,11 +85,10 @@ func (p *officePipeline) convertToPDF(inputPath string, opts client.PipelineRunO return nil, err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(*outputPath) defer func(path string) { diff --git a/conversion/pipeline/pdf_pipeline.go b/conversion/pipeline/pdf_pipeline.go index 444d4315f..79a8222f5 100644 --- a/conversion/pipeline/pdf_pipeline.go +++ b/conversion/pipeline/pdf_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -51,11 +52,10 @@ func (p *pdfPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) if err := p.apiClient.PatchTask(opts.TaskID, client.TaskPatchOptions{ @@ -101,11 +101,10 @@ func (p *pdfPipeline) createThumbnail(inputPath string, opts client.PipelineRunO return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(tmpPath) props, err := p.imageProc.MeasureImage(tmpPath) diff --git a/conversion/pipeline/zip_pipeline.go b/conversion/pipeline/zip_pipeline.go index aa87a7b8a..305121a28 100644 --- a/conversion/pipeline/zip_pipeline.go +++ b/conversion/pipeline/zip_pipeline.go @@ -11,6 +11,7 @@ package pipeline import ( + "errors" "os" "path/filepath" @@ -50,11 +51,10 @@ func (p *zipPipeline) Run(opts client.PipelineRunOptions) error { return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(inputPath) isGLTF, err := p.fi.IsGLTF(inputPath) @@ -113,11 +113,10 @@ func (p *zipPipeline) convertToGLB(inputPath string, opts client.PipelineRunOpti return nil, err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(outputPath) stat, err := os.Stat(outputPath) diff --git a/conversion/processor/pdf_processor.go b/conversion/processor/pdf_processor.go index 74eb45f70..9da81b859 100644 --- a/conversion/processor/pdf_processor.go +++ b/conversion/processor/pdf_processor.go @@ -13,7 +13,6 @@ package processor import ( "errors" "fmt" - "io/fs" "os" "path/filepath" "strconv" @@ -46,7 +45,7 @@ func (p *PDFProcessor) TextFromPDF(inputPath string) (*string, error) { } defer func(path string) { - if err := os.Remove(path); errors.Is(err, fs.ErrNotExist) { + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { return } else if err != nil { infra.GetLogger().Error(err) diff --git a/conversion/processor/video_processor.go b/conversion/processor/video_processor.go index 6e383bef9..9ec1ca142 100644 --- a/conversion/processor/video_processor.go +++ b/conversion/processor/video_processor.go @@ -11,6 +11,7 @@ package processor import ( + "errors" "os" "path/filepath" @@ -39,11 +40,10 @@ func (p *VideoProcessor) Thumbnail(inputPath string, width int, height int, outp return err } defer func(path string) { - _, err := os.Stat(path) - if os.IsExist(err) { - if err := os.Remove(path); err != nil { - infra.GetLogger().Error(err) - } + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) } }(tmpPath) if err := p.imageProc.ResizeImage(tmpPath, width, height, outputPath); err != nil { diff --git a/webdav/handler/method_put.go b/webdav/handler/method_put.go index b8614da7c..3765bea83 100644 --- a/webdav/handler/method_put.go +++ b/webdav/handler/method_put.go @@ -11,6 +11,7 @@ package handler import ( + "errors" "fmt" "io" "net/http" @@ -57,24 +58,23 @@ func (h *Handler) methodPut(w http.ResponseWriter, r *http.Request) { return } outputPath := filepath.Join(os.TempDir(), uuid.New().String()) - ws, err := os.Create(outputPath) //nolint:gosec // Known safe path + //nolint:gosec // Known safe path + file, err := os.Create(outputPath) if err != nil { infra.HandleError(err, w) return } - defer func(ws *os.File) { - err := ws.Close() - if err != nil { + defer func(path string, file *os.File) { + if err := file.Close(); err != nil { infra.HandleError(err, w) } - }(ws) - _, err = io.Copy(ws, r.Body) - if err != nil { - infra.HandleError(err, w) - return - } - err = ws.Close() - if err != nil { + if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { + return + } else if err != nil { + infra.GetLogger().Error(err) + } + }(outputPath, file) + if _, err = io.Copy(file, r.Body); err != nil { infra.HandleError(err, w) return } diff --git a/webdav/main.go b/webdav/main.go index d068725c1..c8ed98302 100644 --- a/webdav/main.go +++ b/webdav/main.go @@ -32,7 +32,7 @@ import ( var ( tokens = make(map[string]*infra.Token) expiries = make(map[string]time.Time) - mu sync.Mutex + mu sync.RWMutex ) func startTokenRefresh(idpClient *client.IdPClient) {