richardgirges / express-fileupload

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

Is it safe to use express-fileupload as a global middleware? #277

Closed leafac closed 3 years ago

leafac commented 3 years ago

The example for express-fileupload installs it as a global middleware, but the documentation for a similar package, Multer, says that at least for that package you shouldn’t use the middleware globally. Hence the question in the title.

As far as I understand the issue is that a route that doesn’t expect files could process files by accident and pollute the temporary directory. Is this not an issue with express-fileupload because it stores the files in memory? If so, could it run into this other issue described in Multer’s MemoryStorage? And how does the useTempFile option affect all this?

Thank you!

tanmaymishu commented 3 years ago

I have the same question about memory usage. Let's say the server has 2GB of memory and I have a 2.1 GB FHD video that I need to upload. What would happen in that case?

tanmaymishu commented 3 years ago

Looks like the memory issue has been addressed in #89. Using useTempFiles option should solve it:

app.use(fileUpload({
    useTempFiles : true,
    tempFileDir : '/tmp/'
}));

Regarding using it as a global middleware, I just tested and found that unhandled/unexpected image uploads will pollute the temporary directory. Maybe per route is the way to go.

leafac commented 3 years ago

I guess the conclusion here is:

If you don’t expect to receive huge files, or a burst of small files, then use memory storage and you may use express-fileupload as a global middleware. That’s the simplest approach.

Otherwise, use file storage and only set express-fileupload as a middleware on routes that expect files and know how to clean up after themselves. That’s the slightly more complicated but more robust approach.

I suppose we may close this.

Thanks for the input, @tanmaymishu!