Issue #2239299 by tim.plunkett: Form errors should only be set during validation.

core/lib/Drupal/Core/Form/FormBuilder.php

@@ -824,17 +824,14 @@ public function validateForm($form_id, &$form, &$form_state) {
         // Stop here and don't run any further validation handlers, because they
         // could invoke non-safe operations which opens the door for CSRF
         // vulnerabilities.
-        $this->validatedForms[$form_id] = TRUE;
+        $this->finalizeValidation($form_id, $form, $form_state);
         return;
       }
     }
 
     // Recursively validate each form element.
     $this->doValidateForm($form, $form_state, $form_id);
-    // After validation, loop through and assign each element its errors.
-    $this->setElementErrorsFromFormState($form, $form_state);
-    // Mark this form as validated.
-    $this->validatedForms[$form_id] = TRUE;
+    $this->finalizeValidation($form_id, $form, $form_state);
 
     // If validation errors are limited then remove any non validated form values,
     // so that only values that passed validation are left for submit callbacks.
@@ -877,6 +874,23 @@ public function validateForm($form_id, &$form, &$form_state) {
     }
   }
 
+  /**
+   * Finalizes validation.
+   *
+   * @param string $form_id
+   *   The unique string identifying the form.
+   * @param array $form
+   *   An associative array containing the structure of the form.
+   * @param array $form_state
+   *   An associative array containing the current state of the form.
+   */
+  protected function finalizeValidation($form_id, &$form, &$form_state) {
+    // After validation, loop through and assign each element its errors.
+    $this->setElementErrorsFromFormState($form, $form_state);
+    // Mark this form as validated.
+    $this->validatedForms[$form_id] = TRUE;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/aggregator/lib/Drupal/aggregator/Form/OpmlFeedAdd.php

@@ -114,7 +114,7 @@ public function validateForm(array &$form, array &$form_state) {
    */
   public function submitForm(array &$form, array &$form_state) {
     $validators = array('file_validate_extensions' => array('opml xml'));
-    if ($file = file_save_upload('upload', $form_state, $validators, FALSE, 0)) {
+    if ($file = file_save_upload('upload', $validators, FALSE, 0)) {
       $data = file_get_contents($file->getFileUri());
     }
     else {

core/modules/config/lib/Drupal/config/Form/ConfigImportForm.php

@@ -100,7 +100,7 @@ public function submitForm(array &$form, array &$form_state) {
         $form_state['redirect_route']['route_name'] = 'config.sync';
       }
       catch (\Exception $e) {
-        $this->setFormError('import_tarball', $form_state, $this->t('Could not extract the contents of the tar file. The error message is <em>@message</em>', array('@message' => $e->getMessage())));
+        drupal_set_message($this->t('Could not extract the contents of the tar file. The error message is <em>@message</em>', array('@message' => $e->getMessage())), 'error');
       }
       drupal_unlink($path);
     }

core/modules/file/file.module

@@ -769,7 +769,7 @@ function file_cron() {
  *   - source: Path to the file before it is moved.
  *   - destination: Path to the file after it is moved (same as 'uri').
  */
-function file_save_upload($form_field_name, array &$form_state, $validators = array(), $destination = FALSE, $delta = NULL, $replace = FILE_EXISTS_RENAME) {
+function file_save_upload($form_field_name, $validators = array(), $destination = FALSE, $delta = NULL, $replace = FILE_EXISTS_RENAME) {
   $user = \Drupal::currentUser();
   static $upload_cache;
 
@@ -927,7 +927,7 @@ function file_save_upload($form_field_name, array &$form_state, $validators = ar
       else {
         $message .= ' ' . array_pop($errors);
       }
-      form_set_error($form_field_name, $form_state, $message);
+      drupal_set_message($message, 'error');
       $files[$i] = FALSE;
       continue;
     }
@@ -937,7 +937,7 @@ function file_save_upload($form_field_name, array &$form_state, $validators = ar
     // operations.
     $file->uri = $file->destination;
     if (!drupal_move_uploaded_file($file_info->getRealPath(), $file->getFileUri())) {
-      form_set_error($form_field_name, $form_state, t('File upload error. Could not move uploaded file.'));
+      drupal_set_message(t('File upload error. Could not move uploaded file.'), 'error');
       watchdog('file', 'Upload error. Could not move uploaded file %file to destination %destination.', array('%file' => $file->filename, '%destination' => $file->uri));
       $files[$i] = FALSE;
       continue;
@@ -1496,7 +1496,7 @@ function file_managed_file_save_upload($element, array &$form_state) {
   $files_uploaded = $element['#multiple'] && count(array_filter($file_upload)) > 0;
   $files_uploaded |= !$element['#multiple'] && !empty($file_upload);
   if ($files_uploaded) {
-    if (!$files = file_save_upload($upload_name, $form_state, $element['#upload_validators'], $destination)) {
+    if (!$files = file_save_upload($upload_name, $element['#upload_validators'], $destination)) {
       watchdog('file', 'The file upload failed. %upload', array('%upload' => $upload_name));
       form_set_error($upload_name, $form_state, t('Files in the !name field were unable to be uploaded.', array('!name' => $element['#title'])));
       return array();

core/modules/file/tests/file_test/lib/Drupal/file_test/Form/FileTestForm.php

@@ -101,7 +101,7 @@ public function submitForm(array &$form, array &$form_state) {
       $validators['file_validate_extensions'] = array($form_state['values']['extensions']);
     }
 
-    $file = file_save_upload('file_test_upload', $form_state, $validators, $destination, 0, $form_state['values']['file_test_replace']);
+    $file = file_save_upload('file_test_upload', $validators, $destination, 0, $form_state['values']['file_test_replace']);
     if ($file) {
       $form_state['values']['file_test_upload'] = $file;
       drupal_set_message(t('File @filepath was uploaded.', array('@filepath' => $file->getFileUri())));

core/modules/locale/locale.bulk.inc

@@ -115,7 +115,7 @@ function locale_translate_import_form($form, &$form_state) {
  */
 function locale_translate_import_form_submit($form, &$form_state) {
   // Ensure we have the file uploaded.
-  if ($file = file_save_upload('file', $form_state, $form['file']['#upload_validators'], 'translations://', 0)) {
+  if ($file = file_save_upload('file', $form['file']['#upload_validators'], 'translations://', 0)) {
 
     // Add language, if not yet supported.
     $language = \Drupal::languageManager()->getLanguage($form_state['values']['langcode']);
@@ -136,7 +136,7 @@ function locale_translate_import_form_submit($form, &$form_state) {
     batch_set($batch);
   }
   else {
-    form_set_error('file', $form_state, t('File to import not found.'));
+    drupal_set_message(t('File to import not found.'), 'error');
     $form_state['rebuild'] = TRUE;
     return;
   }

core/modules/search/lib/Drupal/search/Form/SearchBlockForm.php

@@ -86,7 +86,7 @@ public function submitForm(array &$form, array &$form_state) {
     // "field is required" because the search keywords field has no title.
     // The error message would also complain about a missing #title field.)
     if ($form_state['values']['search_block_form'] == '') {
-      $this->setFormError('keys', $form_state, $this->t('Please enter some keywords.'));
+      drupal_set_message($this->t('Please enter some keywords.'), 'error');
     }
 
     $form_id = $form['form_id']['#value'];
@@ -99,7 +99,7 @@ public function submitForm(array &$form, array &$form_state) {
       );
     }
     else {
-      $this->setFormError('', $form_state, $this->t('Search is currently disabled.'));
+      drupal_set_message($this->t('Search is currently disabled.'), 'error');
     }
   }
 

core/modules/shortcut/shortcut.admin.inc

@@ -118,7 +118,7 @@ function shortcut_set_switch_validate($form, &$form_state) {
   if ($form_state['values']['set'] == 'new') {
     // Check to prevent creating a shortcut set with an empty title.
     if (trim($form_state['values']['label']) == '') {
-      form_set_error('new', $form_state, t('The new set label is required.'));
+      form_set_error('label', $form_state, t('The new set label is required.'));
     }
     // Check to prevent a duplicate title.
     if (shortcut_set_title_exists($form_state['values']['label'])) {

core/modules/system/lib/Drupal/system/Form/ThemeSettingsForm.php

@@ -324,7 +324,7 @@ public function validateForm(array &$form, array &$form_state) {
       $validators = array('file_validate_is_image' => array());
 
       // Check for a new uploaded logo.
-      $file = file_save_upload('logo_upload', $form_state, $validators, FALSE, 0);
+      $file = file_save_upload('logo_upload', $validators, FALSE, 0);
       if (isset($file)) {
         // File upload was attempted.
         if ($file) {
@@ -340,7 +340,7 @@ public function validateForm(array &$form, array &$form_state) {
       $validators = array('file_validate_extensions' => array('ico png gif jpg jpeg apng svg'));
 
       // Check for a new uploaded favicon.
-      $file = file_save_upload('favicon_upload', $form_state, $validators, FALSE, 0);
+      $file = file_save_upload('favicon_upload', $validators, FALSE, 0);
       if (isset($file)) {
         // File upload was attempted.
         if ($file) {

core/modules/update/update.manager.inc

@@ -659,16 +659,16 @@ function update_manager_install_form_submit($form, &$form_state) {
     $field = 'project_url';
     $local_cache = update_manager_file_get($form_state['values']['project_url']);
     if (!$local_cache) {
-      form_set_error($field, $form_state, t('Unable to retrieve Drupal project from %url.', array('%url' => $form_state['values']['project_url'])));
+      drupal_set_message(t('Unable to retrieve Drupal project from %url.', array('%url' => $form_state['values']['project_url'])), 'error');
       return;
     }
   }
   elseif ($_FILES['files']['name']['project_upload']) {
     $validators = array('file_validate_extensions' => array(archiver_get_extensions()));
     $field = 'project_upload';
-    if (!($finfo = file_save_upload($field, $form_state, $validators, NULL, 0, FILE_EXISTS_REPLACE))) {
-      // Failed to upload the file. file_save_upload() calls form_set_error() on
-      // failure.
+    if (!($finfo = file_save_upload($field, $validators, NULL, 0, FILE_EXISTS_REPLACE))) {
+      // Failed to upload the file. file_save_upload() calls
+      // drupal_set_message() on failure.
       return;
     }
     $local_cache = $finfo->getFileUri();
@@ -679,13 +679,13 @@ function update_manager_install_form_submit($form, &$form_state) {
     $archive = update_manager_archive_extract($local_cache, $directory);
   }
   catch (Exception $e) {
-    form_set_error($field, $form_state, $e->getMessage());
+    drupal_set_message($e->getMessage(), 'error');
     return;
   }
 
   $files = $archive->listContents();
   if (!$files) {
-    form_set_error($field, $form_state, t('Provided archive contains no files.'));
+    drupal_set_message(t('Provided archive contains no files.'), 'error');
     return;
   }
 
@@ -696,7 +696,7 @@ function update_manager_install_form_submit($form, &$form_state) {
 
   $archive_errors = update_manager_archive_verify($project, $local_cache, $directory);
   if (!empty($archive_errors)) {
-    form_set_error($field, $form_state, array_shift($archive_errors));
+    drupal_set_message(array_shift($archive_errors), 'error');
     // @todo: Fix me in D8: We need a way to set multiple errors on the same
     // form element and have all of them appear!
     if (!empty($archive_errors)) {
@@ -715,7 +715,7 @@ function update_manager_install_form_submit($form, &$form_state) {
     $updater = Updater::factory($project_location);
   }
   catch (Exception $e) {
-    form_set_error($field, $form_state, $e->getMessage());
+    drupal_set_message($e->getMessage(), 'error');
     return;
   }
 
@@ -723,16 +723,16 @@ function update_manager_install_form_submit($form, &$form_state) {
     $project_title = Updater::getProjectTitle($project_location);
   }
   catch (Exception $e) {
-    form_set_error($field, $form_state, $e->getMessage());
+    drupal_set_message($e->getMessage(), 'error');
     return;
   }
 
   if (!$project_title) {
-    form_set_error($field, $form_state, t('Unable to determine %project name.', array('%project' => $project)));
+    drupal_set_message(t('Unable to determine %project name.', array('%project' => $project)), 'error');
   }
 
   if ($updater->isInstalled()) {
-    form_set_error($field, $form_state, t('%project is already installed.', array('%project' => $project_title)));
+    drupal_set_message(t('%project is already installed.', array('%project' => $project_title)), 'error');
     return;
   }