- Patch #11505 by Steven: 'my account' information is not saved.

  + Drupal 4.4 stored profile data in the serialized user->data column. Drupal 4.5 stores profile data in tables (but user->data is still available and used for other stuff, like locale or themes).  The update from 4.4 to 4.5 didn't remove the old data from the user->data column properly, because there is no mechanism in user_save to do so (it did try to unset the fields, but this has no effect).

  + On registration, hook_user('insert') is invoked after saving the data column. This means that any module-specific data is put into the data field. We cannot move hook_user('insert') higher up, because before that point, we do not have a complete $user object yet.

7 files changed, 129 insertions, 45 deletions

diff --git a/database/database.mysql b/database/database.mysql
index 65641a4ec..528ccbaea 100644
--- a/database/database.mysql
+++ b/database/database.mysql
@@ -759,7 +759,7 @@ INSERT INTO permission VALUES (1,'access content',0);
 INSERT INTO role (rid, name) VALUES (2, 'authenticated user');
 INSERT INTO permission VALUES (2,'access comments, access content, post comments, post comments without approval',0);
 
-REPLACE variable SET name='update_start', value='s:10:"2004-09-17;"';
+REPLACE variable SET name='update_start', value='s:10:"2004-10-16;"';
 REPLACE variable SET name='theme_default', value='s:10:"bluemarine";';
 
 REPLACE blocks SET module = 'user', delta = '0', status = '1';
diff --git a/database/database.pgsql b/database/database.pgsql
index 6e625f74f..ac9e31b99 100644
--- a/database/database.pgsql
+++ b/database/database.pgsql
@@ -752,7 +752,7 @@ INSERT INTO system VALUES ('modules/taxonomy.module','taxonomy','module','',1,0,
 INSERT INTO system VALUES ('themes/bluemarine/xtemplate.xtmpl','bluemarine','theme','themes/engines/xtemplate/xtemplate.engine',1,0,0);
 INSERT INTO system VALUES ('themes/engines/xtemplate/xtemplate.engine','xtemplate','theme_engine','',1,0,0);
 
-INSERT INTO variable(name,value) VALUES('update_start', 's:10:"2004-09-17";');
+INSERT INTO variable(name,value) VALUES('update_start', 's:10:"2004-10-16";');
 INSERT INTO variable(name,value) VALUES('theme_default','s:10:"bluemarine";');
 INSERT INTO users(uid,name,mail) VALUES(0,'','');
 INSERT INTO users_roles(uid,rid) VALUES(0, 1);
diff --git a/database/updates.inc b/database/updates.inc
index 82940d9ed..396000c80 100644
--- a/database/updates.inc
+++ b/database/updates.inc
@@ -83,7 +83,8 @@ $sql_updates = array(
   "2004-08-19" => "update_104",
   "2004-09-14" => "update_105",
   "2004-09-15" => "update_106",
-  "2004-09-17" => "update_107"
+  "2004-09-17" => "update_107",
+  "2004-10-16" => "update_108"
 );
 
 function update_32() {
@@ -946,14 +947,16 @@ function update_80() {
       if ($account->$old) {
         $edit[$new] = $account->$old;
       }
-      unset($account->$old);
+      // Force deletion of old field
+      $edit[$old] = NULL;
     }
 
     // Birthday format change:
     if ($edit['birthday']) {
       $edit['birthday'] = array('day' => $edit['birthday'], 'month' => $account->profile_birthmonth, 'year' => $account->profile_birthyear);
-      unset($account->profile_birthmonth);
-      unset($account->profile_birthyear);
+      // Force deletion of old field
+      $edit['profile_birthmonth'] = NULL;
+      $edit['profile_birthyear'] = NULL;
     }
 
     // Gender specific changes:
@@ -963,13 +966,18 @@ function update_80() {
     // Avatar specific changes:
     if ($account->profile_avatar) {
       $edit['picture'] = $account->profile_avatar;
+      // Force deletion of old field
+      $edit['profile_avatar'] = NULL;
     }
-    unset($account->profile_avatar);
 
     // Save the update record:
     user_save($account, $edit, 'Personal information');
   }
 
+  // This variable is needed to distinguish betweene 4.5-RC sites which ran a faulty
+  // update_80() and 4.5-final sites. See update_108.
+  variable_set('update_80_fix', true);
+
   return $ret;
 }
 
@@ -1873,6 +1881,26 @@ function update_107() {
   return $ret;
 }
 
+function update_108() {
+  // This update is needed for 4.5-RC sites, where profile data was not being
+  // wiped from the user->data column correctly because update_80() was faulty.
+  if (!variable_get('update_80_fix', false)) {
+    // The data field needs to be cleared of profile fields.
+    $result = db_query("SELECT uid FROM {users} WHERE data LIKE '%profile%'");
+    while ($uid = db_fetch_object($result)) {
+      $user = user_load(array('uid' => $uid->uid));
+      $unset = array();
+      foreach ($user as $key => $value) {
+        if (substr($key, 0, 8) == 'profile_') {
+          // Fields with a NULL value are wiped from the data column.
+          $unset[$key] = NULL;
+        }
+      }
+      user_save($user, $unset);
+    }
+  }
+}
+
 function update_sql($sql) {
   $edit = $_POST["edit"];
   $result = db_query($sql);
diff --git a/modules/profile.module b/modules/profile.module
index 276cefa64..3bf611a03 100644
--- a/modules/profile.module
+++ b/modules/profile.module
@@ -162,7 +162,8 @@ function profile_save_profile(&$edit, &$user, $category) {
     }
     db_query("DELETE FROM {profile_values} WHERE fid = %d AND uid = %d", $field->fid, $user->uid);
     db_query("INSERT INTO {profile_values} (fid, uid, value) VALUES (%d, %d, '%s')", $field->fid, $user->uid, $edit[$field->name]);
-    unset($edit[$field->name], $user->{$field->name});
+    // Mark field as handled (prevents saving to user->data).
+    $edit[$field->name] = null;
   }
 }
 
