Escape attributes regardless of whether it's SafeBuffer or not

[k0kubun]

Fixes #993, fixes #1019

Like rails helpers fixed in CVE-2016-6316, an attribute should be HTML-escaped even if it's a SafeBuffer.

[k0kubun]

ActiveSupport monkey-patches this method for html_safe?

[HamptonMakes]

Definitely like seeing the CGI version vs leaning on ActiveSupport

[HamptonMakes]

Just one change of breaking out that test, and you are good-to-merge from my perspective.

[HamptonMakes]

Definitely like seeing the CGI version vs leaning on ActiveSupport

Fixed commented places

[HamptonMakes]

I think you misunderstood what I meant about the files... let me show you in a next PR.

[gurgeous]

Pardon my ignorance here - won't this double escape?

# in rails console
Haml::Engine.new("%p{title: html_escape('hi &')}").render
=> "<p title='hi &amp;amp;'></p>\n"

That seems incorrect and is causing problems for my app. Should we adopt the Rails fix and only escape the quotes? See rails/rails@v4.2.7...v4.2.7.1

        def tag_option(key, value, escape)
          if value.is_a?(Array)
            value = escape ? safe_join(value, " ") : value.join(" ")
          else
            value = escape ? ERB::Util.unwrapped_html_escape(value) : value
          end
          %(#{key}="#{value.gsub(/"/, '"'.freeze)}")
        end

[k0kubun]

Could you open a separate issue or pull request? Because that is an intended behavior in this PR, it seems like a new feature. Double escaping could happen for many other cases when you escape something where escaping automatically happens, and that place is where you don't need to escape it since Haml 5+.

[gurgeous]

Yes, of course, thank you for the quick response!