fix: Explicitly cast upload memory size config values to integers #1138
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #1138 +/- ##
=======================================
Coverage 96.07% 96.07%
=======================================
Files 837 837
Lines 19856 19856
=======================================
Hits 19076 19076
Misses 780 780
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
|
@codecov-ai-reviewer review |
|
On it! We are reviewing the PR and will provide feedback shortly. |
![codecov-ai[bot]](https://avatars.githubusercontent.com/in/797565?s=60&v=4){kind=small}
|
|
||
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "upload_max_memory_size", default=2621440 | ||
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "upload_max_memory_size", default=2621440) |
There was a problem hiding this comment.
Good change. Converting the configuration value to an integer ensures type consistency and prevents potential issues with non-integer values. However, consider adding error handling in case the conversion fails.
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "upload_max_memory_size", default=2621440 | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| ) | |
| try: | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| ) | |
| except ValueError: | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = 2621440 # Default value if conversion fails | |
| logging.warning("Invalid value for DATA_UPLOAD_MAX_MEMORY_SIZE. Using default.") |
| ) | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "file_upload_max_memory_size", default=2621440 | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) |
There was a problem hiding this comment.
Similar to the previous comment, this is a good change for type consistency. Consider applying the same error handling approach here as well.
| ) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "file_upload_max_memory_size", default=2621440 | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| ) | |
| try: | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| ) | |
| except ValueError: | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = 2621440 # Default value if conversion fails | |
| logging.warning("Invalid value for FILE_UPLOAD_MAX_MEMORY_SIZE. Using default.") |
|
|
||
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "upload_max_memory_size", default=2621440 | ||
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | ||
| ) | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "file_upload_max_memory_size", default=2621440 | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) |
There was a problem hiding this comment.
Consider adding a comment explaining why these values are being explicitly converted to integers. This will help future developers understand the reasoning behind the change.
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "upload_max_memory_size", default=2621440 | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| ) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "file_upload_max_memory_size", default=2621440 | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| # Explicitly convert upload size limits to integers to ensure type consistency | |
| # and prevent issues with non-integer values from configuration | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| ) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| ) |
|
|
||
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "upload_max_memory_size", default=2621440 | ||
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | ||
| ) | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | ||
| "setup", "http", "file_upload_max_memory_size", default=2621440 | ||
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | ||
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) |
There was a problem hiding this comment.
These configuration values are critical for controlling upload sizes. Consider adding validation to ensure the values are within an acceptable range to prevent potential security issues or resource exhaustion.
| DATA_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "upload_max_memory_size", default=2621440 | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| ) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = get_config( | |
| "setup", "http", "file_upload_max_memory_size", default=2621440 | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| def validate_upload_size(size, min_size=1024, max_size=10485760): | |
| if min_size <= size <= max_size: | |
| return size | |
| return max_size # Or min_size, depending on your preference | |
| DATA_UPLOAD_MAX_MEMORY_SIZE = validate_upload_size(int( | |
| get_config("setup", "http", "upload_max_memory_size", default=2621440) | |
| )) | |
| FILE_UPLOAD_MAX_MEMORY_SIZE = validate_upload_size(int( | |
| get_config("setup", "http", "file_upload_max_memory_size", default=2621440) | |
| )) |
Purpose/Motivation
What is the feature? Why is this being done?
Links to relevant tickets
What does this PR do?
Include a brief description of the changes in this PR. Bullet points are your friend.
Notes to Reviewer
Anything to note to the team? Any tips on how to review, or where to start?
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.