From 5fad229e43e2b2541ed39c6ef571975176e6a8d2 Mon Sep 17 00:00:00 2001 From: Vladimir Dobriakov Date: Tue, 4 Nov 2008 13:46:36 +0100 Subject: [PATCH] Fixed that FormTagHelper generates illegal html if name contains e.g. square brackets [#1238 state:committed] Signed-off-by: David Heinemeier Hansson --- actionpack/CHANGELOG | 2 ++ .../action_view/helpers/form_tag_helper.rb | 14 +++++--- .../test/template/form_tag_helper_test.rb | 35 ++++++++++++++++++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3d1e5c13ecea..3ce652253549 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *2.2.1 [RC2 or 2.2 final]* +* Fixed that FormTagHelper generated illegal html if name contained square brackets #1238 [Vladimir Dobriakov] + * Fix regression bug that made date_select and datetime_select raise a Null Pointer Exception when a nil date/datetime was passed and only month and year were displayed #1289 [Bernardo Padua/Tor Erik] * Simplified the logging format for parameters (don't include controller, action, and format as duplicates) [DHH] diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 7492348c50c7..4646bc118b5a 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -78,7 +78,7 @@ def form_tag(url_for_options = {}, options = {}, *parameters_for_url, &block) # # def select_tag(name, option_tags = nil, options = {}) html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name - content_tag :select, option_tags, { "name" => html_name, "id" => name }.update(options.stringify_keys) + content_tag :select, option_tags, { "name" => html_name, "id" => sanitize_to_id(name) }.update(options.stringify_keys) end # Creates a standard text field; use these text fields to input smaller chunks of text like a username @@ -112,7 +112,7 @@ def select_tag(name, option_tags = nil, options = {}) # text_field_tag 'ip', '0.0.0.0', :maxlength => 15, :size => 20, :class => "ip-input" # # => def text_field_tag(name, value = nil, options = {}) - tag :input, { "type" => "text", "name" => name, "id" => name, "value" => value }.update(options.stringify_keys) + tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys) end # Creates a label field @@ -130,7 +130,7 @@ def text_field_tag(name, value = nil, options = {}) # label_tag 'name', nil, :class => 'small_label' # # => def label_tag(name, text = nil, options = {}) - content_tag :label, text || name.to_s.humanize, { "for" => name }.update(options.stringify_keys) + content_tag :label, text || name.to_s.humanize, { "for" => sanitize_to_id(name) }.update(options.stringify_keys) end # Creates a hidden form input field used to transmit data that would be lost due to HTTP's statelessness or @@ -282,7 +282,7 @@ def text_area_tag(name, content = nil, options = {}) # check_box_tag 'eula', 'accepted', false, :disabled => true # # => def check_box_tag(name, value = "1", checked = false, options = {}) - html_options = { "type" => "checkbox", "name" => name, "id" => name, "value" => value }.update(options.stringify_keys) + html_options = { "type" => "checkbox", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys) html_options["checked"] = "checked" if checked tag :input, html_options end @@ -470,6 +470,12 @@ def token_tag tag(:input, :type => "hidden", :name => request_forgery_protection_token.to_s, :value => form_authenticity_token) end end + + # see http://www.w3.org/TR/html4/types.html#type-name + def sanitize_to_id(name) + name.to_s.gsub(']','').gsub(/[^-a-zA-Z0-9:.]/, "_") + end + end end end diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 1849a61f2fc6..de82647813f2 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -12,12 +12,19 @@ def url_for(options) @controller = @controller.new end + VALID_HTML_ID = /^[A-Za-z][-_:.A-Za-z0-9]*$/ # see http://www.w3.org/TR/html4/types.html#type-name + def test_check_box_tag actual = check_box_tag "admin" expected = %() assert_dom_equal expected, actual end + def test_check_box_tag_id_sanitized + label_elem = root_elem(check_box_tag("project[2][admin]")) + assert_match VALID_HTML_ID, label_elem['id'] + end + def test_form_tag actual = form_tag expected = %(
) @@ -64,6 +71,11 @@ def test_hidden_field_tag assert_dom_equal expected, actual end + def test_hidden_field_tag_id_sanitized + input_elem = root_elem(hidden_field_tag("item[][title]")) + assert_match VALID_HTML_ID, input_elem['id'] + end + def test_file_field_tag assert_dom_equal "", file_field_tag("picsplz") end @@ -118,6 +130,11 @@ def test_select_tag_disabled assert_dom_equal expected, actual end + def test_select_tag_id_sanitized + input_elem = root_elem(select_tag("project[1]people", "")) + assert_match VALID_HTML_ID, input_elem['id'] + end + def test_text_area_tag_size_string actual = text_area_tag "body", "hello world", "size" => "20x40" expected = %() @@ -184,6 +201,11 @@ def test_text_field_tag_with_multiple_options assert_dom_equal expected, actual end + def test_text_field_tag_id_sanitized + input_elem = root_elem(text_field_tag("item[][title]")) + assert_match VALID_HTML_ID, input_elem['id'] + end + def test_label_tag_without_text actual = label_tag "title" expected = %() @@ -208,11 +230,16 @@ def test_label_tag_class_string assert_dom_equal expected, actual end + def test_label_tag_id_sanitized + label_elem = root_elem(label_tag("item[title]")) + assert_match VALID_HTML_ID, label_elem['for'] + end + def test_boolean_optios assert_dom_equal %(), check_box_tag("admin", 1, true, 'disabled' => true, :readonly => "yes") assert_dom_equal %(), check_box_tag("admin", 1, true, :disabled => false, :readonly => nil) assert_dom_equal %(), select_tag("people", "", :multiple => true) - assert_dom_equal %(), select_tag("people[]", "", :multiple => true) + assert_dom_equal %(), select_tag("people[]", "", :multiple => true) assert_dom_equal %(), select_tag("people", "", :multiple => nil) end @@ -283,4 +310,10 @@ def test_field_set_tag_in_erb def protect_against_forgery? false end + + private + + def root_elem(rendered_content) + HTML::Document.new(rendered_content).root.children[0] + end end