Permalink
Browse files

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.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
1 parent a41c6c3 commit 1b3195b63ca44f0a70b61b75fcf4991cb2fbb944 Phil Darnowsky committed with NZKoz Oct 7, 2009
Showing with 14 additions and 4 deletions.
  1. +1 −1 actionpack/lib/action_view/helpers/url_helper.rb
  2. +13 −3 actionpack/test/template/url_helper_test.rb
@@ -83,7 +83,7 @@ def url_for(options = {})
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
@@ -22,7 +22,7 @@ def url_for(options)
def test_url_for_escapes_urls
@controller.url = "http://www.example.com?a=b&c=d"
- assert_equal "http://www.example.com?a=b&amp;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&amp;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
@@ -42,6 +42,16 @@ def test_url_for_with_back_and_no_referer
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 "<form method=\"post\" action=\"http://www.example.com\" class=\"button-to\"><div><input type=\"submit\" value=\"Hello\" /></div></form>", button_to("Hello", "http://www.example.com")
@@ -298,7 +308,7 @@ def test_link_unless_current
@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&amp;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")
@@ -308,7 +318,7 @@ def test_link_unless_current
@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 "<a href=\"http://www.example.com/weblog/show?order=desc&amp;page=2\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
+ assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&page=2\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&amp;page=2\">Showing</a>", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=2")

0 comments on commit 1b3195b

Please sign in to comment.