richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 259 forks source link

when using useTempFiles: true, setting, cleanup is not exeuted after fileupload is completed. #335

Closed Romkin7 closed 10 months ago

Romkin7 commented 1 year ago

https://github.com/richardgirges/express-fileupload/blob/4f81fc822d385e3e404a3266a76c103f543c3a2a/lib/tempFileHandler.js#L53

elliotsayes commented 1 year ago

Yes and the FileArray does not support iterating through the files to clean them up yourself!

So useTempFiles: true is completely unusable in production without leaking disk space to unexpected file keys 🤡

Could someone e.g. @richardgirges please look at this?

lamberto15 commented 1 year ago

for now just add a trycatch with finally, then add the logic here to clean if tmp/upload has file, dont forget to add logic or else other user that has been upload concurrent will also deleted.

RomanBurunkov commented 10 months ago

https://github.com/richardgirges/express-fileupload/blob/4f81fc822d385e3e404a3266a76c103f543c3a2a/lib/tempFileHandler.js#L53

Why it should be executed after upload completion? This function should be executed in case of errors or abortions.

then it is up to the application how to use temporary files.

RomanBurunkov commented 10 months ago

Yes and the FileArray does not support iterating through the files to clean them up yourself!

So useTempFiles: true is completely unusable in production without leaking disk space to unexpected file keys 🤡

Could someone e.g. @richardgirges please look at this?

For what? You get data with uploaded files, then it is time to work with them by your own. Middleware just do what it should: uploads files and provides them through request object for further processing.

RomanBurunkov commented 10 months ago

Closing, since cleaning up successfully uploaded files is a responsibility of the app, not a middleware.