ALL form values in user data
pwolanin - August 21, 2006 - 13:57
| Project: | Drupal |
| Version: | 4.7.x-dev |
| Component: | user.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
There is a bug in 4.7 and HEAD since ALL form values (e.g. "submit") get saved in the data field of the user table. For example:
a:4:{s:11:"destination";s:22:"admin/user/user/create";s:6:"notify";i:0;s:6:"submit";s:18:"Create new account";s:7:"form_id";s:13:"user_register";}
I think the problem occurs in function user_register_submit on this line:
$account = user_save('', array_merge($form_values, array('pass' => $pass, 'init' => $mail, 'roles' => $roles, 'status' => ($admin || variable_get('user_register', 1) == 1))));
#1
can confirm this, the analysis looks ok too.
#2
Patch attch which tries to unset all those vales as per function user_edit_submit.
also, leaving out 'destination' on this assumption that this patch: http://drupal.org/node/79809
will be applied.
I think patch should apply to 4.7 also (after changing path to module)
#3
setting status- patch works for me.
#4
I managed to get a 'destination' in my user data somehow while creating an account from the admin page.
#5
The patch here: http://drupal.org/node/79809
removes the (usused) 'destination' field. However, I'd be happy to add it to this patch instead
#6
This patch also cuts out the 'destination' field from the form definition. As far as I can tell, this isn't actually used- the array key would need to be '#redirect' to have an effect, right?
#7
Don't try sneaking other behaviors into this patch.
#8
Well, I didn't think I was sneaking!
Does that 'description' code have any actual effect? I can see any from following the code or testing. If not, then I think removing it is just as valid a solution as unsetting the array element.
#9
@drumm - again, I don't think that code has has any effect, so why not just remove it?
#10
in regards in the 'destination' form value, after playing more, I see that it does have an effect. So, the attached patch adds that and the addition form values 'op' to the list of things to unset.
Below the effect of this is demonstrated. Users 1 & 2 were created before the patch. User 4 was created by the admin after the patch, user 5 was self-registered after the patch.
mysql> select uid, data from users where uid > 0;+-----+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| uid | data |
+-----+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| 1 | a:3:{s:2:"op";s:6:"Submit";s:6:"submit";s:18:"Create new account";s:7:"form_id";s:13:"user_register";} |
| 2 | a:5:{s:11:"destination";s:5:"admin";s:6:"notify";i:0;s:2:"op";s:18:"Create new account";s:6:"submit";s:18:"Create new account";s:7:"form_id";s:13:"user_register";} |
| 4 | a:0:{} |
| 5 | a:0:{} |
+-----+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
#11
same code, just moved it down closer to the user_save call.
#12
a very similar patch for the 4.7 branch
#13
attached patch is updated for current HEAD, and now also takes care of the new 'form_token' in the form. This is really a trivial patch and the basic problem/solution confirmed by killes in #1, so setting RTBC.
#14
The existing place this is done is in the user_edit_submit() function. These two places need to have similar lists (such as add form_token to user_edit_submit()).
#15
Can I propose a more robust solution- what not make a $form['data']['#tree'] = TRUE; element, and only save into the data field those form elements specifically entered under there?
Are there even any modules that use this feature of saving values to the data field?
#16
new patch attached that addresses both user_register and user_edit for 5.x via the simple unset(). However, this is still not robust, since modules like Organic groups that add items to the registration forms still can easily cause this to happen.
#17
Of course its not robust; the whole $user->data serialized soup is a bad and unnormalized concept.
Committed to HEAD.
#18
setting to 4.7 for backporting
#19
#20
applied
#21
@killes - I hope this means you backported the 5.x patch, since I'm not sure the 4.7.x one from #12 included everthing (especially 'form_token').
#22