FEATURE: automatically downsize large images

[hussam789]

User description

PR #1

---

PR Type

Enhancement

---

Description

• Automatically downsize large images to allowed size during upload

• Refactor OptimizedImage methods to use unified dimensions format

• Increase client-side upload limit to 10MB for better UX

• Add retry logic for image downsizing with up to 5 attempts

---

Diagram Walkthrough

flowchart LR
  A["User uploads image"] --> B["Check if image exceeds limit"]
  B --> C["Downsize image to 80%"]
  C --> D["Retry up to 5 times if needed"]
  D --> E["Create upload with optimized image"]

Loading

File Walkthrough

Relevant files

Enhancement

uploads_controller.rb Add automatic image downsizing on upload                                  app/controllers/uploads_controller.rb Increased temporary download size limit from SiteSetting.max_image_size_kb to 10MB Added automatic image downsizing logic that reduces images to 80% size Implements retry mechanism (up to 5 attempts) to downsize images exceeding max allowed size Only applies downsizing to valid image files when size exceeds configured limit +10/-1    optimized_image.rb Refactor image optimization methods for consistency            app/models/optimized_image.rb Refactored resize and downsize methods to format dimensions as string before passing to optimize Added new downsize method overload accepting pre-formatted dimensions string Simplified optimize method to accept dimensions as single string parameter Removed dimensions helper method as formatting is now done inline +8/-9      utilities.js Increase client-side upload size limit to 10MB                      app/assets/javascripts/discourse/lib/utilities.js Hardcoded client-side max file size to 10MB instead of using site setting Updated both file validation check and error handling to use fixed 10MB limit Allows larger files to be uploaded for server-side processing and downsizing +2/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Client/server limit mismatch Description: Hardcoding a 10MB client-side upload limit instead of using server-configured settings can lead to mismatch with backend limits, causing user-facing errors and potential resource strain if server limits are higher or lower. utilities.js [182-247] Referred Code var maxSizeKB = 10 * 1024; // 10MB if (fileSizeKB > maxSizeKB) { bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB })); return false; } // everything went fine return true; }, authorizesAllExtensions: function() { return Discourse.SiteSettings.authorized_extensions.indexOf("*") >= 0; }, isAuthorizedUpload: function(file) { if (file && file.name) { var extensions = _.chain(Discourse.SiteSettings.authorized_extensions.split("|")) .reject(function(extension) { return extension.indexOf("*") >= 0; }) .map(function(extension) { return (extension.indexOf(".") === 0 ? extension.substring(1) : extension).replace(".", "\\."); }) .value(); return new RegExp("\\.(" + extensions.join("|") + ")$", "i").test(file.name); ... (clipped 45 lines)
	Command injection risk Description: Image processing uses shell backticks to run commands built from instructions; if any part of the instruction can be influenced by user-controlled filenames or paths, this may enable command injection. optimized_image.rb [153-161] Referred Code def self.optimize(operation, from, to, dimensions, opts={}) method_name = "#{operation}_instructions" method_name += "_animated" if !!opts[:allow_animation] && from =~ /\.GIF$/i instructions = self.send(method_name.to_sym, from, to, dimensions, opts) convert_with(instructions, to) end def self.convert_with(instructions, to) `#{instructions.join(" ")} &> /dev/null`
	Resource exhaustion risk Description: Repeatedly downsizing the same tempfile in place without validating success or enforcing a minimum dimension may allow excessively large images to bypass limits or cause excessive processing time (DoS risk). uploads_controller.rb [63-70] Referred Code # allow users to upload large images that will be automatically reduced to allowed size if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end end
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Silent failures: The downsizing loop lacks error handling and result checks for optimize/convert failures and does not guard against no-op downsizing or invalid tempfiles, risking silent failures and infinite large-file uploads. Referred Code # allow users to upload large images that will be automatically reduced to allowed size if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end end upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: The new automatic downsizing and retry behavior for uploads adds critical actions without any corresponding audit logging of user ID, action, outcome, or attempts. Referred Code # allow users to upload large images that will be automatically reduced to allowed size if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end end upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Fixed client limit: Hardcoding a 10MB client-side limit may bypass server-configured constraints and lacks validation that aligns with server settings, potentially weakening input controls. Referred Code var fileSizeKB = file.size / 1024; var maxSizeKB = 10 * 1024; // 10MB if (fileSizeKB > maxSizeKB) { bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB })); return false; }

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Implement a more efficient, single-step downsizing strategy Replace the inefficient, iterative image downsizing loop with a more robust, single-step approach. This involves calculating the precise size reduction needed and applying it in one operation. Examples: app/controllers/uploads_controller.rb [64-70] if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end end Solution Walkthrough: Before: # app/controllers/uploads_controller.rb if tempfile && tempfile.size > SiteSetting.max_image_size_kb.kilobytes attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes # Repeatedly downsize by a fixed 20% OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", ...) attempt -= 1 end end upload = Upload.create_for(...) After: # app/controllers/uploads_controller.rb if tempfile && tempfile.size > SiteSetting.max_image_size_kb.kilobytes # 1. Get image dimensions and calculate target size current_size = tempfile.size target_size = SiteSetting.max_image_size_kb.kilobytes ratio = Math.sqrt(target_size.to_f / current_size.to_f) # ... logic to get original width/height ... new_width = (original_width * ratio).to_i new_height = (original_height * ratio).to_i # 2. Downsize in a single operation OptimizedImage.downsize(tempfile.path, tempfile.path, new_width, new_height, ...) end upload = Upload.create_for(...) Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant performance and reliability flaw in the core logic of the PR, proposing a much more efficient and robust single-step downsizing approach.	High
Possible issue	Handle failed image downsizing attempts After the image downsizing loop, add a check to ensure the file size is within the allowed limit. If it's still too large, return a specific error to the user instead of proceeding with the upload. app/controllers/uploads_controller.rb [63-72] # allow users to upload large images that will be automatically reduced to allowed size if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end + + if tempfile.size > SiteSetting.max_image_size_kb.kilobytes + return render_json_error(I18n.t("upload.errors.file_is_still_too_large", { max_size_kb: SiteSetting.max_image_size_kb })) + end end upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a logical flaw in the new image downsizing feature where an image that is still too large after all downsizing attempts would proceed, likely causing a less specific validation error later. Implementing this improves error handling and provides better user feedback.	Medium
General	Ensure fresh file size check In the downsizing while loop, use File.size(tempfile.path) instead of tempfile.size to ensure the most up-to-date file size is checked, avoiding potential caching issues. app/controllers/uploads_controller.rb [63-70] # allow users to upload large images that will be automatically reduced to allowed size if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) attempt = 5 - while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes + while attempt > 0 && File.size(tempfile.path) > SiteSetting.max_image_size_kb.kilobytes OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) attempt -= 1 end end Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion to use File.size(tempfile.path) instead of tempfile.size is a defensive coding practice against potential file size caching issues. While tempfile.size should work correctly, this change makes the code more robust and explicit without adding complexity.	Low
More