diff --git a/modules/profile/profile.module b/modules/profile/profile.module
index 276cefa64..3bf611a03 100644
--- a/modules/profile/profile.module
+++ b/modules/profile/profile.module
@@ -162,7 +162,8 @@ function profile_save_profile(&$edit, &$user, $category) {
     }
     db_query("DELETE FROM {profile_values} WHERE fid = %d AND uid = %d", $field->fid, $user->uid);
     db_query("INSERT INTO {profile_values} (fid, uid, value) VALUES (%d, %d, '%s')", $field->fid, $user->uid, $edit[$field->name]);
-    unset($edit[$field->name], $user->{$field->name});
+    // Mark field as handled (prevents saving to user->data).
+    $edit[$field->name] = null;
   }
 }
 
diff --git a/modules/user.module b/modules/user.module
index 586344719..adfcc8425 100644
--- a/modules/user.module
+++ b/modules/user.module
@@ -9,13 +9,15 @@
 /**
  * Invokes hook_user() in every module.
  *
- * We cannot use module_invoke() for this, becuse the arguments need to
+ * We cannot use module_invoke() for this, because the arguments need to
  * be passed by reference.
  */
 function user_module_invoke($type, &$array, &$user, $category = NULL) {
   foreach (module_list() as $module) {
     $function = $module .'_user';
-    if (function_exists($function)) $function($type, $array, $user, $category);
+    if (function_exists($function)) {
+      $function($type, $array, $user, $category);
+    }
   }
 }
 
@@ -82,6 +84,18 @@ function user_load($array = array()) {
   return $user;
 }
 
+/**
+ * Save changes to a user account.
+ *
+ * @param $account
+ *   The $user object for the user to modify.
+ *
+ * @param $array
+ *   An array of fields and values to save. For example array('name' => 'My name');
+ *
+ * @param $category
+ *   (optional) The category for storing profile information in.
+ */
 function user_save($account, $array = array(), $category = 'account') {
   // Dynamically compose a SQL query:
   $user_fields = user_fields();
@@ -96,13 +110,18 @@ function user_save($account, $array = array(), $category = 'account') {
       }
       else if (substr($key, 0, 4) !== 'auth') {
         if (in_array($key, $user_fields)) {
-          // escape '%'s:
-          $value = str_replace('%', '%%', $value);
+          // Save standard fields
           $query .= "$key = '%s', ";
           $v[] = $value;
         }
         else {
-          $data[$key] = $value;
+          if ($value === null) {
+            // Setting a field to null deletes it from the data column.
+            unset($data[$key]);
+          }
+          else {
+            $data[$key] = $value;
+          }
         }
       }
     }
