Fix for haml_tag and haml_concat and XSS

Before this commit, haml_tag relied on haml_concat to write its output.
This created a problem when XSS protection was in use - the tags
themselves needed not to be escaped, but the tags contents should be
escaped. The current workaround used the with_raw_haml_concat method to
set a flag to control whether haml_concat should be escaped, but this is
too crude and results in any use of haml_concat inside a block passed to
haml_tag not being escaped when it should.

Create new private methods in Helpers to allow more control of writing
to the buffer, and change haml_tag to use them so that haml_tag and
haml_concat behave correctly when XSS protection is in use.

Also alter haml_concat_with_haml_xss so it still respects
with_raw_haml_concat.

See #718, #731, #732

lib/haml/helpers.rb

@@ -395,14 +395,35 @@ MESSAGE
     #
     # @param text [#to_s] The text to output
     def haml_concat(text = "")
-      if haml_buffer.options[:ugly] || haml_buffer.tabulation == 0
-        haml_buffer.buffer << "#{text}\n"
-      else
-        haml_buffer.buffer << %[#{haml_indent}#{text.to_s.gsub("\n", "\n#{haml_indent}")}\n]
-      end
+      haml_internal_concat text
       ErrorReturn.new("haml_concat")
     end
 
+    # Internal method to write directly to the buffer with control of
+    # whether the first line shoule be indented, and if there should be a
+    # final newline.
+    #
+    # Lines added will have the proper indentation. This can be controlled
+    # for the first line.
+    #
+    # Used by #haml_concat and #haml_tag.
+    #
+    # @param text [#to_s] The text to output
+    # @param newline [Boolean] Whether to add a newline after the text
+    # @param indent [Boolean] Whether to add indentation to the first line
+    def haml_internal_concat(text = "", newline = true, indent = true)
+      if haml_buffer.options[:ugly] || haml_buffer.tabulation == 0
+        haml_buffer.buffer << "#{text}#{"\n" if newline}"
+      else
+        haml_buffer.buffer << %[#{haml_indent if indent}#{text.to_s.gsub("\n", "\n#{haml_indent}")}#{"\n" if newline}]
+      end
+    end
+    private :haml_internal_concat
+
+    # Allows writing raw content. `haml_internal_concat_raw` isn't
+    # effected by XSS mods. Used by #haml_tag to write the actual tags.
+    alias :haml_internal_concat_raw :haml_internal_concat
+
     # @return [String] The indentation string for the current line
     def haml_indent
       '  ' * haml_buffer.tabulation
@@ -482,7 +503,7 @@ MESSAGE
         attrs)
 
       if text.nil? && block.nil? && (haml_buffer.options[:autoclose].include?(name) || flags.include?(:/))
-        haml_concat "<#{name}#{attributes}#{' /' if haml_buffer.options[:format] == :xhtml}>"
+        haml_internal_concat_raw "<#{name}#{attributes}#{' /' if haml_buffer.options[:format] == :xhtml}>"
         return ret
       end
 
@@ -492,17 +513,19 @@ MESSAGE
       end
 
       tag = "<#{name}#{attributes}>"
+      end_tag = "</#{name}>"
       if block.nil?
         text = text.to_s
         if text.include?("\n")
-          haml_concat tag
+          haml_internal_concat_raw tag
           tab_up
-          haml_concat text
+          haml_internal_concat text
           tab_down
-          haml_concat "</#{name}>"
+          haml_internal_concat_raw end_tag
         else
-          tag << "#{text}</#{name}>"
-          haml_concat tag
+          haml_internal_concat_raw tag, false
+          haml_internal_concat text, false, false
+          haml_internal_concat_raw end_tag, true, false
         end
         return ret
       end
@@ -512,16 +535,17 @@ MESSAGE
       end
 
       if flags.include?(:<)
-        tag << "#{capture_haml(&block).strip}</#{name}>"
-        haml_concat tag
+        haml_internal_concat_raw tag, false
+        haml_internal_concat "#{capture_haml(&block).strip}", false, false
+        haml_internal_concat_raw end_tag, true, false
         return ret
       end
 
-      haml_concat tag
+      haml_internal_concat_raw tag
       tab_up
       block.call
       tab_down
-      haml_concat "</#{name}>"
+      haml_internal_concat_raw end_tag
 
       ret
     end

lib/haml/helpers/xss_mods.rb

@@ -7,8 +7,8 @@ module Haml
     module XssMods
       def self.included(base)
         %w[html_escape find_and_preserve preserve list_of surround
-           precede succeed capture_haml haml_concat haml_indent
-           haml_tag escape_once].each do |name|
+           precede succeed capture_haml haml_concat haml_internal_concat haml_indent
+           escape_once].each do |name|
           base.send(:alias_method, "#{name}_without_haml_xss", name)
           base.send(:alias_method, name, "#{name}_with_haml_xss")
         end
@@ -61,24 +61,29 @@ module Haml
         Haml::Util.html_safe(capture_haml_without_haml_xss(*args, &block))
       end
 
-      # Input is escaped
+      # Input will be escaped unless this is in a `with_raw_haml_concat`
+      # block. See #Haml::Helpers::ActionViewExtensions#with_raw_haml_concat.
       def haml_concat_with_haml_xss(text = "")
         raw = instance_variable_defined?(:@_haml_concat_raw) ? @_haml_concat_raw : false
-        haml_concat_without_haml_xss(raw ? text : haml_xss_html_escape(text))
+        if raw
+          haml_internal_concat_raw text
+        else
+          haml_internal_concat text
+        end
+        ErrorReturn.new("haml_concat")
       end
 
+      # Input is escaped
+      def haml_internal_concat_with_haml_xss(text="", newline=true, indent=true)
+        haml_internal_concat_without_haml_xss(haml_xss_html_escape(text), newline, indent)
+      end
+      private :haml_internal_concat_with_haml_xss
+
       # Output is always HTML safe
       def haml_indent_with_haml_xss
         Haml::Util.html_safe(haml_indent_without_haml_xss)
       end
 
-      # Input is escaped, haml_concat'ed output is always HTML safe
-      def haml_tag_with_haml_xss(name, *rest, &block)
-        name = haml_xss_html_escape(name.to_s)
-        rest.unshift(haml_xss_html_escape(rest.shift.to_s)) unless [Symbol, Hash, NilClass].any? {|t| rest.first.is_a? t}
-        with_raw_haml_concat {haml_tag_without_haml_xss(name, *rest, &block)}
-      end
-
       # Output is always HTML safe
       def escape_once_with_haml_xss(*args)
         Haml::Util.html_safe(escape_once_without_haml_xss(*args))