There is a problem with storing serialize data in the $user->data field. To recreate
* Do a minimal install
* Enable the contact module
* Go to user/1/edit
* Click save - the first time the data is save correctly in the user->data field
* Then when the page is reloaded user->data is returned to during user load in an serialised format
* Click save again the data in user->data is reserialized and saved and looks like this s:24:"0:1:{s:7:"contact";i:0;}";
* When user edit reloads an error is generated in drupal_unpack

Warning: Invalid argument supplied for foreach() in drupal_unpack() (line 1179 of C:\code\drupal\includes\bootstrap.inc).

Comments

marcingy’s picture

StatusFileSize
new415 bytes

Patch to amend unpack so as the data that was unorginally serialized is unserialized and can be subsquentally saved without generating unpack errors when it is reserialised.

marcingy’s picture

Status: Active » Needs review
marcingy’s picture

Status: Needs review » Needs work

After speaking to chx this approach is totally wrong but as it stands enabling the contact module is going to produce drupal_unpack errors after the first save.

chx’s picture

Status: Needs work » Needs review
StatusFileSize
new720 bytes

Changing a function like that this late? That's a recipe for catastrophe. What about this, however?

chx’s picture

marcingy’s picture

Much nicer than my solution. Tested locally if the bot goes green this can be set to RBTC.

marcingy’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Looks good if the testbot comes back green, I got fails before doing it this way 'round, but some other fixes have gone in since then so we might be alright by now.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, be_gentle_to_this_old_old_cycle.patch, failed testing.

catch’s picture

Status: Needs work » Closed (duplicate)

The issue is at the moment $account->data can sometimes be an array, sometimes unserialized.

Either we add a tonne of cruft in user_save() to check for all the possibilities, or we could see how marcingy's approach works. Let's at least see what happens with the test bot on that patch, since to me having that data as an actual array seems like the right fix here and no explanation has been given why it's such a bad idea.

Additionally this is a duplicate of #721436: Remove magical fairy saving of cruft from user_save() which is still open for cleanups.