Defect #37755

Mentioning users with certain characters renders incorrectly

Added by Nicholas Natale about 1 month ago. Updated about 12 hours ago.

Status:ResolvedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:

0%

Category:Issues
Target version:5.0.5
Resolution: Affected version:5.0.2

Description

Mentioning users that have apostrophes in their first/last name (possibly other characters too?) causes them to render incorrectly when a note/description is saved.

Example:

@someusername

Renders as

@Paul O'Neil

Expected rendering:

@Paul O'Neil

Other environment information:

  • Ruby version: 2.7.6
  • Rails version: 6.1.6
  • DB: MariaDB 5.5.68

Associated revisions

Revision 21986
Added by Marius BALTEANU about 12 hours ago

Fix mentioning users with certain characters renders incorrectly (#37755).

Patch Mizuki ISHIKAWA.

History

#1 Updated by Go MAEDA about 1 month ago

  • Status changed from New to Confirmed

#2 Updated by Mizuki ISHIKAWA 2 days ago

The following code is causing the problem.

      # https://github.com/redmine/redmine/blob/df88b9c7ddb485784b1c74c40e7b34675d68f983/app/helpers/application_helper.rb#L62-L63
      name = h(principal.name(options[:format]))
      name = "@" + name if options[:mention]

  • h(principal.name(options[:format])) => ActiveSupport::SafeBuffer object
  • "@" => String object
  • "@" + name => String object

The concatenation of "" changes the class of the object(ActiveSupport::SafeBuffer => String).
Pre-converting "
" to an ActiveSupport::SafeBuffer object by html_safe should prevent the class from being changed.

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index ced1845ebb..ec97845756 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -60,7 +60,7 @@ module ApplicationHelper
     case principal
     when User
       name = h(principal.name(options[:format]))
-      name = "@" + name if options[:mention]
+      name = "@".html_safe + name if options[:mention]
       css_classes = ''
       if principal.active? || (User.current.admin? && principal.logged?)
         url = user_url(principal, :only_path => only_path)
diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb
index a5c533a4fb..d34de2b08c 100644
--- a/test/helpers/application_helper_test.rb
+++ b/test/helpers/application_helper_test.rb
@@ -1841,6 +1841,16 @@ class ApplicationHelperTest < Redmine::HelperTest
     assert_equal result, link_to_principal(unknown_principal, :class => 'bar')
   end

+  def test_link_to_principal_should_escape_principal_name
+    user = User.generate!(firstname: "firstname<>'", lastname: 'lastname&"')
+    group = Group.generate!(lastname: "group<>'&")
+
+    assert_include "firstname&lt;&gt;&#39; lastname&amp;&quot;", link_to_principal(user)
+    assert_include "@firstname&lt;&gt;&#39; lastname&amp;&quot;", link_to_principal(user, {mention: true})
+    assert_include "group&lt;&gt;&#39;&amp;", link_to_principal(group)
+    assert_include "&lt;&gt;&#39;&amp;", link_to_principal("<>'&")
+  end
+
   def test_link_to_group_should_return_only_group_name_for_non_admin_users
     User.current = nil
     group = Group.find(10)

#3 Updated by Marius BALTEANU about 13 hours ago

  • Target version set to 5.0.5

#4 Updated by Marius BALTEANU about 12 hours ago

  • Status changed from Confirmed to Resolved
  • Assignee set to Marius BALTEANU

Fix committed, thanks!

Also available in: Atom PDF