From 74ffd17ab06327ca62ddfe28a186cae7ba6bd459 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Sat, 27 Oct 2012 00:41:03 +0300 Subject: [PATCH] Deprecated form helper function form_prep(). 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 #228 & #1630 --- system/helpers/form_helper.php | 51 ++++--------------- tests/codeigniter/core/Common_test.php | 18 +++++-- .../codeigniter/helpers/form_helper_test.php | 39 +++++++++++--- user_guide_src/source/changelog.rst | 5 +- user_guide_src/source/helpers/form_helper.rst | 47 +++++++++-------- .../source/installation/upgrade_300.rst | 43 ++++++++++------ 6 files changed, 111 insertions(+), 92 deletions(-) diff --git a/system/helpers/form_helper.php b/system/helpers/form_helper.php index 3cce8688f57..d81bb7c08e2 100644 --- a/system/helpers/form_helper.php +++ b/system/helpers/form_helper.php @@ -149,7 +149,7 @@ function form_hidden($name, $value = '', $recursing = FALSE) if ( ! is_array($value)) { - $form .= '\n"; + $form .= '\n"; } else { @@ -263,7 +263,7 @@ function form_textarea($data = '', $value = '', $extra = '') } $name = is_array($data) ? $data['name'] : $data; - return '\n"; + return '\n"; } } @@ -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); } } @@ -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)); } } @@ -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'])) { diff --git a/tests/codeigniter/core/Common_test.php b/tests/codeigniter/core/Common_test.php index 27d48efc296..999b49cb36b 100644 --- a/tests/codeigniter/core/Common_test.php +++ b/tests/codeigniter/core/Common_test.php @@ -2,8 +2,6 @@ class Common_test extends CI_TestCase { - // ------------------------------------------------------------------------ - public function test_is_php() { $this->assertEquals(TRUE, is_php('1.2.0')); @@ -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"')); @@ -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 "quoted" text.' + ); + } + } \ No newline at end of file diff --git a/tests/codeigniter/helpers/form_helper_test.php b/tests/codeigniter/helpers/form_helper_test.php index 48628d2e5ce..03278581d68 100644 --- a/tests/codeigniter/helpers/form_helper_test.php +++ b/tests/codeigniter/helpers/form_helper_test.php @@ -7,6 +7,8 @@ public function set_up() $this->helper('form'); } + // ------------------------------------------------------------------------ + public function test_form_hidden() { $expected = <<assertEquals($expected, form_hidden('username', 'johndoe')); } + // ------------------------------------------------------------------------ + public function test_form_input() { $expected = <<assertEquals($expected, form_input($data)); } + // ------------------------------------------------------------------------ + public function test_form_password() { $expected = <<assertEquals($expected, form_password('password')); } + // ------------------------------------------------------------------------ + public function test_form_upload() { $expected = <<assertEquals($expected, form_upload('attachment')); } + // ------------------------------------------------------------------------ + public function test_form_textarea() { $expected = <<assertEquals($expected, form_textarea('notes', 'Notes')); } + // ------------------------------------------------------------------------ + public function test_form_dropdown() { $expected = <<assertEquals($expected, form_dropdown('cars', $options, array('volvo', 'audi'))); } + // ------------------------------------------------------------------------ + public function test_form_multiselect() { $expected = <<assertEquals($expected, form_multiselect('shirts[]', $options, array('med', 'large'))); } + // ------------------------------------------------------------------------ + public function test_form_fieldset() { $expected = <<assertEquals($expected, form_fieldset('Address Information')); } + // ------------------------------------------------------------------------ + public function test_form_fieldset_close() { $expected = <<assertEquals($expected, form_fieldset_close('')); } + // ------------------------------------------------------------------------ + public function test_form_checkbox() { $expected = <<assertEquals($expected, form_checkbox('newsletter', 'accept', TRUE)); } + // ------------------------------------------------------------------------ + public function test_form_radio() { $expected = <<assertEquals($expected, form_radio('newsletter', 'accept', TRUE)); } + // ------------------------------------------------------------------------ + public function test_form_submit() { $expected = <<assertEquals($expected, form_submit('mysubmit', 'Submit Post!')); } + // ------------------------------------------------------------------------ + public function test_form_label() { $expected = <<assertEquals($expected, form_label('What is your Name', 'username')); } + // ------------------------------------------------------------------------ + public function test_form_reset() { $expected = <<assertEquals($expected, form_reset('myreset', 'Reset')); } + // ------------------------------------------------------------------------ + public function test_form_button() { $expected = <<assertEquals($expected, form_button('name', 'content')); } + // ------------------------------------------------------------------------ + public function test_form_close() { $expected = <<assertEquals($expected, form_close('')); } - public function test_form_prep() - { - $expected = 'Here is a string containing "quoted" text.'; - - $this->assertEquals($expected, form_prep('Here is a string containing "quoted" text.')); - } - } /* End of file form_helper_test.php */ \ No newline at end of file diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 0d832425cbc..54338f3ee7c 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -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 ` 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 ` ``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 `. - :doc:`File Helper ` changes include: @@ -387,6 +389,7 @@ Bug fixes for 3.0 - Fixed a bug (#1506) - :doc:`Form Helpers ` set empty *name* attributes. - Fixed a bug (#59) - :doc:`Query Builder ` method ``count_all_results()`` ignored the DISTINCT clause. - Fixed a bug (#1624) - :doc:`Form Validation Library ` rule **matches** didn't property handle array field names. +- Fixed a bug (#1630) - :doc:`Form Helper ` function ``set_value()`` didn't escape HTML entities. Version 2.1.3 ============= diff --git a/user_guide_src/source/helpers/form_helper.rst b/user_guide_src/source/helpers/form_helper.rst index fa7b3dbf915..015bf11626a 100644 --- a/user_guide_src/source/helpers/form_helper.rst +++ b/user_guide_src/source/helpers/form_helper.rst @@ -463,29 +463,6 @@ the tag. For example echo form_close($string); // Would produce: -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.'; - - -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 - -:: - - - -.. 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() =========== @@ -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. \ No newline at end of file + 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.'; + + +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:: + + + +.. 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. \ No newline at end of file diff --git a/user_guide_src/source/installation/upgrade_300.rst b/user_guide_src/source/installation/upgrade_300.rst index 31a5c0761f6..952108356cc 100644 --- a/user_guide_src/source/installation/upgrade_300.rst +++ b/user_guide_src/source/installation/upgrade_300.rst @@ -71,8 +71,24 @@ Step 7: Check the calls to Array Helper's element() and elements() functions The default return value of these functions, when the required elements don't exist, has been changed from FALSE to NULL. +********************************************************** +Step 8: Change usage of Email library with multiple emails +********************************************************** + +The :doc:`Email library <../libraries/email>` will automatically clear the +set parameters after successfully sending emails. To override this behaviour, +pass FALSE as the first parameter in the ``send()`` method: + +:: + + if ($this->email->send(FALSE)) + { + // Parameters won't be cleared + } + + *************************************************************** -Step 8: Remove usage of (previously) deprecated functionalities +Step 9: Remove usage of (previously) deprecated functionalities *************************************************************** In addition to the ``$autoload['core']`` configuration setting, there's a number of other functionalities @@ -115,6 +131,16 @@ File helper read_file() PHP's native ``file_get_contents()`` function. It is deprecated and scheduled for removal in CodeIgniter 3.1+. +.. note:: This function is still available, but you're strongly encouraged to remove it's usage sooner + rather than later. + +Form helper form_prep() +======================= + +:doc:`Form Helper <../helpers/form_helper>` function ``form_prep()`` is now just an alias for +:doc:`common function <../general/common_functions>` ``html_escape()`` and it's second argument +is ignored. It is deprecated and scheduled for removal in CodeIgniter 3.1+. + .. note:: This function is still available, but you're strongly encouraged to remove it's usage sooner rather than later. @@ -154,17 +180,4 @@ As a result of that, the 'anchor_class' setting is now deprecated and scheduled CodeIgniter 3.1+. .. note:: This setting is still available, but you're strongly encouraged to remove its' usage sooner - rather than later. - -Email library -============= - -The :doc:`Email library <../libraries/email>` will automatically clear the set parameters after successfully sending -emails. To override this behaviour, pass FALSE as the first parameter in the ``send()`` function: - -:: - - if ($this->email->send(FALSE)) - { - // Parameters won't be cleared - } + rather than later. \ No newline at end of file