Image validation and size optimization for Profile image upload from both front-end and back-end #513
Conversation
Co-authored-by: Edward Lee <[email protected]>
| @@ -1 +1 @@ | |||
| python-3.9.13 | |||
| python-3.11.7 | |||
There was a problem hiding this comment.
This is probably going to break things if left as-is, since ex. pyproject.toml and requirements.txt both have restrictions on the python version required for packages.
If you require Python 3.11+, I'd recommend ensuring that #477 is merged in first, and rebase on top of that. Otherwise, I'd recommend sticking to Python 3.9.
| # output from migrations | ||
| - type: bind | ||
| source: ./csm_web/scheduler/migrations/ | ||
| target: /opt/csm_web/csm_web/scheduler/migrations/ |
There was a problem hiding this comment.
This seems to already be in the master branch; perhaps rebase?
| # Check file size | ||
| image_file.seek(0, os.SEEK_END) | ||
| file_size = image_file.tell() | ||
| # reset file pointer | ||
| image_file.seek(0) |
There was a problem hiding this comment.
This is not a good way of validating the file size; these lines will read in the entire file, regardless of how big it is. That means that if a user uploads a 100G file (ex. directly through the HTTP endpoint), you'll still end up reading the entire file in.
I'd recommend changing this to read in from a file stream, and manually stopping the stream when it exceeds the file size limit.
| # not very efficient but way to compress image until it meets the file target size. | ||
| def compress_image(image, target_size_bytes, img_format): | ||
| """Compress the image until it's smaller than target_size_bytes.""" | ||
| buffer = BytesIO() | ||
| quality = 95 # start with high quality | ||
| while True: | ||
| buffer.seek(0) | ||
| if img_format == "JPEG" or img_format == "JPG": | ||
| image.save(buffer, format=img_format, quality=quality) | ||
| else: | ||
| image.save(buffer, format="PNG", optimze=True) | ||
| if buffer.tell() <= target_size_bytes or quality <= 50: | ||
| break | ||
| quality -= 5 # decrease quality to reduce file size | ||
| buffer.seek(0) | ||
| return buffer |
There was a problem hiding this comment.
Is there any particular reason why we're compressing the file? We have a file size limit anyways, which should be sufficient (and can be lowered) if this is for storage concerns.
There was a problem hiding this comment.
There should be tests added for uploading images; in particular one should be added where a request is sent using a file stream that never ends, and ensuring that the request is rejected after only reading in the file size limit.
There was a problem hiding this comment.
These migration files should be merged together at the end prior to merging to master, to reduce on the number of migration files in the repo.
Added validation for extension and file size before reading.
Added validation after reading.
Added file size compression.
Some other considerations:
Things to do: