From c5c46c739c5ef2b9170a67c501db7967ca814b3b Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Thu, 15 Oct 2009 11:47:25 +0000 Subject: [PATCH] - Patch #594650 by sun, c960657: provide central ()['values'] clearance. --- includes/form.inc | 65 ++++++++++++++++++++--- modules/simpletest/tests/form.test | 49 +++++++++++++++++ modules/simpletest/tests/form_test.module | 32 +++++++++++ modules/system/system.module | 2 +- modules/user/user.module | 3 +- modules/user/user.pages.inc | 3 +- 6 files changed, 143 insertions(+), 11 deletions(-) diff --git a/includes/form.inc b/includes/form.inc index 96a182f64cc7..20b3230b9795 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -1107,14 +1107,6 @@ function form_builder($form_id, $element, &$form_state) { // Internet Explorer button-click scenario. _form_builder_ie_cleanup($element, $form_state); - // We should keep the buttons array until the IE clean up function - // has recognized the submit button so the form has been marked - // as submitted. If we already know which button was submitted, - // we don't need the array. - if (!empty($form_state['submitted'])) { - unset($form_state['buttons']); - } - // If some callback set #cache, we need to flip a flag so later it // can be found. if (!empty($element['#cache'])) { @@ -1290,6 +1282,63 @@ function _form_builder_ie_cleanup($form, &$form_state) { } } +/** + * Removes internal Form API elements and buttons from submitted form values. + * + * This function can be used when a module wants to store all submitted form + * values, for example, by serializing them into a single database column. In + * such cases, all internal Form API values and all form button elements should + * not be contained, and this function allows to remove them before the module + * proceeds to storage. Next to button elements, the following internal values + * are removed: + * - form_id + * - form_token + * - form_build_id + * - op + * + * @param &$form_state + * A keyed array containing the current state of the form, including + * submitted form values; altered by reference. + */ +function form_state_values_clean(&$form_state) { + // Remove internal Form API values. + unset($form_state['values']['form_id'], $form_state['values']['form_token'], $form_state['values']['form_build_id'], $form_state['values']['op']); + + // Remove button values. + // form_builder() collects all button elements in a form, keyed by button + // type. We remove the button value separately for each button element. + foreach ($form_state['buttons'] as $button_type => $buttons) { + foreach ($buttons as $button) { + // Remove this button's value from the submitted form values by finding + // the value corresponding to this button. + // We iterate over the #parents of this button and move a reference to + // each parent in $form_state['values']. For example, if #parents is: + // array('foo', 'bar', 'baz') + // then the corresponding $form_state['values'] part will look like this: + // array( + // 'foo' => array( + // 'bar' => array( + // 'baz' => 'button_value', + // ), + // ), + // ) + // We start by (re)moving 'baz' to $last_parent, so we are able unset it + // at the end of the iteration. Initially, $values will contain a + // reference to $form_state['values'], but in the iteration we move the + // reference to $form_state['values']['foo'], and finally to + // $form_state['values']['foo']['bar'], which is the level where we can + // unset 'baz' (that is stored in $last_parent). + $parents = $button['#parents']; + $values = &$form_state['values']; + $last_parent = array_pop($parents); + foreach ($parents as $parent) { + $values = &$values[$parent]; + } + unset($values[$last_parent]); + } + } +} + /** * Helper function to determine the value for an image button form element. * diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test index fb922ec8cec9..f6dbb85b8bd3 100644 --- a/modules/simpletest/tests/form.test +++ b/modules/simpletest/tests/form.test @@ -463,3 +463,52 @@ class FormsFormWrapperTestCase extends DrupalWebTestCase { } } +/** + * Test $form_state clearance. + */ +class FormStateValuesCleanTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'Form state values clearance', + 'description' => 'Test proper removal of submitted form values using form_state_values_clean().', + 'group' => 'Form API', + ); + } + + function setUp() { + parent::setUp('form_test'); + } + + /** + * Tests form_state_values_clean(). + */ + function testFormStateValuesClean() { + $this->drupalPost('form_test/form-state-values-clean', array(), t('Submit')); + $values = json_decode($this->content, TRUE); + + // Setup the expected result. + $result = array( + 'beer' => 1000, + 'baz' => array('beer' => 2000), + ); + + // Verify that all internal Form API elements were removed. + $this->assertFalse(isset($values['form_id']), t('%element was removed.', array('%element' => 'form_id'))); + $this->assertFalse(isset($values['form_token']), t('%element was removed.', array('%element' => 'form_token'))); + $this->assertFalse(isset($values['form_build_id']), t('%element was removed.', array('%element' => 'form_build_id'))); + $this->assertFalse(isset($values['op']), t('%element was removed.', array('%element' => 'op'))); + + // Verify that all buttons were removed. + $this->assertFalse(isset($values['foo']), t('%element was removed.', array('%element' => 'foo'))); + $this->assertFalse(isset($values['bar']), t('%element was removed.', array('%element' => 'bar'))); + $this->assertFalse(isset($values['baz']['foo']), t('%element was removed.', array('%element' => 'foo'))); + $this->assertFalse(isset($values['baz']['baz']), t('%element was removed.', array('%element' => 'baz'))); + + // Verify that nested form value still exists. + $this->assertTrue(isset($values['baz']['beer']), t('Nested form value still exists.')); + + // Verify that actual form values equal resulting form values. + $this->assertEqual($values, $result, t('Expected form values equal actual form values.')); + } +} + diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module index ee47b4f1429d..a502e8e8a4c4 100644 --- a/modules/simpletest/tests/form_test.module +++ b/modules/simpletest/tests/form_test.module @@ -62,6 +62,14 @@ function form_test_menu() { 'type' => MENU_CALLBACK, ); + $items['form_test/form-state-values-clean'] = array( + 'title' => 'Form state values clearance test', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('form_test_form_state_values_clean_form'), + 'access arguments' => array('access content'), + 'type' => MENU_CALLBACK, + ); + return $items; } @@ -361,3 +369,27 @@ function form_test_wrapper_callback_form($form, &$form_state) { return $form; } +/** + * Form builder for form_state_values_clean() test. + */ +function form_test_form_state_values_clean_form($form, &$form_state) { + // Build an example form containing multiple submit and button elements; not + // only on the top-level. + $form = array('#tree' => TRUE); + $form['foo'] = array('#type' => 'submit', '#value' => t('Submit')); + $form['bar'] = array('#type' => 'submit', '#value' => t('Submit')); + $form['beer'] = array('#type' => 'value', '#value' => 1000); + $form['baz']['foo'] = array('#type' => 'button', '#value' => t('Submit')); + $form['baz']['baz'] = array('#type' => 'submit', '#value' => t('Submit')); + $form['baz']['beer'] = array('#type' => 'value', '#value' => 2000); + return $form; +} + +/** + * Form submit handler for form_state_values_clean() test form. + */ +function form_test_form_state_values_clean_form_submit($form, &$form_state) { + form_state_values_clean($form_state); + drupal_json_output($form_state['values']); + exit; +} diff --git a/modules/system/system.module b/modules/system/system.module index a4c1ff883502..a6278b485d3d 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -2332,7 +2332,7 @@ function system_settings_form($form, $automatic_defaults = TRUE) { */ function system_settings_form_submit($form, &$form_state) { // Exclude unnecessary elements. - unset($form_state['values']['submit'], $form_state['values']['reset'], $form_state['values']['form_id'], $form_state['values']['op'], $form_state['values']['form_token'], $form_state['values']['form_build_id']); + form_state_values_clean($form_state); foreach ($form_state['values'] as $key => $value) { if (is_array($value) && isset($form_state['values']['array_filter'])) { diff --git a/modules/user/user.module b/modules/user/user.module index 3a0dded2a6b9..2d400f54c2a3 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -3116,7 +3116,8 @@ function user_register_submit($form, &$form_state) { // The unset below is needed to prevent these form values from being saved as // user data. - unset($form_state['values']['form_token'], $form_state['values']['submit'], $form_state['values']['op'], $form_state['values']['notify'], $form_state['values']['form_id'], $form_state['values']['affiliates'], $form_state['values']['destination'], $form_state['values']['form_build_id']); + form_state_values_clean($form_state); + unset($form_state['values']['notify']); $form_state['values']['pass'] = $pass; $form_state['values']['init'] = $form_state['values']['mail']; diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc index fc88e395a90e..1f94e421d3b9 100644 --- a/modules/user/user.pages.inc +++ b/modules/user/user.pages.inc @@ -269,7 +269,8 @@ function user_profile_form_validate($form, &$form_state) { function user_profile_form_submit($form, &$form_state) { $account = $form['#user']; $category = $form['#user_category']; - unset($form_state['values']['op'], $form_state['values']['submit'], $form_state['values']['cancel'], $form_state['values']['form_token'], $form_state['values']['form_id'], $form_state['values']['form_build_id']); + // Remove unneeded values. + form_state_values_clean($form_state); $edit = (object)$form_state['values']; field_attach_submit('user', $edit, $form, $form_state); -- GitLab