From cba1460a2fe2bbe1153620582a66e03cec9ba7a5 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 29 Jun 2010 19:55:57 +0200 Subject: url_for no longer escapes HTML, the :escape option is also gone Rationale: url_for is just a path/URL generator, it is the responsability of the caller to escape conveniently HTML needs it, JavaScript needs different escaping, a text mail needs no escaping at all, etc. --- actionpack/CHANGELOG | 2 ++ .../lib/action_view/helpers/form_tag_helper.rb | 2 ++ .../lib/action_view/helpers/prototype_helper.rb | 2 +- actionpack/lib/action_view/helpers/url_helper.rb | 21 +++++---------------- actionpack/test/template/url_helper_test.rb | 20 ++++---------------- 5 files changed, 14 insertions(+), 33 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index cdd64edec0..80be915407 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [Release Candidate] (unreleased)* +* url_for returns always unescaped strings, and the :escape option is gone. [fxn] + * Added accept-charset parameter and _snowman hidden field to force the contents of Rails POSTed forms to be in UTF-8 [Yehuda Katz] diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index efa1446d96..7fea5eb055 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -529,6 +529,8 @@ module ActionView def html_options_for_form(url_for_options, options, *parameters_for_url) returning options.stringify_keys do |html_options| html_options["enctype"] = "multipart/form-data" if html_options.delete("multipart") + # The following URL is unescaped, this is just a hash of options, and it is the + # responsability of the caller to escape all the values. html_options["action"] = url_for(url_for_options, *parameters_for_url) html_options["accept-charset"] = "UTF-8" html_options["data-remote"] = true if html_options.delete("remote") diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index 678e6348b6..5c0ff5d59c 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -131,7 +131,7 @@ module ActionView url_options = options[:url] url_options = url_options.merge(:escape => false) if url_options.is_a?(Hash) - function << "'#{escape_javascript(url_for(url_options))}'" + function << "'#{html_escape(escape_javascript(url_for(url_options)))}'" function << ", #{javascript_options})" function = "#{options[:before]}; #{function}" if options[:before] diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index cbde9b94a7..7d1d00d1fa 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -38,9 +38,6 @@ module ActionView # :only_path is true so you'll get the relative "/controller/action" # instead of the fully qualified URL like "http://example.com/controller/action". # - # When called from a view, +url_for+ returns an HTML escaped url. If you - # need an unescaped url, pass :escape => false in the +options+. - # # ==== Options # * :anchor - Specifies the anchor name to be appended to the path. # * :only_path - If true, returns the relative URL (omitting the protocol, host name, and port) (true by default unless :host is specified). @@ -50,7 +47,6 @@ module ActionView # * :protocol - Overrides the default (current) protocol if provided. # * :user - Inline HTTP authentication (only plucked out if :password is also present). # * :password - Inline HTTP authentication (only plucked out if :user is also present). - # * :escape - Determines whether the returned URL will be HTML escaped or not (true by default). # # ==== Relying on named routes # @@ -72,10 +68,7 @@ module ActionView # <%= url_for(:action => 'play', :anchor => 'player') %> # # => /messages/play/#player # - # <%= url_for(:action => 'checkout', :anchor => 'tax&ship') %> - # # => /testing/jump/#tax&ship - # - # <%= url_for(:action => 'checkout', :anchor => 'tax&ship', :escape => false) %> + # <%= url_for(:action => 'jump', :anchor => 'tax&ship') %> # # => /testing/jump/#tax&ship # # <%= url_for(Workshop.new) %> @@ -100,21 +93,17 @@ module ActionView options ||= {} url = case options when String - escape = true options when Hash options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) - escape = options.key?(:escape) ? options.delete(:escape) : true super when :back - escape = false controller.request.env["HTTP_REFERER"] || 'javascript:history.back()' else - escape = false polymorphic_path(options) end - escape ? escape_once(url).html_safe : url + url end # Creates a link tag of the given +name+ using a URL created by the set @@ -254,8 +243,8 @@ module ActionView tag_options = nil end - href_attr = "href=\"#{url}\"" unless href - "#{ERB::Util.h(name || url)}".html_safe + href_attr = "href=\"#{escape_once(url)}\"" unless href + "#{html_escape(name || url)}".html_safe end end @@ -574,7 +563,7 @@ module ActionView "in a #request method" end - url_string = CGI.unescapeHTML(url_for(options)) + url_string = url_for(options) # We ignore any extra parameters in the request_uri if the # submitted url doesn't have any either. This lets the function diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 897228677b..035f501f03 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -40,20 +40,8 @@ class UrlHelperTest < ActiveSupport::TestCase end alias url_hash hash_for - def test_url_for_escapes_urls - assert_equal "/?a=b&c=d", url_for(abcd) - assert_equal "/?a=b&c=d", url_for(abcd(:escape => true)) - assert_equal "/?a=b&c=d", url_for(abcd(:escape => false)) - end - - def test_url_for_escaping_is_safety_aware - assert url_for(abcd(:escape => true)).html_safe?, "escaped urls should be html_safe?" - assert !url_for(abcd(:escape => false)).html_safe?, "non-escaped urls should not be html_safe?" - end - - def test_url_for_escapes_url_once - assert_equal "/?a=b&c=d", url_for("/?a=b&c=d") - assert_equal "/?a=b&c=d", url_for(abcd) + def test_url_for_does_not_escape_urls + assert_equal "/?a=b&c=d", url_for(abcd) end def test_url_for_with_back @@ -151,7 +139,7 @@ class UrlHelperTest < ActiveSupport::TestCase end def test_link_tag_with_query_and_no_name - link = link_to(nil, "http://www.example.com?q1=v1&q2=v2") + link = link_to(nil, "http://www.example.com?q1=v1&q2=v2") expected = %{http://www.example.com?q1=v1&q2=v2} assert_dom_equal expected, link end @@ -308,7 +296,7 @@ class UrlHelperTest < ActiveSupport::TestCase @request = request_for_url("/?order=desc&page=1") assert current_page?(hash_for(:order => "desc", :page => "1")) - assert current_page?("http://www.example.com/?order=desc&page=1") + assert current_page?("http://www.example.com/?order=desc&page=1") end def test_link_unless_current -- cgit v1.2.3