caktus / django-sticky-uploads

Enhanced file input widget for Django which uploads the file in the background and retains value on form errors.
BSD 3-Clause "New" or "Revised" License
32 stars 9 forks source link

Put files in random directories to prevent filename collisions #19

Closed vstoykov closed 8 years ago

vstoykov commented 8 years ago

Fixes #14

This will also helps in security because will make file name guessing harder.

vstoykov commented 8 years ago

There is an update in progressbar and they dropped Python 3.2 support. This is why tests are failing. Probably #17 need to be merged first and then I need to rebase my branch.

mlavin commented 8 years ago

Yes let's not worry about Python 3.2 support. That should be dropped.

vstoykov commented 8 years ago

Actually in this PR there is no option to place all of the random directories in one directory under temp for easy traversing for old files and empty directories and removing them. This can be another PR.

vstoykov commented 8 years ago

I updated my PR with using mkdtemp. Now I'm looking for the filename key of the response. It is used only for frontend display and not for locating where the file is stored right? This means that I need to change UploadForm.stash method to return os.path.basename('name') for filename. Then on the tests I will not use 'filename': result['filename'].

Are my observations correct or I'm missing sometihng?

mlavin commented 8 years ago

Correct. Stash does the saving and the filename as part of the view response does not matter. It's not used at all. Only the is_valid and stored values are used by the frontend currently.

vstoykov commented 8 years ago

But will be good to show the name of the file in frontend and the value in the ajax response will be good candidate. I will change stash method to allays return basename of the file name in the dict's filename .

mlavin commented 8 years ago

The stash value is used for the hidden input. It isn't displayed to the user. It's the serialized location of the file so that it can be found on the next submission.

vstoykov commented 8 years ago

But from the data in the dict returned by the stash method only stored is used to identify the file right?

mlavin commented 8 years ago

Sorry. I haven't had enough coffee this morning. You are on the right track. Carry on the good work.

vstoykov commented 8 years ago

OK now I think that the code is working as you want.

Cleaning the old files and folders can be in another PR right?

mlavin commented 8 years ago

Yes the cleanup can be another PR. One question but otherwise I think this looks great.

mlavin commented 8 years ago

:sparkling_heart: :sparkling_heart: :sparkling_heart: :sparkling_heart:

mlavin commented 8 years ago

Thank you for all of your hard work and patience!