@@ -111,7 +130,7 @@ function user_save($account, $array = array(), $category = 'account') {
 
     db_query("UPDATE {users} SET $query changed = %d WHERE uid = %d", array_merge($v, array(time(), $account->uid)));
 
-    // reload user roles if provided
+    // Reload user roles if provided
     if (is_array($array['roles'])) {
       db_query('DELETE FROM {users_roles} WHERE uid = %d', $account->uid);
 
@@ -120,6 +139,7 @@ function user_save($account, $array = array(), $category = 'account') {
       }
     }
 
+    // Refresh user object
     $user = user_load(array('uid' => $account->uid));
   }
   else {
@@ -127,6 +147,9 @@ function user_save($account, $array = array(), $category = 'account') {
     $array['changed'] = time();
     $array['uid'] = db_next_id('{users}_uid');
 
+    // Note, we wait with saving the data column to prevent module-handled
+    // fields from being saved there. We cannot invoke hook_user('insert') here
+    // because we don't have a fully initialized user object yet.
     foreach ($array as $key => $value) {
       if ($key == 'pass') {
         $fields[] = check_query($key);
@@ -139,36 +162,40 @@ function user_save($account, $array = array(), $category = 'account') {
           $values[] = $value;
           $s[] = "'%s'";
         }
-        else {
-          $data[$key] = $value;
-        }
       }
     }
-
-    $fields[] = 'data';
-    $values[] = serialize($data);
-    $s[] = "'%s'";
-
     db_query('INSERT INTO {users} ('. implode(', ', $fields) .') VALUES ('. implode(', ', $s) .')', $values);
 
     // Reload user roles (delete just to be safe).
     db_query('DELETE FROM {users_roles} WHERE uid = %d', $array['uid']);
-
     foreach ($array['roles'] as $rid) {
       db_query('INSERT INTO {users_roles} (uid, rid) VALUES (%d, %d)', $array['uid'], $rid);
     }
 
-    $user = user_load(array('name' => $array['name']));
+    // Build the initial user object.
+    $user = user_load(array('uid' => $array['uid']));
+
+    user_module_invoke('insert', $array, $user, $category);
 
-    module_invoke_all('user', 'insert', $array, $user, $category);
+    // Build and save the serialized data field now
+    $data = array();
+    foreach ($array as $key => $value) {
+      if ((substr($key, 0, 4) !== 'auth') && (!in_array($key, $user_fields)) && ($value !== null)) {
+        $data[$key] = $value;
+      }
+    }
+    db_query("UPDATE {users} SET data = '%s' WHERE uid = %d", serialize($data), $user->uid);
+
+    // Build the finished user object.
+    $user = user_load(array('uid' => $array['uid']));
   }
 
+  // Save distributed authentication mappings
   foreach ($array as $key => $value) {
     if (substr($key, 0, 4) == 'auth') {
       $authmaps[$key] = $value;
     }
   }
-
   if ($authmaps) {
     user_set_authmaps($user, $authmaps);
   }
diff --git a/modules/user/user.module b/modules/user/user.module
index 586344719..adfcc8425 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -9,13 +9,15 @@
 /**
  * Invokes hook_user() in every module.
  *
- * We cannot use module_invoke() for this, becuse the arguments need to
+ * We cannot use module_invoke() for this, because the arguments need to
  * be passed by reference.
  */
 function user_module_invoke($type, &$array, &$user, $category = NULL) {
   foreach (module_list() as $module) {
     $function = $module .'_user';
-    if (function_exists($function)) $function($type, $array, $user, $category);
+    if (function_exists($function)) {
+      $function($type, $array, $user, $category);
+    }
   }
 }
 
@@ -82,6 +84,18 @@ function user_load($array = array()) {
   return $user;
 }
 
+/**
+ * Save changes to a user account.
+ *
+ * @param $account
+ *   The $user object for the user to modify.
+ *
+ * @param $array
+ *   An array of fields and values to save. For example array('name' => 'My name');
+ *
+ * @param $category
+ *   (optional) The category for storing profile information in.
+ */
 function user_save($account, $array = array(), $category = 'account') {
   // Dynamically compose a SQL query:
   $user_fields = user_fields();
@@ -96,13 +110,18 @@ function user_save($account, $array = array(), $category = 'account') {
       }
       else if (substr($key, 0, 4) !== 'auth') {
         if (in_array($key, $user_fields)) {
-          // escape '%'s:
-          $value = str_replace('%', '%%', $value);
+          // Save standard fields
           $query .= "$key = '%s', ";
           $v[] = $value;
         }
         else {
-          $data[$key] = $value;
+          if ($value === null) {
+            // Setting a field to null deletes it from the data column.
+            unset($data[$key]);
+          }
+          else {
+            $data[$key] = $value;
+          }
         }
       }
     }
@@ -111,7 +130,7 @@ function user_save($account, $array = array(), $category = 'account') {
 
     db_query("UPDATE {users} SET $query changed = %d WHERE uid = %d", array_merge($v, array(time(), $account->uid)));
 
-    // reload user roles if provided
+    // Reload user roles if provided
     if (is_array($array['roles'])) {
       db_query('DELETE FROM {users_roles} WHERE uid = %d', $account->uid);
 
@@ -120,6 +139,7 @@ function user_save($account, $array = array(), $category = 'account') {
       }
     }
 
+    // Refresh user object
     $user = user_load(array('uid' => $account->uid));
   }
   else {
@@ -127,6 +147,9 @@ function user_save($account, $array = array(), $category = 'account') {
     $array['changed'] = time();
     $array['uid'] = db_next_id('{users}_uid');
 
+    // Note, we wait with saving the data column to prevent module-handled
+    // fields from being saved there. We cannot invoke hook_user('insert') here
+    // because we don't have a fully initialized user object yet.
     foreach ($array as $key => $value) {
       if ($key == 'pass') {
         $fields[] = check_query($key);
@@ -139,36 +162,40 @@ function user_save($account, $array = array(), $category = 'account') {
           $values[] = $value;
           $s[] = "'%s'";
         }
-        else {
-          $data[$key] = $value;
-        }
       }
     }
-
-    $fields[] = 'data';
-    $values[] = serialize($data);
-    $s[] = "'%s'";
-
     db_query('INSERT INTO {users} ('. implode(', ', $fields) .') VALUES ('. implode(', ', $s) .')', $values);
 
     // Reload user roles (delete just to be safe).
     db_query('DELETE FROM {users_roles} WHERE uid = %d', $array['uid']);
-
     foreach ($array['roles'] as $rid) {
       db_query('INSERT INTO {users_roles} (uid, rid) VALUES (%d, %d)', $array['uid'], $rid);
     }
 
-    $user = user_load(array('name' => $array['name']));
+    // Build the initial user object.
+    $user = user_load(array('uid' => $array['uid']));
+
+    user_module_invoke('insert', $array, $user, $category);
 
-    module_invoke_all('user', 'insert', $array, $user, $category);
+    // Build and save the serialized data field now
+    $data = array();
+    foreach ($array as $key => $value) {
+      if ((substr($key, 0, 4) !== 'auth') && (!in_array($key, $user_fields)) && ($value !== null)) {
+        $data[$key] = $value;
+      }
+    }
+    db_query("UPDATE {users} SET data = '%s' WHERE uid = %d", serialize($data), $user->uid);
+
+    // Build the finished user object.
+    $user = user_load(array('uid' => $array['uid']));
   }
 
+  // Save distributed authentication mappings
   foreach ($array as $key => $value) {
     if (substr($key, 0, 4) == 'auth') {
       $authmaps[$key] = $value;
     }
   }
-
   if ($authmaps) {
     user_set_authmaps($user, $authmaps);
   }