diff --git a/includes/form.inc b/includes/form.inc
index 3d60afee37f0a91c39f542347c9693860a891c72..e33a19d8dd7439508f4c2cf45b15f3f278b41648 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -1337,10 +1337,83 @@ function form_error(&$element, $message = '') {
}
/**
- * Walk through the structured form array, adding any required
- * properties to each element and mapping the incoming input
- * data to the proper elements. Also, execute any #process handlers
- * attached to a specific element.
+ * Walk through the structured form array, adding any required properties to
+ * each element and mapping the incoming input data to the proper elements.
+ * Also, execute any #process handlers attached to a specific element.
+ *
+ * This is one of the three primary functions that recursively iterates a form
+ * array. This one does it for completing the form building process. The other
+ * two are _form_validate() (invoked via drupal_validate_form() and used to
+ * invoke validation logic for each element) and drupal_render() (for rendering
+ * each element). Each of these three pipelines provides ample opportunity for
+ * modules to customize what happens. For example, during this function's life
+ * cycle, the following functions get called for each element:
+ * - $element['#value_callback']: A function that implements how user input is
+ * mapped to an element's #value property. This defaults to a function named
+ * 'form_type_TYPE_value' where TYPE is $element['#type'].
+ * - $element['#process']: An array of functions called after user input has
+ * been mapped to the element's #value property. These functions can be used
+ * to dynamically add child elements: for example, for the 'date' element
+ * type, one of the functions in this array is form_process_date(), which adds
+ * the individual 'year', 'month', 'day', etc. child elements. These functions
+ * can also be used to set additional properties or implement special logic
+ * other than adding child elements: for example, for the 'fieldset' element
+ * type, one of the functions in this array is form_process_fieldset(), which
+ * adds the attributes and JavaScript needed to make the fieldset collapsible
+ * if the #collapsible property is set. The #process functions are called in
+ * preorder traversal, meaning they are called for the parent element first,
+ * then for the child elements.
+ * - $element['#after_build']: An array of functions called after form_builder()
+ * is done with its processing of the element. These are called in postorder
+ * traversal, meaning they are called for the child elements first, then for
+ * the parent element.
+ * There are similar properties containing callback functions invoked by
+ * _form_validate() and drupal_render(), appropriate for those operations.
+ *
+ * Developers are strongly encouraged to integrate the functionality needed by
+ * their form or module within one of these three pipelines, using the
+ * appropriate callback property, rather than implementing their own recursive
+ * traversal of a form array. This facilitates proper integration between
+ * multiple modules. For example, module developers are familiar with the
+ * relative order in which hook_form_alter() implementations and #process
+ * functions run. A custom traversal function that affects the building of a
+ * form is likely to not integrate with hook_form_alter() and #process in the
+ * expected way. Also, deep recursion within PHP is both slow and memory
+ * intensive, so it is best to minimize how often it's done.
+ *
+ * As stated above, each element's #process functions are executed after its
+ * #value has been set. This enables those functions to execute conditional
+ * logic based on the current value. However, all of form_builder() runs before
+ * drupal_validate_form() is called, so during #process function execution, the
+ * element's #value has not yet been validated, so any code that requires
+ * validated values must reside within a submit handler.
+ *
+ * As a security measure, user input is used for an element's #value only if the
+ * element exists within $form, is not disabled (as per the #disabled property),
+ * and can be accessed (as per the #access property, except that forms submitted
+ * using drupal_form_submit() bypass #access restrictions). When user input is
+ * ignored due to #disabled and #access restrictions, the element's default
+ * value is used.
+ *
+ * Because of the preorder traversal, where #process functions of an element run
+ * before user input for its child elements is processed, and because of the
+ * Form API security of user input processing with respect to #access and
+ * #disabled described above, this generally means that #process functions
+ * should not use an element's (unvalidated) #value to affect the #disabled or
+ * #access of child elements. Use-cases where a developer may be tempted to
+ * implement such conditional logic usually fall into one of two categories:
+ * - Where user input from the current submission must affect the structure of a
+ * form, including properties like #access and #disabled that affect how the
+ * next submission needs to be processed, a multi-step workflow is needed.
+ * This is most commonly implemented with a submit handler setting persistent
+ * data within $form_state based on *validated* values in
+ * $form_state['values'] and setting $form_state['rebuild']. The form building
+ * functions must then be implmented to use the $form_state data to rebuild
+ * the form with the structure appropriate for the new state.
+ * - Where user input must affect the rendering of the form without affecting
+ * its structure, the necessary conditional rendering logic should reside
+ * within functions that run during the rendering phase (#pre_render, #theme,
+ * #theme_wrappers, and #post_render).
*
* @param $form_id
* A unique string identifying the form for validation, submission,
@@ -1572,19 +1645,20 @@ function _form_builder_handle_input_element($form_id, &$element, &$form_state) {
}
}
+ // With JavaScript or other easy hacking, input can be submitted even for
+ // elements with #access=FALSE or #disabled=TRUE. For security, these must
+ // not be processed. Forms that set #disabled=TRUE on an element do not
+ // expect input for the element, and even forms submitted with
+ // drupal_form_submit() must not be able to get around this. Forms that set
+ // #access=FALSE on an element usually allow access for some users, so forms
+ // submitted with drupal_form_submit() may bypass access restriction and be
+ // treated as high-privelege users instead.
+ $process_input = empty($element['#disabled']) && ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access'])));
+
// Set the element's #value property.
if (!isset($element['#value']) && !array_key_exists('#value', $element)) {
$value_callback = !empty($element['#value_callback']) ? $element['#value_callback'] : 'form_type_' . $element['#type'] . '_value';
-
- // With JavaScript or other easy hacking, input can be submitted even for
- // elements with #access=FALSE or #disabled=TRUE. For security, these must
- // not be processed. Forms that set #disabled=TRUE on an element do not
- // expect input for the element, and even forms submitted with
- // drupal_form_submit() must not be able to get around this. Forms that set
- // #access=FALSE on an element usually allow access for some users, so forms
- // submitted with drupal_form_submit() may bypass access restriction and be
- // treated as high-privelege users instead.
- if (empty($element['#disabled']) && ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access'])))) {
+ if ($process_input) {
// Get the input for the current element. NULL values in the input need to
// be explicitly distinguished from missing input. (see below)
$input = $form_state['input'];
@@ -1647,18 +1721,10 @@ function _form_builder_handle_input_element($form_id, &$element, &$form_state) {
}
// Determine which element (if any) triggered the submission of the form and
- // keep track of all the buttons in the form for form_state_values_clean().
- // @todo We need to add a #access check here, so that someone can't fake the
- // click of a button they shouldn't have access to, but first we need to
- // fix file.module's managed_file element pipeline to handle the click of
- // the remove button in a submit handler instead of in a #process function.
- // During the first run of form_builder() after the form is submitted,
- // #process functions need to return the expanded element with child
- // elements' #access properties matching what they were when the form was
- // displayed to the user, since that is what we are processing input for.
- // Changes to the form (like toggling the upload/remove button) need to wait
- // until form rebuild: http://drupal.org/node/736298.
- if (!empty($form_state['input'])) {
+ // keep track of all the clickable buttons in the form for
+ // form_state_values_clean(). Enforce the same input processing restrictions
+ // as above.
+ if ($process_input) {
// Detect if the element triggered the submission via AJAX.
if (_form_element_triggered_scripted_submission($element, $form_state)) {
$form_state['triggering_element'] = $element;
@@ -1671,11 +1737,7 @@ function _form_builder_handle_input_element($form_id, &$element, &$form_state) {
// All buttons in the form need to be tracked for
// form_state_values_clean() and for the form_builder() code that handles
// a form submission containing no button information in $_POST.
- // @todo When #access is checked in an outer if statement (see above), it
- // won't need to be checked here.
- if ($form_state['programmed'] || !isset($element['#access']) || $element['#access']) {
- $form_state['buttons'][] = $element;
- }
+ $form_state['buttons'][] = $element;
if (_form_button_was_clicked($element, $form_state)) {
$form_state['triggering_element'] = $element;
}
diff --git a/modules/file/file.field.inc b/modules/file/file.field.inc
index 12420d846fc7422006a6a7ea291f6fd551f104f9..e0d452bd1ba8ba865b791124841f3b52f62b5922 100644
--- a/modules/file/file.field.inc
+++ b/modules/file/file.field.inc
@@ -767,32 +767,45 @@ function theme_file_widget_multiple($variables) {
continue;
}
- // Render all the buttons in the field as an "operation".
- $operations = '';
+ // Delay rendering of the buttons, so that they can be rendered later in the
+ // "operations" column.
+ $operations_elements = array();
foreach (element_children($element[$key]) as $sub_key) {
if (isset($element[$key][$sub_key]['#type']) && $element[$key][$sub_key]['#type'] == 'submit') {
- $operations .= drupal_render($element[$key][$sub_key]);
+ hide($element[$key][$sub_key]);
+ $operations_elements[] = &$element[$key][$sub_key];
}
}
- // Render the "Display" option in its own own column.
+ // Delay rendering of the "Display" option and the weight selector, so that
+ // each can be rendered later in its own column.
+ if ($element['#display_field']) {
+ hide($element[$key]['display']);
+ }
+ hide($element[$key]['_weight']);
+
+ // Render everything else together in a column, without the normal wrappers.
+ $element[$key]['#theme_wrappers'] = array();
+ $information = drupal_render($element[$key]);
+
+ // Render the previously hidden elements, using render() instead of
+ // drupal_render(), to undo the earlier hide().
+ $operations = '';
+ foreach ($operations_elements as $operation_element) {
+ $operations .= render($operation_element);
+ }
$display = '';
if ($element['#display_field']) {
unset($element[$key]['display']['#title']);
$display = array(
- 'data' => drupal_render($element[$key]['display']),
+ 'data' => render($element[$key]['display']),
'class' => array('checkbox'),
);
}
-
- // Render the weight in its own column.
$element[$key]['_weight']['#attributes']['class'] = array($weight_class);
- $weight = drupal_render($element[$key]['_weight']);
-
- // Render everything else together in a column, without the normal wrappers.
- $element[$key]['#theme_wrappers'] = array();
- $information = drupal_render($element[$key]);
+ $weight = render($element[$key]['_weight']);
+ // Arrange the row with all of the rendered columns.
$row = array();
$row[] = $information;
if ($element['#display_field']) {
diff --git a/modules/file/file.module b/modules/file/file.module
index 2f73d72675c7941f5a4f4ffac60a9d7d6153bfd3..8d923ac6f391ea5e953a331bb7cf4b2c09c58b44 100644
--- a/modules/file/file.module
+++ b/modules/file/file.module
@@ -65,6 +65,7 @@ function file_element_info() {
'#process' => array('file_managed_file_process'),
'#value_callback' => 'file_managed_file_value',
'#element_validate' => array('file_managed_file_validate'),
+ '#pre_render' => array('file_managed_file_pre_render'),
'#theme' => 'file_managed_file',
'#theme_wrappers' => array('form_element'),
'#progress_indicator' => 'throbber',
@@ -384,25 +385,6 @@ function file_managed_file_process($element, &$form_state, $form) {
'#weight' => -5,
);
- // @todo It is not good to call these private functions. This should be
- // refactored so that the file deletion happens during a submit handler,
- // and form changes affected by that (such as toggling the upload and remove
- // buttons) happens during the 2nd run of this function that is triggered by
- // a form rebuild: http://drupal.org/node/736298.
- if (_form_button_was_clicked($element['remove_button'], $form_state) || _form_element_triggered_scripted_submission($element['remove_button'], $form_state)) {
- // If it's a temporary file we can safely remove it immediately, otherwise
- // it's up to the implementing module to clean up files that are in use.
- if ($element['#file'] && $element['#file']->status == 0) {
- file_delete($element['#file']);
- }
- $element['#file'] = FALSE;
- $fid = 0;
- }
-
- // Set access on the buttons.
- $element['upload_button']['#access'] = empty($fid);
- $element['remove_button']['#access'] = !empty($fid);
-
$element['fid'] = array(
'#type' => 'hidden',
'#value' => $fid,
@@ -436,7 +418,6 @@ function file_managed_file_process($element, &$form_state, $form) {
'#name' => 'files[' . implode('_', $element['#parents']) . ']',
'#type' => 'file',
'#size' => 22,
- '#access' => empty($fid),
'#theme_wrappers' => array(),
'#weight' => -10,
);
@@ -565,13 +546,50 @@ function file_managed_file_validate(&$element, &$form_state) {
}
/**
- * Submit handler for non-JavaScript uploads.
+ * Submit handler for upload and remove buttons of managed_file elements.
*/
function file_managed_file_submit($form, &$form_state) {
- // Do not redirect and leave the page after uploading a file. This keeps
- // all the current form values in place. The file is saved by the
- // #value_callback on the form element.
- $form_state['redirect'] = FALSE;
+ // Determine whether it was the upload or the remove button that was clicked,
+ // and set $element to the managed_file element that contains that button.
+ $parents = $form_state['triggering_element']['#array_parents'];
+ $button_key = array_pop($parents);
+ $element = $form;
+ foreach ($parents as $parent) {
+ $element = $element[$parent];
+ }
+
+ // No action is needed here for the upload button, because all file uploads on
+ // the form are processed by file_managed_file_value() regardless of which
+ // button was clicked. Action is needed here for the remove button, because we
+ // only remove a file in response to its remove button being clicked.
+ if ($button_key == 'remove_button') {
+ // If it's a temporary file we can safely remove it immediately, otherwise
+ // it's up to the implementing module to clean up files that are in use.
+ if ($element['#file'] && $element['#file']->status == 0) {
+ file_delete($element['#file']);
+ }
+ // Update both $form_state['values'] and $form_state['input'] to reflect
+ // that the file has been removed, so that the form is rebuilt correctly.
+ // $form_state['values'] must be updated in case additional submit handlers
+ // run, and for form building functions that run during the rebuild, such as
+ // when the managed_file element is part of a field widget.
+ // $form_state['input'] must be updated so that file_managed_file_value()
+ // has correct information during the rebuild. The Form API provides no
+ // equivalent of form_set_value() for updating $form_state['input'], so
+ // inline that implementation with the same logic that form_set_value()
+ // uses.
+ $values_element = $element['#extended'] ? $element['fid'] : $element;
+ form_set_value($values_element, NULL, $form_state);
+ _form_set_value($form_state['input'], $values_element, $values_element['#parents'], NULL);
+ }
+
+ // Set the form to rebuild so that $form is correctly updated in response to
+ // processing the file removal. Since this function did not change $form_state
+ // if the upload button was clicked, a rebuild isn't necessary in that
+ // situation and setting $form_state['redirect'] to FALSE would suffice.
+ // However, we choose to always rebuild, to keep the form processing workflow
+ // consistent between the two buttons.
+ $form_state['rebuild'] = TRUE;
}
/**
@@ -625,6 +643,38 @@ function theme_file_managed_file($variables) {
return $output;
}
+/**
+ * #pre_render callback to hide display of the upload or remove controls.
+ *
+ * Upload controls are hidden when a file is already uploaded. Remove controls
+ * are hidden when there is no file attached. Controls are hidden here instead
+ * of in file_managed_file_process(), because #access for these buttons depends
+ * on the managed_file element's #value. See the documentation of form_builder()
+ * for more detailed information about the relationship between #process,
+ * #value, and #access.
+ *
+ * Because #access is set here, it affects display only and does not prevent
+ * JavaScript or other untrusted code from submitting the form as though access
+ * were enabled. The form processing functions for these elements should not
+ * assume that the buttons can't be "clicked" just because they are not
+ * displayed.
+ *
+ * @see file_managed_file_process()
+ * @see form_builder()
+ */
+function file_managed_file_pre_render($element) {
+ // If we already have a file, we don't want to show the upload controls.
+ if (!empty($element['#value']['fid'])) {
+ $element['upload']['#access'] = FALSE;
+ $element['upload_button']['#access'] = FALSE;
+ }
+ // If we don't already have a file, there is nothing to remove.
+ else {
+ $element['remove_button']['#access'] = FALSE;
+ }
+ return $element;
+}
+
/**
* Returns HTML for a link to a file.
*
diff --git a/modules/file/tests/file.test b/modules/file/tests/file.test
index b228cb329c725a1b38dcc87711ea10aba2acf0eb..ee01ffa613524ec5da846dcc2d8e1153a76169c1 100644
--- a/modules/file/tests/file.test
+++ b/modules/file/tests/file.test
@@ -14,7 +14,7 @@ class FileFieldTestCase extends DrupalWebTestCase {
function setUp() {
parent::setUp('file');
- $this->admin_user = $this->drupalCreateUser(array('access content', 'access administration pages', 'administer site configuration', 'administer users', 'administer content types', 'administer nodes', 'create article content', 'edit any article content', 'delete any article content'));
+ $this->admin_user = $this->drupalCreateUser(array('access content', 'access administration pages', 'administer site configuration', 'administer users', 'administer content types', 'administer nodes', 'bypass node access'));
$this->drupalLogin($this->admin_user);
}
@@ -202,6 +202,84 @@ class FileFieldTestCase extends DrupalWebTestCase {
}
}
+
+/**
+ * Test class to test file field upload and remove buttons, with and without AJAX.
+ */
+class FileFieldWidgetTestCase extends FileFieldTestCase {
+ public static function getInfo() {
+ return array(
+ 'name' => 'File field widget test',
+ 'description' => 'Test upload and remove buttons, with and without AJAX.',
+ 'group' => 'File',
+ );
+ }
+
+ /**
+ * Tests upload and remove buttons, with and without AJAX.
+ *
+ * @todo This function currently only tests the "remove" button of a single-
+ * valued field. Tests should be added for the "upload" button and for each
+ * button of a multi-valued field. Tests involving multiple AJAX steps on
+ * the same page will become easier after http://drupal.org/node/789186
+ * lands. Testing the "upload" button in AJAX context requires more
+ * investigation into how jQuery uploads files, so that drupalPostAJAX() can
+ * emulate that correctly.
+ */
+ function testWidget() {
+ // Use 'page' instead of 'article', so that the 'article' image field does
+ // not conflict with this test. If in the future the 'page' type gets its
+ // own default file or image field, this test can be made more robust by
+ // using a custom node type.
+ $type_name = 'page';
+ $field_name = strtolower($this->randomName());
+ $this->createFileField($field_name, $type_name);
+ $field = field_info_field($field_name);
+ $instance = field_info_instance('node', $field_name, $type_name);
+
+ $test_file = $this->getTestFile('text');
+
+ foreach (array('nojs', 'js') as $type) {
+ // Create a new node with the uploaded file and ensure it got uploaded
+ // successfully.
+ $nid = $this->uploadNodeFile($test_file, $field_name, $type_name);
+ $node = node_load($nid, NULL, TRUE);
+ $node_file = (object) $node->{$field_name}[LANGUAGE_NONE][0];
+ $this->assertFileExists($node_file, t('New file saved to disk on node creation.'));
+
+ // Ensure the edit page has a remove button instead of an upload button.
+ $this->drupalGet("node/$nid/edit");
+ $this->assertNoFieldByXPath('//input[@type="submit"]', t('Upload'), t('Node with file does not display the "Upload" button.'));
+ $this->assertFieldByXpath('//input[@type="submit"]', t('Remove'), t('Node with file displays the "Remove" button.'));
+
+ // "Click" the remove button (emulating either a nojs or js submission).
+ switch ($type) {
+ case 'nojs':
+ $this->drupalPost(NULL, array(), t('Remove'));
+ break;
+ case 'js':
+ // @todo This can be simplified after http://drupal.org/node/789186
+ // lands.
+ preg_match('/jQuery\.extend\(Drupal\.settings, (.*?)\);/', $this->content, $matches);
+ $settings = drupal_json_decode($matches[1]);
+ $button = $this->xpath('//input[@type="submit" and @value="' . t('Remove') . '"]');
+ $button_id = (string) $button[0]['id'];
+ $this->drupalPostAJAX(NULL, array(), array((string) $button[0]['name'] => (string) $button[0]['value']), $settings['ajax'][$button_id]['url'], array(), array(), NULL, $settings['ajax'][$button_id]);
+ break;
+ }
+
+ // Ensure the page now has an upload button instead of a remove button.
+ $this->assertNoFieldByXPath('//input[@type="submit"]', t('Remove'), t('After clicking the "Remove" button, it is no longer displayed.'));
+ $this->assertFieldByXpath('//input[@type="submit"]', t('Upload'), t('After clicking the "Remove" button, the "Upload" button is displayed.'));
+
+ // Save the node and ensure it does not have the file.
+ $this->drupalPost(NULL, array(), t('Save'));
+ $node = node_load($nid, NULL, TRUE);
+ $this->assertTrue(empty($node->{$field_name}[LANGUAGE_NONE][0]['fid']), t('File was successfully removed from the node.'));
+ }
+ }
+}
+
/**
* Test class to test file handling with node revisions.
*/
diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test
index fb46d94e5a1882ccc6a68b3a211e93c035743ef2..8821ec97ceb62a56bd74328d636fe36ff49da440 100644
--- a/modules/simpletest/tests/form.test
+++ b/modules/simpletest/tests/form.test
@@ -996,14 +996,14 @@ class FormsProgrammaticTestCase extends DrupalWebTestCase {
}
/**
- * Test that FAPI correctly determines $form_state['clicked_button'].
+ * Test that FAPI correctly determines $form_state['triggering_element'].
*/
-class FormsClickedButtonTestCase extends DrupalWebTestCase {
+class FormsTriggeringElementTestCase extends DrupalWebTestCase {
public static function getInfo() {
return array(
- 'name' => 'Form clicked button determination',
- 'description' => 'Test the determination of $form_state[\'clicked_button\'].',
+ 'name' => 'Form triggering element determination',
+ 'description' => 'Test the determination of $form_state[\'triggering_element\'].',
'group' => 'Form API',
);
}
@@ -1013,59 +1013,85 @@ class FormsClickedButtonTestCase extends DrupalWebTestCase {
}
/**
- * Test the determination of $form_state['clicked_button'] when no button
+ * Test the determination of $form_state['triggering_element'] when no button
* information is included in the POST data, as is sometimes the case when
* the ENTER key is pressed in a textfield in Internet Explorer.
*/
function testNoButtonInfoInPost() {
$path = 'form-test/clicked-button';
$edit = array();
- $form_id = 'form-test-clicked-button';
+ $form_html_id = 'form-test-clicked-button';
// Ensure submitting a form with no buttons results in no
- // $form_state['clicked_button'] and the form submit handler not running.
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path, $edit, NULL, array(), array(), $form_id);
- $this->assertText('There is no clicked button.', t('$form_state[\'clicked_button\'] set to NULL.'));
+ // $form_state['triggering_element'] and the form submit handler not
+ // running.
+ $this->drupalPost($path, $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('There is no clicked button.', t('$form_state[\'triggering_element\'] set to NULL.'));
$this->assertNoText('Submit handler for form_test_clicked_button executed.', t('Form submit handler did not execute.'));
// Ensure submitting a form with one or more submit buttons results in
- // $form_state['clicked_button'] being set to the first one the user has
+ // $form_state['triggering_element'] being set to the first one the user has
// access to. An argument with 'r' in it indicates a restricted
// (#access=FALSE) button.
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/s', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button1.', t('$form_state[\'clicked_button\'] set to only button.'));
+ $this->drupalPost($path . '/s', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button1.', t('$form_state[\'triggering_element\'] set to only button.'));
$this->assertText('Submit handler for form_test_clicked_button executed.', t('Form submit handler executed.'));
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/s/s', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button1.', t('$form_state[\'clicked_button\'] set to first button.'));
+
+ $this->drupalPost($path . '/s/s', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button1.', t('$form_state[\'triggering_element\'] set to first button.'));
$this->assertText('Submit handler for form_test_clicked_button executed.', t('Form submit handler executed.'));
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/rs/s', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button2.', t('$form_state[\'clicked_button\'] set to first available button.'));
+
+ $this->drupalPost($path . '/rs/s', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button2.', t('$form_state[\'triggering_element\'] set to first available button.'));
$this->assertText('Submit handler for form_test_clicked_button executed.', t('Form submit handler executed.'));
// Ensure submitting a form with buttons of different types results in
- // $form_state['clicked_button'] being set to the first button, regardless
- // of type. For the FAPI 'button' type, this should result in the submit
- // handler not executing. The types are 's'(ubmit), 'b'(utton), and
+ // $form_state['triggering_element'] being set to the first button,
+ // regardless of type. For the FAPI 'button' type, this should result in the
+ // submit handler not executing. The types are 's'(ubmit), 'b'(utton), and
// 'i'(mage_button).
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/s/b/i', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button1.', t('$form_state[\'clicked_button\'] set to first button.'));
+ $this->drupalPost($path . '/s/b/i', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button1.', t('$form_state[\'triggering_element\'] set to first button.'));
$this->assertText('Submit handler for form_test_clicked_button executed.', t('Form submit handler executed.'));
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/b/s/i', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button1.', t('$form_state[\'clicked_button\'] set to first button.'));
+
+ $this->drupalPost($path . '/b/s/i', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button1.', t('$form_state[\'triggering_element\'] set to first button.'));
$this->assertNoText('Submit handler for form_test_clicked_button executed.', t('Form submit handler did not execute.'));
- drupal_static_reset('drupal_html_id');
- $this->drupalPost($path . '/i/s/b', $edit, NULL, array(), array(), $form_id);
- $this->assertText('The clicked button is button1.', t('$form_state[\'clicked_button\'] set to first button.'));
+
+ $this->drupalPost($path . '/i/s/b', $edit, NULL, array(), array(), $form_html_id);
+ $this->assertText('The clicked button is button1.', t('$form_state[\'triggering_element\'] set to first button.'));
$this->assertText('Submit handler for form_test_clicked_button executed.', t('Form submit handler executed.'));
}
-}
+ /**
+ * Test that $form_state['triggering_element'] does not get set to a button
+ * with #access=FALSE.
+ */
+ function testAttemptAccessControlBypass() {
+ $path = 'form-test/clicked-button';
+ $form_html_id = 'form-test-clicked-button';
+
+ // Retrieve a form where 'button1' has #access=FALSE and 'button2' doesn't.
+ $this->drupalGet($path . '/rs/s');
+
+ // Submit the form with 'button1=button1' in the POST data, which someone
+ // trying to get around security safeguards could easily do. We have to do
+ // a little trickery here, to work around the safeguards in drupalPost(): by
+ // renaming the text field that is in the form to 'button1', we can get the
+ // data we want into $_POST.
+ $elements = $this->xpath('//form[@id="' . $form_html_id . '"]//input[@name="text"]');
+ $elements[0]['name'] = 'button1';
+ $this->drupalPost(NULL, array('button1' => 'button1'), NULL, array(), array(), $form_html_id);
+
+ // Ensure that $form_state['triggering_element'] was not set to the
+ // restricted button. Do this with both a negative and positive assertion,
+ // because negative assertions alone can be brittle. See
+ // testNoButtonInfoInPost() for why the triggering element gets set to
+ // 'button2'.
+ $this->assertNoText('The clicked button is button1.', t('$form_state[\'triggering_element\'] not set to a restricted button.'));
+ $this->assertText('The clicked button is button2.', t('$form_state[\'triggering_element\'] not set to a restricted button.'));
+ }
+}
/**
* Tests rebuilding of arbitrary forms by altering them.
diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module
index 0b9284adb7a2dade77aebc6f2856849d017489f2..09cea654fd7196eaa5da9ad84ce375e4431ece5b 100644
--- a/modules/simpletest/tests/form_test.module
+++ b/modules/simpletest/tests/form_test.module
@@ -1114,8 +1114,8 @@ function form_test_clicked_button($form, &$form_state) {
* Form validation handler for the form_test_clicked_button() form.
*/
function form_test_clicked_button_validate($form, &$form_state) {
- if (isset($form_state['clicked_button'])) {
- drupal_set_message(t('The clicked button is %name.', array('%name' => $form_state['clicked_button']['#name'])));
+ if (isset($form_state['triggering_element'])) {
+ drupal_set_message(t('The clicked button is %name.', array('%name' => $form_state['triggering_element']['#name'])));
}
else {
drupal_set_message('There is no clicked button.');
@@ -1129,7 +1129,6 @@ function form_test_clicked_button_submit($form, &$form_state) {
drupal_set_message('Submit handler for form_test_clicked_button executed.');
}
-
/**
* Implements hook_form_FORM_ID_alter() for the registration form.
*/