From 8824f88ef6182a9504a3e75fe4875e1c73f691fa Mon Sep 17 00:00:00 2001 From: Phil Darnowsky Date: Wed, 7 Oct 2009 14:49:38 -0400 Subject: [PATCH] ActionView.url_for doesn't escape by default ActionView::Helpers::UrlHelper#url_for used to escape the URLs it generated by default. This was most commonly seen when generating a path with multiple query parameters, e.g. url_for(:controller => :foo, :action => :bar, :this => 123, :that => 456) would return http://example.com/foo/bar?that=456&this=123 escaping an ampersand that shouldn't be escaped. This is both wrong and inconsistent with the behavior of ActionController#url_for, and is changed. --- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- actionpack/test/template/url_helper_test.rb | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 204d4d7..0ec6ac8 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -83,7 +83,7 @@ module ActionView options when Hash options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) - escape = options.key?(:escape) ? options.delete(:escape) : true + escape = options.key?(:escape) ? options.delete(:escape) : false @controller.send(:url_for, options) when :back escape = false diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index ce99482..f6f9e06 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -19,7 +19,7 @@ class UrlHelperTest < ActionView::TestCase def test_url_for_escapes_urls @controller.url = "http://www.example.com?a=b&c=d" - assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd') + assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd') assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => true) assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => false) end @@ -39,6 +39,16 @@ class UrlHelperTest < ActionView::TestCase assert_equal 'javascript:history.back()', url_for(:back) end + def test_url_for_from_hash_doesnt_escape_ampersand + @controller = TestController.new + @view = ActionView::Base.new + @view.controller = @controller + + path = @view.url_for(:controller => :cheeses, :foo => :bar, :baz => :quux) + + assert_equal '/cheeses?baz=quux&foo=bar', path + end + # todo: missing test cases def test_button_to_with_straight_url assert_dom_equal "
", button_to("Hello", "http://www.example.com") @@ -295,7 +305,7 @@ class UrlHelperTest < ActionView::TestCase @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1") @controller.url = "http://www.example.com/weblog/show?order=desc&page=1" assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog", :order=>'desc', :page=>'1' }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1") + assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1") assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1") @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc") @@ -305,7 +315,7 @@ class UrlHelperTest < ActionView::TestCase @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1") @controller.url = "http://www.example.com/weblog/show?order=desc&page=2" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) + assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=2") -- 1.6.0.4