Defect #20278

Wrong syntax for resizing inline images will throw a 500 error

Added by Michael Skrynski over 2 years ago. Updated about 2 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Text formatting
Target version:3.1.1
Resolution:Fixed Affected version:

Description

Given the following Textile code:

|_.Text|
|!width:50%FE-Image.jpg!|

will throw a 500 error in Redmine with an attachment file called FE-Image.jpg after saving.

ActionView::Template::Error
invalid byte sequence in UTF-8

Stacktrace (most recent call first):

  attachment.rb:259:in `downcase'
    |att| att.filename.downcase == filename.downcase
  attachment.rb:259:in `block in latest_attach'
    |att| att.filename.downcase == filename.downcase
  attachment.rb:258:in `each'
    attachments.sort_by(&:created_on).reverse.detect {
  attachment.rb:258:in `detect'
    attachments.sort_by(&:created_on).reverse.detect {
  attachment.rb:258:in `latest_attach'
    attachments.sort_by(&:created_on).reverse.detect {
  application_helper.rb:657:in `block in parse_inline_attachments'
    if found = Attachment.latest_attach(attachments, CGI.unescape(filename))
  application_helper.rb:654:in `gsub!'
    text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png))"(\s+alt="([^"]*)")?/i) do |m|
  application_helper.rb:654:in `parse_inline_attachments'
    text.gsub!(/src="([^\/"]+\.(bmp|gif|jpg|jpe|jpeg|png))"(\s+alt="([^"]*)")?/i) do |m|
  application_helper.rb:605:in `block (2 levels) in textilizable'
    send method_name, text, project, obj, attr, only_path, options
  application_helper.rb:604:in `each'
    [:parse_inline_attachments, :parse_wiki_links, :parse_redmine_links].each do |method_name|
  application_helper.rb:604:in `block in textilizable'
    [:parse_inline_attachments, :parse_wiki_links, :parse_redmine_links].each do |method_name|
  application_helper.rb:625:in `parse_non_pre_blocks'
    yield text

Using the correct syntax:

|_.Text|
|!{width:50%}FE-Image.jpg!|

will display the correct image downsized to 50%.
Is there a way to catch such user-generated error?


Related issues

Related to Redmine - Patch #20369: Use String#casecmp for case insensitive comparison Closed

Associated revisions

Revision 14473
Added by Jean-Philippe Lang over 2 years ago

Wrong syntax for resizing inline images may throw a 500 error (#20278).

Fix by Go MAEDA.

History

#1 Updated by Go MAEDA over 2 years ago

Although this is not a solution to the root of the error, we can avoid the error and save some cycles.
(String#casecmp is much faster than calling String#downcase twice)

Index: app/models/attachment.rb
===================================================================
--- app/models/attachment.rb    (revision 14445)
+++ app/models/attachment.rb    (working copy)
@@ -294,7 +294,7 @@

   def self.latest_attach(attachments, filename)
     attachments.sort_by(&:created_on).reverse.detect do |att|
-      att.filename.downcase == filename.downcase
+      filename.casecmp(att.filename)
     end
   end

#2 Updated by Go MAEDA over 2 years ago

Sorry, workaround on #20278#note-1 is wrong.

Should be replaced with the following:

Index: app/models/attachment.rb
===================================================================
--- app/models/attachment.rb    (revision 14445)
+++ app/models/attachment.rb    (working copy)
@@ -294,7 +294,7 @@

   def self.latest_attach(attachments, filename)
     attachments.sort_by(&:created_on).reverse.detect do |att|
-      att.filename.downcase == filename.downcase
+      filename.casecmp(att.filename) == 0
     end
   end

#3 Updated by Go MAEDA over 2 years ago

  • Related to Patch #20369: Use String#casecmp for case insensitive comparison added

#4 Updated by Jean-Philippe Lang over 2 years ago

  • Category set to Text formatting
  • Status changed from New to Resolved
  • Assignee set to Jean-Philippe Lang
  • Target version set to 3.1.1
  • Resolution set to Fixed

Fix committed with a test in r14473, thanks.

#5 Updated by Jean-Philippe Lang about 2 years ago

  • Status changed from Resolved to Closed

Merged.

Also available in: Atom PDF