search

sroze / SRIORestUploadBundle

A symfony bundle to handle multiple upload ways on your REST API.
46 stars 17 forks source link

Flysystem integration #23

Closed NinoFloris closed 8 years ago

NinoFloris commented 8 years ago

Ok this is a big PR, but it essentially allows users of this bundle to choose which filesystem abstraction layer they'd like to use.

As we are using Flysystem that is how this PR came to be...

I have tried to keep everything within logical commits and rebased heavily to keep it that way, I hope you are able to review this in the coming weeks.

Enjoy!

P.S. There still is quite a big DOS vulnerability with resumable uploads (and multipart/related in a lesser way), especially with Gaufrette because of its lack of stream support the entire file is read into ram on each chunk upload. What an attacker could very easily do is upload 98 MB of a 100 MB file and thereafter keep sending requests or keep uploading chunks of 1 byte each, which will totally overwhelm the server in ram, compute and transfer time. This could also literally cost you money if it is attached to an expensive storage location with high transfer fees.

I have a local stash which tries to mitigate the ram and transfer time issues by using a storage as a designated 'temp storage' that is used across resumes, that could default to a local folder. This is of course much faster to read and gaufrette and flysystem both have stream support for local files so are therefore able to stream this file, I stream them into a stream copy which can be used for seek + write type of actions. This copying from stream to stream happens in the runtime with 8KB chunks at a time and once the stream reaches 'maxmemory' it overflows into a file on disk. Therefore never reaching more than the allotted memory.

Just let me know if you are interested in such an addition and I'll whip it into shape for a PR!

sroze commented 8 years ago

Indeed, that's a large PR! Can you first run php-cs-fixer on your branch and update the .travis.yml configuration file to run the tests with your matrix of TEST_ENV (that I would rename to TEST_FILESYSTEM) ?

sroze commented 8 years ago

I would definitely be amazed if you can send a (different) PR regarding the DoS vulnerability you are talking about!

NinoFloris commented 8 years ago

Done!

NinoFloris commented 8 years ago

Not meaning to rush you but how is it going?

sroze commented 8 years ago

Thank you @NinoFloris! I'll probably refactor a little bit later that's a already good enough. Really appreciate the hard work 👍