Skip to content

Commit

Permalink
Deprecated form helper function form_prep().
Browse files Browse the repository at this point in the history
This function has been broken for YEARS and it's value-caching
logic has only introduced various problems. We have html_escape()
since CI 2.1.0 which is a perfect replacement, so it should be
used instead.

Fixes bcit-ci#228 & bcit-ci#1630
  • Loading branch information
narfbg committed Oct 26, 2012
1 parent a779b2c commit 74ffd17
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 92 deletions.
51 changes: 11 additions & 40 deletions system/helpers/form_helper.php
Expand Up @@ -149,7 +149,7 @@ function form_hidden($name, $value = '', $recursing = FALSE)

if ( ! is_array($value))
{
$form .= '<input type="hidden" name="'.$name.'" value="'.form_prep($value, $name)."\" />\n";
$form .= '<input type="hidden" name="'.$name.'" value="'.html_escape($value)."\" />\n";
}
else
{
Expand Down Expand Up @@ -263,7 +263,7 @@ function form_textarea($data = '', $value = '', $extra = '')
}

$name = is_array($data) ? $data['name'] : $data;
return '<textarea '._parse_form_attributes($data, $defaults).$extra.'>'.form_prep($val, $name)."</textarea>\n";
return '<textarea '._parse_form_attributes($data, $defaults).$extra.'>'.html_escape($val)."</textarea>\n";
}
}

Expand Down Expand Up @@ -600,44 +600,15 @@ function form_close($extra = '')
*
* Formats text so that it can be safely placed in a form field in the event it has HTML tags.
*
* @param string
* @param string
* @deprecated 3.0.0 This function has been broken for a long time
* and is now just an alias for html_escape(). It's
* second argument is ignored.
* @param string $str = ''
* @param string $field_name = ''
* @return string
*/
function form_prep($str = '', $field_name = '')
{
static $prepped_fields = array();

// if the field name is an array we do this recursively
if (is_array($str))
{
foreach ($str as $key => $val)
{
$str[$key] = form_prep($val);
}

return $str;
}

if ($str === '')
{
return '';
}

// we've already prepped a field with this name
// @todo need to figure out a way to namespace this so
// that we know the *exact* field and not just one with
// the same name
if (isset($prepped_fields[$field_name]))
{
return $str;
}

if ($field_name !== '')
{
$prepped_fields[$field_name] = $field_name;
}

return html_escape($str);
}
}
Expand All @@ -663,13 +634,13 @@ function set_value($field = '', $default = '')
{
if ( ! isset($_POST[$field]))
{
return $default;
return html_escape($default);
}

return form_prep($_POST[$field], $field);
return html_escape($_POST[$field]);
}

return form_prep($OBJ->set_value($field, $default), $field);
return html_escape($OBJ->set_value($field, $default));
}
}

