Macros processing overhaul (#3061, #11633).

* macro arguments are no longer parsed by text formatters
* macro output is escaped unless it's html safe

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10209 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/helpers/application_helper.rb

@@ -527,6 +527,8 @@ module ApplicationHelper
     project = options[:project] || @project || (obj && obj.respond_to?(:project) ? obj.project : nil)
     only_path = options.delete(:only_path) == false ? false : true
 
+    text = text.dup
+    macros = catch_macros(text)
     text = Redmine::WikiFormatting.to_html(Setting.text_formatting, text, :object => obj, :attribute => attr)
 
     @parsed_headings = []
@@ -534,8 +536,8 @@ module ApplicationHelper
     @current_section = 0 if options[:edit_section_links]
 
     parse_sections(text, project, obj, attr, only_path, options)
-    text = parse_non_pre_blocks(text) do |text|
-      [:parse_inline_attachments, :parse_wiki_links, :parse_redmine_links, :parse_macros].each do |method_name|
+    text = parse_non_pre_blocks(text, obj, macros) do |text|
+      [:parse_inline_attachments, :parse_wiki_links, :parse_redmine_links].each do |method_name|
         send method_name, text, project, obj, attr, only_path, options
       end
     end
@@ -548,7 +550,7 @@ module ApplicationHelper
     text.html_safe
   end
 
-  def parse_non_pre_blocks(text)
+  def parse_non_pre_blocks(text, obj, macros)
     s = StringScanner.new(text)
     tags = []
     parsed = ''
@@ -557,6 +559,9 @@ module ApplicationHelper
       text, full_tag, closing, tag = s[1], s[2], s[3], s[4]
       if tags.empty?
         yield text
+        inject_macros(text, obj, macros) if macros.any?
+      else
+        inject_macros(text, obj, macros, false) if macros.any?
       end
       parsed << text
       if tag
@@ -856,7 +861,7 @@ module ApplicationHelper
     end
   end
 
-  MACROS_RE = /
+  MACROS_RE = /(
                 (!)?                        # escaping
                 (
                 \{\{                        # opening tag
@@ -864,22 +869,48 @@ module ApplicationHelper
                 (\((.*?)\))?                # optional arguments
                 \}\}                        # closing tag
                 )
-              /x unless const_defined?(:MACROS_RE)
+               )/x unless const_defined?(:MACROS_RE)
 
-  # Macros substitution
-  def parse_macros(text, project, obj, attr, only_path, options)
+  MACRO_SUB_RE = /(
+                  \{\{
+                  macro\((\d+)\)
+                  \}\}
+                  )/x unless const_defined?(:MACROS_SUB_RE)
+
+  # Extracts macros from text
+  def catch_macros(text)
+    macros = {}
     text.gsub!(MACROS_RE) do
-      esc, all, macro, args = $1, $2, $3.downcase, $5.to_s
-      if esc.nil?
-        begin
-          exec_macro(macro, obj, args)
-        rescue => e
-          "<div class=\"flash error\">Error executing the <strong>#{macro}</strong> macro (#{e})</div>"
-        end || all
+      all, macro = $1, $4.downcase
+      if macro_exists?(macro) || all =~ MACRO_SUB_RE
+        index = macros.size
+        macros[index] = all
+        "{{macro(#{index})}}"
       else
         all
       end
     end
+    macros
+  end
+
+  # Executes and replaces macros in text
+  def inject_macros(text, obj, macros, execute=true)
+    text.gsub!(MACRO_SUB_RE) do
+      all, index = $1, $2.to_i
+      orig = macros.delete(index)
+      if execute && orig && orig =~ MACROS_RE
+        esc, all, macro, args = $2, $3, $4.downcase, $6.to_s
+        if esc.nil?
+          h(exec_macro(macro, obj, args) || all)
+        else
+          h(all)
+        end
+      elsif orig
+        h(orig)
+      else
+        h(all)
+      end
+    end
   end
 
   TOC_RE = /<p>\{\{([<>]?)toc\}\}<\/p>/i unless const_defined?(:TOC_RE)

lib/redmine/wiki_formatting.rb

@@ -15,6 +15,8 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
+require 'digest/md5'
+
 module Redmine
   module WikiFormatting
     class StaleSectionError < Exception; end
@@ -50,7 +52,7 @@ module Redmine
       end
 
       def to_html(format, text, options = {})
-        text = if Setting.cache_formatted_text? && text.size > 2.kilobyte && cache_store && cache_key = cache_key_for(format, options[:object], options[:attribute])
+        text = if Setting.cache_formatted_text? && text.size > 2.kilobyte && cache_store && cache_key = cache_key_for(format, text, options[:object], options[:attribute])
           # Text retrieved from the cache store may be frozen
           # We need to dup it so we can do in-place substitutions with gsub!
           cache_store.fetch cache_key do
@@ -67,10 +69,10 @@ module Redmine
         (formatter.instance_methods & ['update_section', :update_section]).any?
       end
 
-      # Returns a cache key for the given text +format+, +object+ and +attribute+ or nil if no caching should be done
-      def cache_key_for(format, object, attribute)
-        if object && attribute && !object.new_record? && object.respond_to?(:updated_on) && !format.blank?
-          "formatted_text/#{format}/#{object.class.model_name.cache_key}/#{object.id}-#{attribute}-#{object.updated_on.to_s(:number)}"
+      # Returns a cache key for the given text +format+, +text+, +object+ and +attribute+ or nil if no caching should be done
+      def cache_key_for(format, text, object, attribute)
+        if object && attribute && !object.new_record? && format.present?
+          "formatted_text/#{format}/#{object.class.model_name.cache_key}/#{object.id}-#{attribute}-#{Digest::MD5.hexdigest text}"
         end
       end
 

lib/redmine/wiki_formatting/macros.rb

@@ -19,6 +19,11 @@ module Redmine
   module WikiFormatting
     module Macros
       module Definitions
+        # Returns true if +name+ is the name of an existing macro
+        def macro_exists?(name)
+          Redmine::WikiFormatting::Macros.available_macros.key?(name.to_sym)
+        end
+
         def exec_macro(name, obj, args)
           macro_options = Redmine::WikiFormatting::Macros.available_macros[name.to_sym]
           return unless macro_options
@@ -27,7 +32,12 @@ module Redmine
           unless macro_options[:parse_args] == false
             args = args.split(',').map(&:strip)
           end
-          send(method_name, obj, args) if respond_to?(method_name)
+
+          begin
+            send(method_name, obj, args) if respond_to?(method_name)
+          rescue => e
+            "<div class=\"flash error\">Error executing the <strong>#{h name}</strong> macro (#{h e.to_s})</div>".html_safe
+          end
         end
 
         def extract_macro_options(args, *keys)
@@ -97,7 +107,7 @@ module Redmine
       # Builtin macros
       desc "Sample macro."
       macro :hello_world do |obj, args|
-        "Hello world! Object: #{obj.class.name}, " + (args.empty? ? "Called with no argument." : "Arguments: #{args.join(', ')}")
+        h("Hello world! Object: #{obj.class.name}, " + (args.empty? ? "Called with no argument." : "Arguments: #{args.join(', ')}"))
       end
 
       desc "Displays a list of all available macros, including description if available."

test/unit/lib/redmine/wiki_formatting/macros_test.rb

@@ -76,12 +76,71 @@ class Redmine::WikiFormatting::MacrosTest < ActionView::TestCase
     assert_equal '<p>no args args: c,d</p>', textilizable("{{foo}} {{foo(c,d)}}")
   end
 
+  def test_macro_should_receive_the_object_as_argument_when_with_object_and_attribute
+    issue = Issue.find(1)
+    issue.description = "{{hello_world}}"
+    assert_equal '<p>Hello world! Object: Issue, Called with no argument.</p>', textilizable(issue, :description)
+  end
+
+  def test_macro_should_receive_the_object_as_argument_when_called_with_object_option
+    text = "{{hello_world}}"
+    assert_equal '<p>Hello world! Object: Issue, Called with no argument.</p>', textilizable(text, :object => Issue.find(1))
+  end
+
+  def test_macro_exception_should_be_displayed
+    Redmine::WikiFormatting::Macros.macro :exception do |obj, args|
+      raise "My message"
+    end
+
+    text = "{{exception}}"
+    assert_include '<div class="flash error">Error executing the <strong>exception</strong> macro (My message)</div>', textilizable(text)
+  end
+
+  def test_macro_arguments_should_not_be_parsed_by_formatters
+    text = '{{hello_world(http://www.redmine.org, #1)}}'
+    assert_include 'Arguments: http://www.redmine.org, #1', textilizable(text)
+  end
+
+  def test_exclamation_mark_should_not_run_macros
+    text = "!{{hello_world}}"
+    assert_equal '<p>{{hello_world}}</p>', textilizable(text)
+  end
+
+  def test_exclamation_mark_should_escape_macros
+    text = "!{{hello_world(<tag>)}}"
+    assert_equal '<p>{{hello_world(&lt;tag&gt;)}}</p>', textilizable(text)
+  end
+
+  def test_unknown_macros_should_not_be_replaced
+    text = "{{unknown}}"
+    assert_equal '<p>{{unknown}}</p>', textilizable(text)
+  end
+
+  def test_unknown_macros_should_parsed_as_text
+    text = "{{unknown(*test*)}}"
+    assert_equal '<p>{{unknown(<strong>test</strong>)}}</p>', textilizable(text)
+  end
+
+  def test_unknown_macros_should_be_escaped
+    text = "{{unknown(<tag>)}}"
+    assert_equal '<p>{{unknown(&lt;tag&gt;)}}</p>', textilizable(text)
+  end
+
+  def test_html_safe_macro_output_should_not_be_escaped
+    Redmine::WikiFormatting::Macros.macro :safe_macro do |obj, args|
+      "<tag>".html_safe
+    end
+    assert_equal '<p><tag></p>', textilizable("{{safe_macro}}")
+  end
+
   def test_macro_hello_world
     text = "{{hello_world}}"
     assert textilizable(text).match(/Hello world!/)
-    # escaping
-    text = "!{{hello_world}}"
-    assert_equal '<p>{{hello_world}}</p>', textilizable(text)
+  end
+
+  def test_macro_hello_world_should_escape_arguments
+    text = "{{hello_world(<tag>)}}"
+    assert_include 'Arguments: &lt;tag&gt;', textilizable(text)
   end
 
   def test_macro_macro_list
@@ -93,18 +152,18 @@ class Redmine::WikiFormatting::MacrosTest < ActionView::TestCase
     @project = Project.find(1)
     # include a page of the current project wiki
     text = "{{include(Another page)}}"
-    assert textilizable(text).match(/This is a link to a ticket/)
+    assert_include 'This is a link to a ticket', textilizable(text)
 
     @project = nil
     # include a page of a specific project wiki
     text = "{{include(ecookbook:Another page)}}"
-    assert textilizable(text).match(/This is a link to a ticket/)
+    assert_include 'This is a link to a ticket', textilizable(text)
 
     text = "{{include(ecookbook:)}}"
-    assert textilizable(text).match(/CookBook documentation/)
+    assert_include 'CookBook documentation', textilizable(text)
 
     text = "{{include(unknowidentifier:somepage)}}"
-    assert textilizable(text).match(/Page not found/)
+    assert_include 'Page not found', textilizable(text)
   end
 
   def test_macro_child_pages
@@ -164,4 +223,40 @@ class Redmine::WikiFormatting::MacrosTest < ActionView::TestCase
     assert_include 'test.png not found',
       textilizable("{{thumbnail(test.png)}}", :object => Issue.find(14))
   end
+
+  def test_macros_should_not_be_executed_in_pre_tags
+    text = <<-RAW
+{{hello_world(foo)}}
+
+<pre>
+{{hello_world(pre)}}
+!{{hello_world(pre)}}
+</pre>
+
+{{hello_world(bar)}}
+RAW
+
+    expected = <<-EXPECTED
+<p>Hello world! Object: NilClass, Arguments: foo</p>
+
+<pre>
+{{hello_world(pre)}}
+!{{hello_world(pre)}}
+</pre>
+
+<p>Hello world! Object: NilClass, Arguments: bar</p>
+EXPECTED
+
+    assert_equal expected.gsub(%r{[\r\n\t]}, ''), textilizable(text).gsub(%r{[\r\n\t]}, '')
+  end
+
+  def test_macros_should_be_escaped_in_pre_tags
+    text = "<pre>{{hello_world(<tag>)}}</pre>"
+    assert_equal "<pre>{{hello_world(&lt;tag&gt;)}}</pre>", textilizable(text)
+  end
+
+  def test_macros_should_not_mangle_next_macros_outputs
+    text = '{{macro(2)}} !{{macro(2)}} {{hello_world(foo)}}'
+    assert_equal '<p>{{macro(2)}} {{macro(2)}} Hello world! Object: NilClass, Arguments: foo</p>', textilizable(text)
+  end
 end

test/unit/lib/redmine/wiki_formatting_test.rb

@@ -18,6 +18,7 @@
 require File.expand_path('../../../../test_helper', __FILE__)
 
 class Redmine::WikiFormattingTest < ActiveSupport::TestCase
+  fixtures :issues
 
   def test_textile_formatter
     assert_equal Redmine::WikiFormatting::Textile::Formatter, Redmine::WikiFormatting.formatter_for('textile')
@@ -52,4 +53,8 @@ EXPECTED
       assert_equal false, Redmine::WikiFormatting.supports_section_edit?
     end
   end
+
+  def test_cache_key_for_saved_object_should_no_be_nil
+    assert_not_nil Redmine::WikiFormatting.cache_key_for('textile', 'Text', Issue.find(1), :description)
+  end
 end