Skip to content
This repository
Browse code

Fixed that FormTagHelper generates illegal html if name contains e.g.…

… square brackets [#1238 state:committed]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information...
commit 5fad229e43e2b2541ed39c6ef571975176e6a8d2 1 parent b2cd318
Vladimir Dobriakov authored November 04, 2008 dhh committed November 04, 2008
2  actionpack/CHANGELOG
... ...
@@ -1,5 +1,7 @@
1 1
 *2.2.1 [RC2 or 2.2 final]*
2 2
 
  3
+* Fixed that FormTagHelper generated illegal html if name contained square brackets #1238 [Vladimir Dobriakov]
  4
+
3 5
 * 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]
4 6
 
5 7
 * Simplified the logging format for parameters (don't include controller, action, and format as duplicates) [DHH]
14  actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -78,7 +78,7 @@ def form_tag(url_for_options = {}, options = {}, *parameters_for_url, &block)
78 78
       #   #    <option>Paris</option><option>Rome</option></select>
79 79
       def select_tag(name, option_tags = nil, options = {})
80 80
         html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name
81  
-        content_tag :select, option_tags, { "name" => html_name, "id" => name }.update(options.stringify_keys)
  81
+        content_tag :select, option_tags, { "name" => html_name, "id" => sanitize_to_id(name) }.update(options.stringify_keys)
82 82
       end
83 83
 
84 84
       # 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 = {})
112 112
       #   text_field_tag 'ip', '0.0.0.0', :maxlength => 15, :size => 20, :class => "ip-input"
113 113
       #   # => <input class="ip-input" id="ip" maxlength="15" name="ip" size="20" type="text" value="0.0.0.0" />
114 114
       def text_field_tag(name, value = nil, options = {})
115  
-        tag :input, { "type" => "text", "name" => name, "id" => name, "value" => value }.update(options.stringify_keys)
  115
+        tag :input, { "type" => "text", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
116 116
       end
117 117
 
118 118
       # Creates a label field
@@ -130,7 +130,7 @@ def text_field_tag(name, value = nil, options = {})
130 130
       #   label_tag 'name', nil, :class => 'small_label'
131 131
       #   # => <label for="name" class="small_label">Name</label>
132 132
       def label_tag(name, text = nil, options = {})
133  
-        content_tag :label, text || name.to_s.humanize, { "for" => name }.update(options.stringify_keys)
  133
+        content_tag :label, text || name.to_s.humanize, { "for" => sanitize_to_id(name) }.update(options.stringify_keys)
134 134
       end
135 135
 
136 136
       # 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 = {})
282 282
       #   check_box_tag 'eula', 'accepted', false, :disabled => true
283 283
       #   # => <input disabled="disabled" id="eula" name="eula" type="checkbox" value="accepted" />
284 284
       def check_box_tag(name, value = "1", checked = false, options = {})
285  
-        html_options = { "type" => "checkbox", "name" => name, "id" => name, "value" => value }.update(options.stringify_keys)
  285
+        html_options = { "type" => "checkbox", "name" => name, "id" => sanitize_to_id(name), "value" => value }.update(options.stringify_keys)
286 286
         html_options["checked"] = "checked" if checked
287 287
         tag :input, html_options
288 288
       end
@@ -470,6 +470,12 @@ def token_tag
470 470
             tag(:input, :type => "hidden", :name => request_forgery_protection_token.to_s, :value => form_authenticity_token)
471 471
           end
472 472
         end
  473
+
  474
+        # see http://www.w3.org/TR/html4/types.html#type-name
  475
+        def sanitize_to_id(name)
  476
+          name.to_s.gsub(']','').gsub(/[^-a-zA-Z0-9:.]/, "_")
  477
+        end
  478
+
473 479
     end
474 480
   end
475 481
 end
35  actionpack/test/template/form_tag_helper_test.rb
@@ -12,12 +12,19 @@ def url_for(options)
12 12
     @controller = @controller.new
13 13
   end
14 14
 
  15
+  VALID_HTML_ID = /^[A-Za-z][-_:.A-Za-z0-9]*$/ # see http://www.w3.org/TR/html4/types.html#type-name
  16
+
15 17
   def test_check_box_tag
16 18
     actual = check_box_tag "admin"
17 19
     expected = %(<input id="admin" name="admin" type="checkbox" value="1" />)
18 20
     assert_dom_equal expected, actual
19 21
   end
20 22
 
  23
+  def test_check_box_tag_id_sanitized
  24
+    label_elem = root_elem(check_box_tag("project[2][admin]"))
  25
+    assert_match VALID_HTML_ID, label_elem['id']
  26
+  end
  27
+
21 28
   def test_form_tag
22 29
     actual = form_tag
23 30
     expected = %(<form action="http://www.example.com" method="post">)
@@ -64,6 +71,11 @@ def test_hidden_field_tag
64 71
     assert_dom_equal expected, actual
65 72
   end
66 73
 
  74
+  def test_hidden_field_tag_id_sanitized
  75
+    input_elem = root_elem(hidden_field_tag("item[][title]"))
  76
+    assert_match VALID_HTML_ID, input_elem['id']
  77
+  end
  78
+
67 79
   def test_file_field_tag
68 80
     assert_dom_equal "<input name=\"picsplz\" type=\"file\" id=\"picsplz\" />", file_field_tag("picsplz")
69 81
   end
@@ -118,6 +130,11 @@ def test_select_tag_disabled
118 130
     assert_dom_equal expected, actual
119 131
   end
120 132
 
  133
+  def test_select_tag_id_sanitized
  134
+    input_elem = root_elem(select_tag("project[1]people", "<option>david</option>"))
  135
+    assert_match VALID_HTML_ID, input_elem['id']
  136
+  end
  137
+
121 138
   def test_text_area_tag_size_string
122 139
     actual = text_area_tag "body", "hello world", "size" => "20x40"
123 140
     expected = %(<textarea cols="20" id="body" name="body" rows="40">hello world</textarea>)
@@ -184,6 +201,11 @@ def test_text_field_tag_with_multiple_options
184 201
     assert_dom_equal expected, actual
185 202
   end
186 203
 
  204
+  def test_text_field_tag_id_sanitized
  205
+    input_elem = root_elem(text_field_tag("item[][title]"))
  206
+    assert_match VALID_HTML_ID, input_elem['id']
  207
+  end
  208
+
187 209
   def test_label_tag_without_text
188 210
     actual = label_tag "title"
189 211
     expected = %(<label for="title">Title</label>)
@@ -208,11 +230,16 @@ def test_label_tag_class_string
208 230
     assert_dom_equal expected, actual
209 231
   end
210 232
 
  233
+  def test_label_tag_id_sanitized
  234
+    label_elem = root_elem(label_tag("item[title]"))
  235
+    assert_match VALID_HTML_ID, label_elem['for']
  236
+  end
  237
+
211 238
   def test_boolean_optios
212 239
     assert_dom_equal %(<input checked="checked" disabled="disabled" id="admin" name="admin" readonly="readonly" type="checkbox" value="1" />), check_box_tag("admin", 1, true, 'disabled' => true, :readonly => "yes")
213 240
     assert_dom_equal %(<input checked="checked" id="admin" name="admin" type="checkbox" value="1" />), check_box_tag("admin", 1, true, :disabled => false, :readonly => nil)
214 241
     assert_dom_equal %(<select id="people" multiple="multiple" name="people[]"><option>david</option></select>), select_tag("people", "<option>david</option>", :multiple => true)
215  
-    assert_dom_equal %(<select id="people[]" multiple="multiple" name="people[]"><option>david</option></select>), select_tag("people[]", "<option>david</option>", :multiple => true)
  242
+    assert_dom_equal %(<select id="people_" multiple="multiple" name="people[]"><option>david</option></select>), select_tag("people[]", "<option>david</option>", :multiple => true)
216 243
     assert_dom_equal %(<select id="people" name="people"><option>david</option></select>), select_tag("people", "<option>david</option>", :multiple => nil)
217 244
   end
218 245
 
@@ -283,4 +310,10 @@ def test_field_set_tag_in_erb
283 310
   def protect_against_forgery?
284 311
     false
285 312
   end
  313
+
  314
+  private
  315
+
  316
+  def root_elem(rendered_content)
  317
+    HTML::Document.new(rendered_content).root.children[0]
  318
+  end
286 319
 end

0 notes on commit 5fad229

Please sign in to comment.
Something went wrong with that request. Please try again.