Expand Down Expand Up @@ -919,7 +890,7 @@ function _parse_form_attributes($attributes, $default)
{
if ($key === 'value')
{
$val = form_prep($val, $default['name']);
$val = html_escape($val);
}
elseif ($key === 'name' && ! strlen($default['name']))
{
Expand Down
18 changes: 13 additions & 5 deletions tests/codeigniter/core/Common_test.php
Expand Up @@ -2,8 +2,6 @@

class Common_test extends CI_TestCase {

// ------------------------------------------------------------------------

public function test_is_php()
{
$this->assertEquals(TRUE, is_php('1.2.0'));
Expand All @@ -16,12 +14,12 @@ public function test_stringify_attributes()
{
$this->assertEquals(' class="foo" id="bar"', _stringify_attributes(array('class' => 'foo', 'id' => 'bar')));

$atts = new Stdclass;
$atts = new stdClass;
$atts->class = 'foo';
$atts->id = 'bar';
$this->assertEquals(' class="foo" id="bar"', _stringify_attributes($atts));

$atts = new Stdclass;
$atts = new stdClass;
$this->assertEquals('', _stringify_attributes($atts));

$this->assertEquals(' class="foo" id="bar"', _stringify_attributes('class="foo" id="bar"'));
Expand All @@ -35,10 +33,20 @@ public function test_stringify_js_attributes()
{
$this->assertEquals('width=800,height=600', _stringify_attributes(array('width' => '800', 'height' => '600'), TRUE));

$atts = new Stdclass;
$atts = new stdClass;
$atts->width = 800;
$atts->height = 600;
$this->assertEquals('width=800,height=600', _stringify_attributes($atts, TRUE));
}

// ------------------------------------------------------------------------

public function test_html_escape()
{
$this->assertEquals(
html_escape('Here is a string containing "quoted" text.'),
'Here is a string containing &quot;quoted&quot; text.'
);
}

}
39 changes: 32 additions & 7 deletions tests/codeigniter/helpers/form_helper_test.php
Expand Up @@ -7,6 +7,8 @@ public function set_up()
$this->helper('form');
}

// ------------------------------------------------------------------------

public function test_form_hidden()
{
$expected = <<<EOH
Expand All @@ -18,6 +20,8 @@ public function test_form_hidden()
$this->assertEquals($expected, form_hidden('username', 'johndoe'));
}

// ------------------------------------------------------------------------

public function test_form_input()
{
$expected = <<<EOH
Expand All @@ -37,6 +41,8 @@ public function test_form_input()
$this->assertEquals($expected, form_input($data));
}

// ------------------------------------------------------------------------

public function test_form_password()
{
$expected = <<<EOH
Expand All @@ -47,6 +53,8 @@ public function test_form_password()
$this->assertEquals($expected, form_password('password'));
}

// ------------------------------------------------------------------------

public function test_form_upload()
{
$expected = <<<EOH
Expand All @@ -57,6 +65,8 @@ public function test_form_upload()
$this->assertEquals($expected, form_upload('attachment'));
}

// ------------------------------------------------------------------------

public function test_form_textarea()
{
$expected = <<<EOH
Expand All @@ -67,6 +77,8 @@ public function test_form_textarea()
$this->assertEquals($expected, form_textarea('notes', 'Notes'));
}

// ------------------------------------------------------------------------

public function test_form_dropdown()
{
$expected = <<<EOH
Expand Down Expand Up @@ -130,6 +142,8 @@ public function test_form_dropdown()
$this->assertEquals($expected, form_dropdown('cars', $options, array('volvo', 'audi')));
}

// ------------------------------------------------------------------------

public function test_form_multiselect()
{
$expected = <<<EOH
Expand All @@ -152,6 +166,8 @@ public function test_form_multiselect()
$this->assertEquals($expected, form_multiselect('shirts[]', $options, array('med', 'large')));
}

// ------------------------------------------------------------------------

public function test_form_fieldset()
{
$expected = <<<EOH
Expand All @@ -163,6 +179,8 @@ public function test_form_fieldset()
$this->assertEquals($expected, form_fieldset('Address Information'));
}

// ------------------------------------------------------------------------

public function test_form_fieldset_close()
{
$expected = <<<EOH
Expand All @@ -172,6 +190,8 @@ public function test_form_fieldset_close()
$this->assertEquals($expected, form_fieldset_close('</div></div>'));
}

// ------------------------------------------------------------------------

public function test_form_checkbox()
{
$expected = <<<EOH
Expand All @@ -182,6 +202,8 @@ public function test_form_checkbox()
$this->assertEquals($expected, form_checkbox('newsletter', 'accept', TRUE));
}

// ------------------------------------------------------------------------

public function test_form_radio()
{
$expected = <<<EOH
Expand All @@ -192,6 +214,8 @@ public function test_form_radio()
$this->assertEquals($expected, form_radio('newsletter', 'accept', TRUE));
}

// ------------------------------------------------------------------------

public function test_form_submit()
{
$expected = <<<EOH
Expand All @@ -202,6 +226,8 @@ public function test_form_submit()
$this->assertEquals($expected, form_submit('mysubmit', 'Submit Post!'));
}

// ------------------------------------------------------------------------

public function test_form_label()
{
$expected = <<<EOH
Expand All @@ -211,6 +237,8 @@ public function test_form_label()
$this->assertEquals($expected, form_label('What is your Name', 'username'));
}

// ------------------------------------------------------------------------

public function test_form_reset()
{
$expected = <<<EOH
Expand All @@ -221,6 +249,8 @@ public function test_form_reset()
$this->assertEquals($expected, form_reset('myreset', 'Reset'));
}

// ------------------------------------------------------------------------

public function test_form_button()
{
$expected = <<<EOH
Expand All @@ -231,6 +261,8 @@ public function test_form_button()
$this->assertEquals($expected, form_button('name', 'content'));
}

// ------------------------------------------------------------------------

public function test_form_close()
{
$expected = <<<EOH
Expand All @@ -240,13 +272,6 @@ public function test_form_close()
$this->assertEquals($expected, form_close('</div></div>'));
}

public function test_form_prep()
{
$expected = 'Here is a string containing &quot;quoted&quot; text.';

$this->assertEquals($expected, form_prep('Here is a string containing "quoted" text.'));
}

}

/* End of file form_helper_test.php */
5 changes: 4 additions & 1 deletion user_guide_src/source/changelog.rst
Expand Up @@ -75,7 +75,9 @@ Release Date: Not Released
- Refactored ``plural()`` and ``singular()`` to avoid double pluralization and support more words.
- Added an optional third parameter to ``force_download()`` that enables/disables sending the actual file MIME type in the Content-Type header (disabled by default).
- Added a work-around in ``force_download()`` for a bug Android <= 2.1, where the filename extension needs to be in uppercase.
- ``form_dropdown()`` will now also take an array for unity with other form helpers.
- :doc:`Form Helper <helpers/form_helper>` changes include:
- ``form_dropdown()`` will now also take an array for unity with other form helpers.
- ``form_prep()`` is now **DEPRECATED** and only acts as an alias for :doc:`common function <general/common_functions>` ``html_escape()``.
- ``do_hash()`` now uses PHP's native ``hash()`` function (supporting more algorithms) and is deprecated.
- Removed previously deprecated helper function ``js_insert_smiley()`` from :doc:`Smiley Helper <helpers/smiley_helper>`.
- :doc:`File Helper <helpers/file_helper>` changes include:
Expand Down Expand Up @@ -387,6 +389,7 @@ Bug fixes for 3.0
- Fixed a bug (#1506) - :doc:`Form Helpers <helpers/form_helper>` set empty *name* attributes.
- Fixed a bug (#59) - :doc:`Query Builder <database/query_builder>` method ``count_all_results()`` ignored the DISTINCT clause.
- Fixed a bug (#1624) - :doc:`Form Validation Library <libraries/form_validation>` rule **matches** didn't property handle array field names.
- Fixed a bug (#1630) - :doc:`Form Helper <helpers/form_helper>` function ``set_value()`` didn't escape HTML entities.

Version 2.1.3
=============
Expand Down
47 changes: 23 additions & 24 deletions user_guide_src/source/helpers/form_helper.rst
Expand Up @@ -463,29 +463,6 @@ the tag. For example
echo form_close($string);
// Would produce: </form> </div></div>

form_prep()
===========

Allows you to safely use HTML and characters such as quotes within form
elements without breaking out of the form. Consider this example

::

$string = 'Here is a string containing "quoted" text.';
<input type="text" name="myform" value="$string" />

Since the above string contains a set of quotes it will cause the form
to break. The `form_prep()` function converts HTML so that it can be used
safely

::

<input type="text" name="myform" value="<?php echo form_prep($string); ?>" />

.. note:: If you use any of the form helper functions listed in this page the form
values will be prepped automatically, so there is no need to call this
function. Use it only if you are creating your own form elements.

set_value()
===========

Expand Down Expand Up @@ -546,4 +523,26 @@ This function is identical to the **set_checkbox()** function above.
.. note:: If you are using the Form Validation class, you must always specify a rule for your field,
even if empty, in order for the set_*() functions to work. This is because if a Form Validation object
is defined, the control for set_*() is handed over to a method of the class instead of the generic helper
function.
function.

Escaping field values
=====================

You may need to use HTML and characters such as quotes within form
elements. In order to do that safely, you'll need to use
:doc:`common function <../general/common_functions>` ``html_escape()``.

Consider the following example::

$string = 'Here is a string containing "quoted" text.';
<input type="text" name="myform" value="$string" />

Since the above string contains a set of quotes it will cause the form
to break. The ``html_escape()`` function converts HTML so that it can be
used safely::

<input type="text" name="myform" value="<?php echo html_escape($string); ?>" />

.. note:: If you use any of the form helper functions listed in this page, the form
values will be prepped automatically, so there is no need to call this
function. Use it only if you are creating your own form elements.

0 comments on commit 74ffd17

Please sign in to comment.