Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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)
Copy link

Choose a reason for hiding this comment

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

logic: passing "80%" as dimensions but downsize method expects either "WIDTHxHEIGHT" format or separate width/height integers - this will likely not work as intended with ImageMagick

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/uploads_controller.rb
Line: 67:67

Comment:
**logic:** passing "80%" as dimensions but `downsize` method expects either "WIDTHxHEIGHT" format or separate width/height integers - this will likely not work as intended with ImageMagick

How can I resolve this? If you propose a fix, please make it concise.

attempt -= 1
end
end
Comment on lines +63 to +70
Copy link

Choose a reason for hiding this comment

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

logic: loop doesn't check if downsizing actually reduced file size - if OptimizedImage.downsize fails or file is already compressed, this creates infinite loop until attempts exhausted

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/uploads_controller.rb
Line: 63:70

Comment:
**logic:** loop doesn't check if downsizing actually reduced file size - if `OptimizedImage.downsize` fails or file is already compressed, this creates infinite loop until attempts exhausted

How can I resolve this? If you propose a fix, please make it concise.


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
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
Copy link

Choose a reason for hiding this comment

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

logic: duplicate method definition - both downsize methods will cause the second to override the first, breaking calls that pass 4 arguments (like in resize_emoji.rb:14)

Suggested change
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.downsize(from, to, dimensions_or_width, height_or_opts=nil, opts={})
if height_or_opts.is_a?(Hash)
# Called with dimensions string: downsize(from, to, "80%", opts)
optimize("downsize", from, to, dimensions_or_width, height_or_opts)
else
# Called with width/height: downsize(from, to, 100, 100, opts)
optimize("downsize", from, to, "#{dimensions_or_width}x#{height_or_opts}", opts)
end
end
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/models/optimized_image.rb
Line: 145:151

Comment:
**logic:** duplicate method definition - both `downsize` methods will cause the second to override the first, breaking calls that pass 4 arguments (like in `resize_emoji.rb:14`)

```suggestion
  def self.downsize(from, to, dimensions_or_width, height_or_opts=nil, opts={})
    if height_or_opts.is_a?(Hash)
      # Called with dimensions string: downsize(from, to, "80%", opts)
      optimize("downsize", from, to, dimensions_or_width, height_or_opts)
    else
      # Called with width/height: downsize(from, to, 100, 100, opts)
      optimize("downsize", from, to, "#{dimensions_or_width}x#{height_or_opts}", opts)
    end
  end
```

How can I resolve this? If you propose a fix, please make it concise.


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