Unlink old(previous) profile images #1415
Comments
|
This is something that has crossed my mind as well. I think we have a couple options:
The latter may be a less complicated implementation, but could have security implications when we're using the same file name over long periods of time (or for the lifetime of the profile image). However, this is just a profile image that isn't a private asset. |
|
IMHO both options are fine. However, I think the first one will be easier to implement and with less changes to existing code. Something like that:
|
|
@aanev Your proposed solution is fine, and would work. Most likely requiring just 4 lines of code in this method. However, I'd like to see this done in a more configurable manner, and the actual deletion abstracted away from the User's profile server controller. Whether or not to delete, or overwrite, the existing profile picture seems to be more of a business requirement that may vary among this framework's users. It would be nice to provide an option for allowing our users full control over this. |
|
@aanev good catch. I think we should simplify this, whether we overwrite an existing file name, or create a new one, both are ok. Is anyone getting a PR proposed? |
|
@aanev Would you like to submit a PR for this? If we opted for overwriting the existing file by using the same name, we wouldn't have a need to update the database. I fiddled around with this idea, and wasn't able to change the file name before upload on the client-side. I'm not sure if this is a limitation of angular-file-upload, or with our implementation of it. The solution in this comment didn't work for me. We could always remove the old file, and rename the new one on the server-side after the successful upload; we wouldn't need to update the database in this case either. Just food for thought :) Either option we go with, we can keep it simple as @lirantal suggested. |
* fix(user): fix changeProfilePicture * use promises to simplify callbacks * use fs.unlink to delete old picture once the profile is updated Fixes #1415 * fix(user): fix changeProfilePicture * use promises to simplify callbacks * use fs.unlink to delete old picture once the profile is updated * log file errors to console Fixes #1415 * fix(user): fix changeProfilePicture * use promises to simplify callbacks * use fs.unlink to delete old picture once the profile is updated * log file errors to console * update error handler module to handle file upload errors Fixes #1415 * fix(user): fix changeProfilePicture * use promises to simplify callbacks * use fs.unlink to delete old picture once the profile is updated * log file errors to console * update error handler module to handle file upload errors Fixes #1415
Currently old profile images are not unlinked upon change and remain on disk. If a user changes his/her profile image frequently that can lead to disk space leakage.
The text was updated successfully, but these errors were encountered: