From fab93718055121673be5b2d267a129f128eb771a Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Tue, 20 Sep 2022 12:34:19 +0200 Subject: [PATCH] Fix unclosed file-handles (attachments) #410 --- compose/automation/attachment_handler.go | 10 ++++++++-- compose/rest/attachment.go | 4 +++- compose/service/attachment.go | 8 ++++---- pkg/objstore/interfaces.go | 2 +- pkg/objstore/minio/store.go | 2 +- pkg/objstore/plain/store.go | 2 +- system/rest/attachment.go | 4 +++- system/service/attachment.go | 8 ++++---- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/compose/automation/attachment_handler.go b/compose/automation/attachment_handler.go index 84db2e25f..8494502f7 100644 --- a/compose/automation/attachment_handler.go +++ b/compose/automation/attachment_handler.go @@ -18,8 +18,8 @@ type ( FindByID(ctx context.Context, namespaceID, attachmentID uint64) (*types.Attachment, error) CreateRecordAttachment(ctx context.Context, namespaceID uint64, name string, size int64, fh io.ReadSeeker, moduleID, recordID uint64, fieldName string) (att *types.Attachment, err error) DeleteByID(ctx context.Context, namespaceID uint64, attachmentID uint64) error - OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) - OpenPreview(att *types.Attachment) (io.ReadSeeker, error) + OpenOriginal(att *types.Attachment) (io.ReadSeekCloser, error) + OpenPreview(att *types.Attachment) (io.ReadSeekCloser, error) } attachmentHandler struct { @@ -64,6 +64,9 @@ func (h attachmentHandler) openOriginal(ctx context.Context, args *attachmentOpe return nil, err } + // @todo we need to call Close() when file is read (or at the end of the workflow) + // some kind of workflow-cleanup facility is needed + return r, nil } @@ -79,6 +82,9 @@ func (h attachmentHandler) openPreview(ctx context.Context, args *attachmentOpen return nil, err } + // @todo we need to call Close() when file is read (or at the end of the workflow) + // some kind of workflow-cleanup facility is needed + return r, nil } diff --git a/compose/rest/attachment.go b/compose/rest/attachment.go index 16c3d4286..821947e52 100644 --- a/compose/rest/attachment.go +++ b/compose/rest/attachment.go @@ -123,7 +123,7 @@ func (ctrl Attachment) serve(ctx context.Context, namespaceID, attachmentID uint return } - var fh io.ReadSeeker + var fh io.ReadSeekCloser if preview { fh, err = ctrl.attachment.OpenPreview(att) @@ -136,6 +136,8 @@ func (ctrl Attachment) serve(ctx context.Context, namespaceID, attachmentID uint return } + defer fh.Close() + name := url.QueryEscape(att.Name) if download { diff --git a/compose/service/attachment.go b/compose/service/attachment.go index 90f9dba5d..7b9e13190 100644 --- a/compose/service/attachment.go +++ b/compose/service/attachment.go @@ -61,8 +61,8 @@ type ( CreatePageAttachment(ctx context.Context, namespaceID uint64, name string, size int64, fh io.ReadSeeker, pageID uint64) (*types.Attachment, error) CreateRecordAttachment(ctx context.Context, namespaceID uint64, name string, size int64, fh io.ReadSeeker, moduleID, recordID uint64, fieldName string) (*types.Attachment, error) CreateNamespaceAttachment(ctx context.Context, name string, size int64, fh io.ReadSeeker) (*types.Attachment, error) - OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) - OpenPreview(att *types.Attachment) (io.ReadSeeker, error) + OpenOriginal(att *types.Attachment) (io.ReadSeekCloser, error) + OpenPreview(att *types.Attachment) (io.ReadSeekCloser, error) DeleteByID(ctx context.Context, namespaceID, attachmentID uint64) error } ) @@ -231,7 +231,7 @@ func (svc attachment) DeleteByID(ctx context.Context, namespaceID, attachmentID // return r, nil //} -func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) { +func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeekCloser, error) { if len(att.Url) == 0 { return nil, nil } @@ -239,7 +239,7 @@ func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) return svc.objects.Open(att.Url) } -func (svc attachment) OpenPreview(att *types.Attachment) (io.ReadSeeker, error) { +func (svc attachment) OpenPreview(att *types.Attachment) (io.ReadSeekCloser, error) { if len(att.PreviewUrl) == 0 { return nil, nil } diff --git a/pkg/objstore/interfaces.go b/pkg/objstore/interfaces.go index 03df562db..463048f99 100644 --- a/pkg/objstore/interfaces.go +++ b/pkg/objstore/interfaces.go @@ -19,7 +19,7 @@ type Store interface { Remove(filename string) error // Open returns file handle - Open(filename string) (io.ReadSeeker, error) + Open(filename string) (io.ReadSeekCloser, error) // Healthcheck checks health status of the store Healthcheck(ctx context.Context) error diff --git a/pkg/objstore/minio/store.go b/pkg/objstore/minio/store.go index ff75079b0..de983fa90 100644 --- a/pkg/objstore/minio/store.go +++ b/pkg/objstore/minio/store.go @@ -132,7 +132,7 @@ func (s store) Remove(name string) error { return s.mc.RemoveObject(s.bucket, s.getObjectName(name)) } -func (s store) Open(name string) (io.ReadSeeker, error) { +func (s store) Open(name string) (io.ReadSeekCloser, error) { return s.mc.GetObject(s.bucket, s.getObjectName(name), minio.GetObjectOptions{ ServerSideEncryption: s.sse, }) diff --git a/pkg/objstore/plain/store.go b/pkg/objstore/plain/store.go index d6e8a54db..7c1aead0e 100644 --- a/pkg/objstore/plain/store.go +++ b/pkg/objstore/plain/store.go @@ -90,7 +90,7 @@ func (s *store) Remove(filename string) error { return s.fs.Remove(filename) } -func (s *store) Open(filename string) (io.ReadSeeker, error) { +func (s *store) Open(filename string) (io.ReadSeekCloser, error) { // check filename for validity if err := s.check(filename); err != nil { return nil, err diff --git a/system/rest/attachment.go b/system/rest/attachment.go index 013d74205..c1f0ab456 100644 --- a/system/rest/attachment.go +++ b/system/rest/attachment.go @@ -110,7 +110,7 @@ func (ctrl Attachment) serve(ctx context.Context, attachmentID uint64, preview, return } - var fh io.ReadSeeker + var fh io.ReadSeekCloser if preview { fh, err = ctrl.attachment.OpenPreview(att) @@ -118,6 +118,8 @@ func (ctrl Attachment) serve(ctx context.Context, attachmentID uint64, preview, fh, err = ctrl.attachment.OpenOriginal(att) } + defer fh.Close() + if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/system/service/attachment.go b/system/service/attachment.go index dc5d847ff..3e23bfb36 100644 --- a/system/service/attachment.go +++ b/system/service/attachment.go @@ -43,8 +43,8 @@ type ( Find(ctx context.Context, filter types.AttachmentFilter) (types.AttachmentSet, types.AttachmentFilter, error) CreateSettingsAttachment(ctx context.Context, name string, size int64, fh io.ReadSeeker, labels map[string]string) (*types.Attachment, error) CreateApplicationAttachment(ctx context.Context, name string, size int64, fh io.ReadSeeker, labels map[string]string) (*types.Attachment, error) - OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) - OpenPreview(att *types.Attachment) (io.ReadSeeker, error) + OpenOriginal(att *types.Attachment) (io.ReadSeekCloser, error) + OpenPreview(att *types.Attachment) (io.ReadSeekCloser, error) DeleteByID(ctx context.Context, ID uint64) error } ) @@ -116,7 +116,7 @@ func (svc attachment) Find(ctx context.Context, filter types.AttachmentFilter) ( return aa, f, svc.recordAction(ctx, aaProps, AttachmentActionSearch, err) } -func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) { +func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeekCloser, error) { if len(att.Url) == 0 { return nil, nil } @@ -124,7 +124,7 @@ func (svc attachment) OpenOriginal(att *types.Attachment) (io.ReadSeeker, error) return svc.files.Open(att.Url) } -func (svc attachment) OpenPreview(att *types.Attachment) (io.ReadSeeker, error) { +func (svc attachment) OpenPreview(att *types.Attachment) (io.ReadSeekCloser, error) { if len(att.PreviewUrl) == 0 { return nil, nil }