Skip to content

FEATURE: automatically downsize large images #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ShashankFC wants to merge 1 commit into
base: image-processing-optimization
Choose a base branch
from
Open

FEATURE: automatically downsize large images #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/assets/javascripts/discourse/lib/utilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ Discourse.Utilities = {

// check file size
var fileSizeKB = file.size / 1024;
var maxSizeKB = Discourse.SiteSettings['max_' + type + '_size_kb'];
var maxSizeKB = 10 * 1024; // 10MB
if (fileSizeKB > maxSizeKB) {
bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
return false;
Expand Down Expand Up @@ -243,7 +243,7 @@ Discourse.Utilities = {

// entity too large, usually returned from the web server
case 413:
var maxSizeKB = Discourse.SiteSettings.max_image_size_kb;
var maxSizeKB = 10 * 1024; // 10 MB
bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
return;

Expand Down
11 changes: 10 additions & 1 deletion app/controllers/uploads_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,23 @@ def create_upload(type, file, url)
begin
# API can provide a URL
if file.nil? && url.present? && is_api?
tempfile = FileHelper.download(url, SiteSetting.max_image_size_kb.kilobytes, "discourse-upload-#{type}") rescue nil
tempfile = FileHelper.download(url, 10.megabytes, "discourse-upload-#{type}") rescue nil
filename = File.basename(URI.parse(url).path)
else
tempfile = file.tempfile
filename = file.original_filename
content_type = file.content_type
end

# 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
Comment on lines +63 to +70

@coderabbitai coderabbitai Bot Jan 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does ImageMagick convert -resize accept percentage values like "80%"?

💡 Result:

Yes. -resize accepts percentage values (e.g. magick input.png -resize 80% output.png). [1][2]

Note: on Windows batch files you must escape % by doubling it (use %%). [1]

🏁 Script executed:

# Search for OptimizedImage.downsize implementation
fd -type f -name "*.rb" | xargs rg -l "OptimizedImage" | head -20

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 226

🏁 Script executed:

# Find the downsize method definition
rg "def downsize" -A 15

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 67

Improve error handling and file size tracking in the downsize loop.

The downsizing logic has several issues:

  1. No error handling: If OptimizedImage.downsize fails, the loop continues without logging or indication of failure.

  2. File size not refreshed after modification: tempfile.size may not reflect the updated file size after in-place modification. Use File.size(tempfile.path) to get the current file size on disk.

  3. In-place file modification: Using the same path for source and destination carries inherent risk if the operation fails mid-write, potentially corrupting the file.

The "80%" dimension string is a valid ImageMagick resize format and will correctly reduce dimensions by 20%.

🐛 Proposed fix 
       # 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)
+        while attempt > 0 && File.size(tempfile.path) > SiteSetting.max_image_size_kb.kilobytes
+          unless OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)
+            Rails.logger.warn("Failed to downsize image #{filename}, attempt #{6 - attempt}")
+            break
+          end
           attempt -= 1
         end
       end


upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)

@coderabbitai coderabbitai Bot Jan 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

File size passed to Upload.create_for may be stale after downsizing.

After the downsizing loop modifies the file, tempfile.size may not reflect the new size. Use File.size(tempfile.path) to get the accurate post-downsize file size.

🐛 Proposed fix 
-      upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
+      upload = Upload.create_for(current_user.id, tempfile, filename, File.size(tempfile.path), content_type: content_type, image_type: type)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
upload = Upload.create_for(current_user.id, tempfile, filename, File.size(tempfile.path), content_type: content_type, image_type: type)
🤖 Prompt for AI Agents 
In @app/controllers/uploads_controller.rb at line 72, The call to
Upload.create_for uses tempfile.size which may be stale after the downsizing
loop; replace that argument with the actual on-disk size by calling
File.size(tempfile.path) (and ensure the tempfile is flushed/rewound if needed)
when invoking Upload.create_for(current_user.id, tempfile, filename,
File.size(tempfile.path), content_type: content_type, image_type: type) so the
stored size reflects the post-downsize file; locate the call to
Upload.create_for in the uploads controller and update the size parameter
accordingly.


if upload.errors.empty? && current_user.admin?
Expand Down
17 changes: 8 additions & 9 deletions app/models/optimized_image.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,25 +139,24 @@ def self.downsize_instructions_animated(from, to, dimensions, opts={})
end

def self.resize(from, to, width, height, opts={})
optimize("resize", from, to, width, height, opts)
optimize("resize", from, to, "#{width}x#{height}", opts)
end

def self.downsize(from, to, max_width, max_height, opts={})
optimize("downsize", from, to, max_width, max_height, opts)
optimize("downsize", from, to, "#{max_width}x#{max_height}", opts)
end

def self.optimize(operation, from, to, width, height, opts={})
dim = dimensions(width, height)
def self.downsize(from, to, dimensions, opts={})
optimize("downsize", from, to, dimensions, opts)
end

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, dim, opts)
instructions = self.send(method_name.to_sym, from, to, dimensions, opts)
convert_with(instructions, to)
end

def self.dimensions(width, height)
"#{width}x#{height}"
end

def self.convert_with(instructions, to)
`#{instructions.join(" ")} &> /dev/null`
return false if $?.exitstatus != 0
Expand Down