From 4c1699df6c2a70ac268bf9fb3890fa7af775834d Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Thu, 13 Dec 2018 18:15:28 +0100 Subject: [PATCH] webhook: resulting file was deleted because of defer outside of goroutine --- internal/app/api/api.go | 11 ++++++++++- internal/app/api/html.go | 15 +++++++-------- internal/app/api/markdown.go | 15 +++++++-------- internal/app/api/merge.go | 13 +++++++++---- internal/app/api/office.go | 11 +++++------ internal/app/api/resource.go | 6 +++--- 6 files changed, 41 insertions(+), 30 deletions(-) diff --git a/internal/app/api/api.go b/internal/app/api/api.go index 37652ba3..1ddbbcf0 100644 --- a/internal/app/api/api.go +++ b/internal/app/api/api.go @@ -87,11 +87,12 @@ func newContext(r *resource) (context.Context, context.CancelFunc) { func print(c echo.Context, p printer.Printer, r *resource) error { baseFilename, err := rand.Get() if err != nil { - return fmt.Errorf("getting result file name: %v", err) + return hijackErr(fmt.Errorf("getting result file name: %v", err), r) } filename := fmt.Sprintf("%s.pdf", baseFilename) fpath := fmt.Sprintf("%s/%s", r.dirPath, filename) if r.webhookURL() == "" { + defer r.removeAll() // if no webhook URL given, run conversion // and directly return the resulting PDF file // or an error. @@ -104,6 +105,7 @@ func print(c echo.Context, p printer.Printer, r *resource) error { // run the following lines in a goroutine so that // it doesn't block. go func() { + defer r.removeAll() if err := p.Print(fpath); err != nil { c.Logger().Errorf("%v", err) return @@ -123,3 +125,10 @@ func print(c echo.Context, p printer.Printer, r *resource) error { }() return nil } + +func hijackErr(err error, r *resource) error { + if r != nil { + defer r.removeAll() + } + return err +} diff --git a/internal/app/api/html.go b/internal/app/api/html.go index 1b400b25..48a37e85 100644 --- a/internal/app/api/html.go +++ b/internal/app/api/html.go @@ -8,9 +8,8 @@ import ( func convertHTML(c echo.Context) error { r, err := newResource(c) if err != nil { - return err + return hijackErr(err, r) } - defer r.removeAll() ctx, cancel := newContext(r) if cancel != nil { defer cancel() @@ -18,26 +17,26 @@ func convertHTML(c echo.Context) error { p := &printer.HTML{Context: ctx} indexPath, err := r.filePath("index.html") if err != nil { - return err + return hijackErr(err, r) } p.WithLocalURL(indexPath) headerPath, _ := r.filePath("header.html") if err := p.WithHeaderFile(headerPath); err != nil { - return err + return hijackErr(err, r) } footerPath, _ := r.filePath("footer.html") if err := p.WithFooterFile(footerPath); err != nil { - return err + return hijackErr(err, r) } paperSize, err := r.paperSize() if err != nil { - return err + return hijackErr(err, r) } p.PaperWidth = paperSize[0] p.PaperHeight = paperSize[1] paperMargins, err := r.paperMargins() if err != nil { - return err + return hijackErr(err, r) } p.MarginTop = paperMargins[0] p.MarginBottom = paperMargins[1] @@ -45,7 +44,7 @@ func convertHTML(c echo.Context) error { p.MarginRight = paperMargins[3] landscape, err := r.landscape() if err != nil { - return err + return hijackErr(err, r) } p.Landscape = landscape return print(c, p, r) diff --git a/internal/app/api/markdown.go b/internal/app/api/markdown.go index 8f52e6e8..d34211b4 100644 --- a/internal/app/api/markdown.go +++ b/internal/app/api/markdown.go @@ -8,35 +8,34 @@ import ( func convertMarkdown(c echo.Context) error { r, err := newResource(c) if err != nil { - return err + return hijackErr(err, r) } - defer r.removeAll() ctx, cancel := newContext(r) if cancel != nil { defer cancel() } indexPath, err := r.filePath("index.html") if err != nil { - return err + return hijackErr(err, r) } p := &printer.Markdown{Context: ctx, TemplatePath: indexPath} headerPath, _ := r.filePath("header.html") if err := p.WithHeaderFile(headerPath); err != nil { - return err + return hijackErr(err, r) } footerPath, _ := r.filePath("footer.html") if err := p.WithFooterFile(footerPath); err != nil { - return err + return hijackErr(err, r) } paperSize, err := r.paperSize() if err != nil { - return err + return hijackErr(err, r) } p.PaperWidth = paperSize[0] p.PaperHeight = paperSize[1] paperMargins, err := r.paperMargins() if err != nil { - return err + return hijackErr(err, r) } p.MarginTop = paperMargins[0] p.MarginBottom = paperMargins[1] @@ -44,7 +43,7 @@ func convertMarkdown(c echo.Context) error { p.MarginRight = paperMargins[3] landscape, err := r.landscape() if err != nil { - return err + return hijackErr(err, r) } p.Landscape = landscape return print(c, p, r) diff --git a/internal/app/api/merge.go b/internal/app/api/merge.go index 2f3aa403..6d606715 100644 --- a/internal/app/api/merge.go +++ b/internal/app/api/merge.go @@ -1,6 +1,7 @@ package api import ( + "errors" "fmt" "net/http" "os" @@ -13,20 +14,23 @@ import ( func merge(c echo.Context) error { r, err := newResource(c) if err != nil { - return err + return hijackErr(err, r) } - defer r.removeAll() fpaths, err := r.filePaths([]string{".pdf"}) if err != nil { - return err + return hijackErr(err, r) + } + if len(fpaths) == 0 { + return hijackErr(errors.New("no suitable PDF files to merge"), r) } baseFilename, err := rand.Get() if err != nil { - return fmt.Errorf("getting result file name: %v", err) + return hijackErr(fmt.Errorf("getting result file name: %v", err), r) } filename := fmt.Sprintf("%s.pdf", baseFilename) fpath := fmt.Sprintf("%s/%s", r.dirPath, filename) if r.webhookURL() == "" { + defer r.removeAll() // if no webhook URL given, run merge // and directly return the resulting PDF file // or an error. @@ -39,6 +43,7 @@ func merge(c echo.Context) error { // run the following lines in a goroutine so that // it doesn't block. go func() { + defer r.removeAll() if err := printer.Merge(fpaths, fpath); err != nil { c.Logger().Errorf("%v", err) return diff --git a/internal/app/api/office.go b/internal/app/api/office.go index 15c72ef9..ee6df2f0 100644 --- a/internal/app/api/office.go +++ b/internal/app/api/office.go @@ -24,30 +24,29 @@ var officeExts = []string{ func convertOffice(c echo.Context) error { r, err := newResource(c) if err != nil { - return err + return hijackErr(err, r) } - defer r.removeAll() ctx, cancel := newContext(r) if cancel != nil { defer cancel() } fpaths, err := r.filePaths(officeExts) if err != nil { - return err + return hijackErr(err, r) } if len(fpaths) == 0 { - return errors.New("no suitable office documents to convert") + return hijackErr(errors.New("no suitable office documents to convert"), r) } p := &printer.Office{Context: ctx, FilePaths: fpaths} paperSize, err := r.paperSize() if err != nil { - return err + return hijackErr(err, r) } p.PaperWidth = paperSize[0] p.PaperHeight = paperSize[1] landscape, err := r.landscape() if err != nil { - return err + return hijackErr(err, r) } p.Landscape = landscape return print(c, p, r) diff --git a/internal/app/api/resource.go b/internal/app/api/resource.go index c02e6033..c4854121 100644 --- a/internal/app/api/resource.go +++ b/internal/app/api/resource.go @@ -49,17 +49,17 @@ func newResource(c echo.Context) (*resource, error) { r := &resource{values: v, dirPath: dirPath} form, err := c.MultipartForm() if err != nil { - return nil, fmt.Errorf("getting multipart form: %v", err) + return r, fmt.Errorf("getting multipart form: %v", err) } for _, files := range form.File { for _, fh := range files { in, err := fh.Open() if err != nil { - return nil, fmt.Errorf("%s: opening file: %v", fh.Filename, err) + return r, fmt.Errorf("%s: opening file: %v", fh.Filename, err) } defer in.Close() if err := r.writeFile(fh.Filename, in); err != nil { - return nil, err + return r, err } } }