remove SafeBuffer from link_to helper options, fixes padrino#1529, ad…

…d tests
Ortuna · Jan 17, 2014 · 2f16092 · 2f16092
1 parent 9e126d6
commit 2f16092
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 25 deletions.
diff --git a/padrino-helpers/lib/padrino-helpers/asset_tag_helpers.rb b/padrino-helpers/lib/padrino-helpers/asset_tag_helpers.rb
@@ -4,7 +4,6 @@ module Helpers
     # Helpers related to producing assets (images, stylesheets, js, etc) within templates.
     #
     module AssetTagHelpers
-      FRAGMENT_HASH = "#".html_safe.freeze
       APPEND_ASSET_EXTENSIONS = ["js", "css"]
       ABSOLUTE_URL_PATTERN = %r{^(https?://)}
 
@@ -79,32 +78,16 @@ def link_to(*args, &block)
         options  = args.extract_options!
         fragment = options.delete(:anchor).to_s if options[:anchor]
         fragment = options.delete(:fragment).to_s if options[:fragment]
-
-        url = ActiveSupport::SafeBuffer.new
-        if block_given?
-          if args[0]
-            url.concat(args[0])
-            url.concat(FRAGMENT_HASH).concat(fragment) if fragment
-          else
-            url.concat(FRAGMENT_HASH)
-            url.concat(fragment) if fragment
-          end
-          options.reverse_merge!(:href => url)
-          return '' unless parse_conditions(url, options)
-          content_tag(:a, options, &block)
+        name = block_given? ? '' : args.shift
+        if url = args.first
+          url << '#' << fragment if fragment
         else
-          if args[1]
-            url.concat(args[1])
-            url.safe_concat(FRAGMENT_HASH).concat(fragment) if fragment
-          else
-            url.concat(FRAGMENT_HASH)
-            url.concat(fragment) if fragment
-          end
-          name = args[0]
-          return name unless parse_conditions(url, options)
-          options.reverse_merge!(:href => url)
-          content_tag(:a, name, options)
+          url = '#'
+          url << fragment if fragment
         end
+        options.reverse_merge!(:href => url)
+        return name unless parse_conditions(url, options)
+        block_given? ? content_tag(:a, options, &block) : content_tag(:a, name, options)
       end
 
       ##

diff --git a/padrino-helpers/test/test_asset_tag_helpers.rb b/padrino-helpers/test/test_asset_tag_helpers.rb
@@ -98,6 +98,16 @@ def flash
       assert_have_selector :a, :content => "Test 1 No Block", :href => '/test1', :class => 'test', :id => 'test1'
       assert_have_selector :a, :content => "Test 2 With Block", :href => '/test2', :class => 'test', :id => 'test2'
     end
+
+    should "not double-escape" do
+      actual_link = link_to('test escape', '?a=1&b=2')
+      assert_has_tag('a', :href => '?a=1&b=2') { actual_link }
+    end
+
+    should "escape scary things" do
+      actual_link = link_to('test escape<adfs>', '?a=1&b=<script>alert(1)</script>')
+      assert_no_match('<script', actual_link)
+    end
   end
 
   context 'for #mail_to method' do