Skip to content

FEATURE: automatically downsize large images #10

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
akshayutture-augment wants to merge 1 commit into
base: image-processing-optimization
Choose a base branch
from
Open

FEATURE: automatically downsize large images #10

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;
Comment on lines 245 to 248

@coderabbitai coderabbitai Bot Nov 21, 2025

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

Wrap the variable declaration in a block to prevent access from other switch clauses.

JavaScript case clauses don't create block scope, so the maxSizeKB variable could be accessed by other cases, potentially causing issues.

Based on static analysis.

Apply this diff:

   // entity too large, usually returned from the web server
-  case 413:
+  case 413: {
     var maxSizeKB = 10 * 1024; // 10 MB
     bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
     return;
+  }
📝 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
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;
case 413: {
var maxSizeKB = 10 * 1024; // 10 MB
bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
return;
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 246-246: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🤖 Prompt for AI Agents 
In app/assets/javascripts/discourse/lib/utilities.js around lines 245 to 248,
the case 413 clause declares maxSizeKB in function/global scope because case
clauses don’t create block scope; wrap the case body in a block ({ ... }) so
maxSizeKB is scoped to that case only (e.g. replace the raw statements with a
braced block containing the declaration, alert call, and 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 Nov 21, 2025

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 | 🔴 Critical

Critical: File size not refreshed after downsampling.

The tempfile.size attribute is cached and won't reflect the new file size after OptimizedImage.downsize modifies the file on disk. This means the while condition will always evaluate based on the original file size, causing the loop to run exactly 5 times regardless of whether downsampling succeeds or reduces the file size.

Additionally, there's no check for whether downsize succeeds (it returns a boolean).

Apply this diff to 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)
+    success = OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)
+    break unless success
+    # Reload file size from disk after modification
+    tempfile = File.open(tempfile.path)
     attempt -= 1
   end
 end
📝 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
# 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
# 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
success = OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)
break unless success
# Reload file size from disk after modification
tempfile = File.open(tempfile.path)
attempt -= 1
end
end


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

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
Comment on lines 145 to +151

@coderabbitai coderabbitai Bot Nov 21, 2025

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 | 🔴 Critical

🧩 Analysis chain

Critical: Method overloading creates a breaking change.

Ruby does not support method overloading. The second downsize definition at line 149 completely replaces the first one at line 145, meaning the 4-parameter signature downsize(from, to, max_width, max_height, opts={}) no longer exists.

Any existing code calling downsize with 4 positional parameters (width and height) will fail with an ArgumentError.

Based on static analysis.

Run the following script to find all callers of the downsize method and verify they use the correct signature:

Consider either:

  1. Renaming one of the methods (e.g., downsize_by_dimensions for the string version)
  2. Using Ruby's flexible argument handling to support both signatures:
def self.downsize(from, to, width_or_dimensions, height_or_opts={}, opts={})
  if width_or_dimensions.is_a?(String)
    # New API: dimensions string
    dimensions = width_or_dimensions
    opts = height_or_opts
  else
    # Old API: separate width/height
    dimensions = "#{width_or_dimensions}x#{height_or_opts}"
  end
  optimize("downsize", from, to, dimensions, opts)
end

🏁 Script executed:

#!/bin/bash
# Description: Find all calls to OptimizedImage.downsize to verify compatibility

# Search for downsize method calls
rg -n -C3 -P '\.downsize\s*\(' --type=rb

Length of output: 26

Let me retry with a different approach to search for downsize method calls:

🏁 Script executed:

#!/bin/bash
# Find all .rb files and search for downsize calls
fd '\.rb$' -exec rg -l 'downsize' {} \;

Length of output: 241

🏁 Script executed:

#!/bin/bash
# Search for downsize method calls with context (no file type filter)
rg -n -C3 '\.downsize\s*\(' --include='*.rb'

Length of output: 441

🏁 Script executed:

#!/bin/bash
# Examine the actual optimized_image.rb file to confirm the duplicate definitions
cat app/models/optimized_image.rb | sed -n '140,155p'

Length of output: 559

🏁 Script executed:

#!/bin/bash
# Simple search for downsize calls - try basic pattern
rg 'downsize' -n

Length of output: 839

🏁 Script executed:

#!/bin/bash
# Search specifically in Ruby files for downsize calls with simpler pattern
find . -name "*.rb" -type f | head -20

Length of output: 971

Critical: Duplicate method definition breaks existing caller in resize_emoji.rb.

The second downsize definition (line 149) completely replaces the first (line 145) in Ruby. However, app/jobs/regular/resize_emoji.rb:14 calls OptimizedImage.downsize(path, path, 100, 100, opts) using the 4-parameter signature that no longer exists.

This call will fail with ArgumentError because the remaining definition expects (from, to, dimensions, opts), not (from, to, max_width, max_height, opts).

Fix: Either:

  1. Rename one method (e.g., downsize_by_dimensions for the string version)
  2. Use a unified signature that accepts both formats:
def self.downsize(from, to, width_or_dimensions, height_or_opts={}, opts={})
  if width_or_dimensions.is_a?(String)
    dimensions = width_or_dimensions
    opts = height_or_opts
  else
    dimensions = "#{width_or_dimensions}x#{height_or_opts}"
  end
  optimize("downsize", from, to, dimensions, opts)
end

Update both call sites: app/controllers/uploads_controller.rb:67 and app/jobs/regular/resize_emoji.rb:14.

🧰 Tools
🪛 RuboCop (1.81.7)

[warning] 149-149: Method OptimizedImage.downsize is defined at both app/models/optimized_image.rb:145 and app/models/optimized_image.rb:149.

(Lint/DuplicateMethods)

🤖 Prompt for AI Agents 
In app/models/optimized_image.rb around lines 145 to 151, there are two
conflicting downsize method definitions causing the 4-argument call in
app/jobs/regular/resize_emoji.rb to raise ArgumentError; replace them with a
single unified downsize that accepts either (from, to, "WxH"/dimensions_string,
opts) or (from, to, width, height, opts) by detecting if the third param is a
String and normalizing to a dimensions string before calling optimize, and then
update the two call sites (app/jobs/regular/resize_emoji.rb:14 and
app/controllers/uploads_controller.rb:67) to use the unified signature (no
behavior change expected) so both string and numeric width/height invocations
work.


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