#204705 by pwolanin: abort user_save on SQL errors, to avoid data corruption

1 files changed, 29 insertions, 5 deletions

diff --git a/modules/user/user.module b/modules/user/user.module
index aca41f838..b31dce5cd 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -200,6 +200,9 @@ function user_load($array = array()) {
  *
  * @param $category
  *   (optional) The category for storing profile information in.
+ *
+ * @return
+ *   A fully-loaded $user object upon successful save or FALSE if the save failed.
  */
 function user_save($account, $array = array(), $category = 'account') {
   // Dynamically compose a SQL query:
@@ -238,7 +241,11 @@ function user_save($account, $array = array(), $category = 'account') {
     $query .= "data = '%s' ";
     $v[] = serialize($data);
 
-    db_query("UPDATE {users} SET $query WHERE uid = %d", array_merge($v, array($account->uid)));
+    $success = db_query("UPDATE {users} SET $query WHERE uid = %d", array_merge($v, array($account->uid)));
+    if (!$success) {
+      // The query failed - better to abort the save than risk further data loss.
+      return FALSE;
+    }
 
     // Reload user roles if provided
     if (isset($array['roles']) && is_array($array['roles'])) {
@@ -311,10 +318,15 @@ function user_save($account, $array = array(), $category = 'account') {
           break;
       }
     }
-    db_query('INSERT INTO {users} ('. implode(', ', $fields) .') VALUES ('. implode(', ', $s) .')', $values);
-    $array['uid'] = db_last_insert_id('users', 'uid');
-
+    $success = db_query('INSERT INTO {users} ('. implode(', ', $fields) .') VALUES ('. implode(', ', $s) .')', $values);
+    if (!$success) {
+      // On a failed INSERT some other existing user's uid may be returned.  We
+      // must abort to avoid overwirting their account.
+      return FALSE;
+    }
+    
     // Build the initial user object.
+    $array['uid'] = db_last_insert_id('users', 'uid');
     $user = user_load(array('uid' => $array['uid']));
 
     user_module_invoke('insert', $array, $user, $category);
@@ -1361,7 +1373,13 @@ function user_external_login_register($name, $module) {
   if (!isset($user->uid)) {
     // Register this new user.
     $userinfo = array('name' => $name, 'pass' => user_password(), 'init' => $name, 'status' => 1, "authname_$module" => $name);
-    $user = user_save('', $userinfo);
+    $account = user_save('', $userinfo);
+    // Terminate if an error occured during user_save().
+    if (!$account) {
+      drupal_set_message(t("Error saving user account."), 'error');
+      return;
+    }
+    $user = $account;
     watchdog('user', 'New external user: %name using module %module.', array('%name' => $name, '%module' => $module), WATCHDOG_NOTICE, l(t('edit'), 'user/'. $user->uid .'/edit'));
   }
 }
@@ -2207,6 +2225,12 @@ function user_register_submit($form, &$form_state) {
     $merge_data['status'] = variable_get('user_register', 1) == 1;
   }
   $account = user_save('', array_merge($form_state['values'], $merge_data));
+  // Terminate if an error occured during user_save().
+  if (!$account) {
+    drupal_set_message(t("Error saving user account."), 'error');
+    $form_state['redirect'] = '';
+    return;
+  }
   $form_state['user'] = $account;
 
   watchdog('user', 'New user: %name (%email).', array('%name' => $name, '%email' => $mail), WATCHDOG_NOTICE, l(t('edit'), 'user/'. $account->uid .'/edit'));