the hook user expects a user object in its third parameter, $account.
however the user_register_validate function invokes the user hook sending an array instead...
user_module_invoke('validate', $form_state['values'], $form_state['values'], 'account');

This means that I can't user_access($account) in hook_user if $op == 'validate' because user_access expects an object, not an array.
The workaround is to user_access((object)$account);

I've a feeling I'm missing something here, but on the other hand, it's perfectly straight forward.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pillarsdotnet’s picture

Version: 6.15 » 6.x-dev
FileSize
491 bytes

Same problem here.

Patch solves the problem for me.

pillarsdotnet’s picture

Status: Needs review » Active
Issue tags: -Quick fix, -backport, -quickfix

The bug has apparently been around as long as function itself: Mon, 21 Nov 2005.

pillarsdotnet’s picture

Priority: Critical » Major
Status: Active » Needs review
FileSize
459 bytes

Slighly smaller/simpler patch; needs testing.

pillarsdotnet’s picture

Issue tags: +Quick fix, +backport, +quickfix

Tagging. Note that D7 likewise casts $form_state['values'] to an object, in the entity_form_field_validate function.

The user_register_validate function was created Mon, 21 Nov 2005, removed Sat, 10 Oct 2009, and recreated Tue, 30 Nov 2010.

So this patch is technically a backport of the fix incidental to:

pillarsdotnet’s picture

Status: Active » Needs review
Issue tags: +Quick fix, +backport, +CiviCRM, +quickfix

Affects CiviCRM -- that's how I found/fixed it in the first place.

pillarsdotnet’s picture

Title: user_register_validate sends array into hook_user object » user_register_validate() should cast array to object in third parameter of user_module_invoke()

Better title?

pillarsdotnet’s picture

Title: user_register_validate() should cast array to object in third parameter of user_module_invoke() » Third parameter of user_module_invoke() should be object but user_register_validate() calls it with an array instead.

I can't believe I'm bikeshedding this...

pillarsdotnet’s picture

FileSize
491 bytes

Patch in #1 was the correct approach. Re-uploading for the sake of clarity.

Gábor Hojtsy’s picture

Looks good. Who else tested this?

pillarsdotnet’s picture

Dunno, but at least one other person (the original reporter) was affected by the problem, and the suggested fix seems self-evident to me. The only other fix I could imagine is to move the cast to user_module_invoke():

D7 patch:
diff --git modules/user/user.module modules/user/user.module
index 7f22f7c..43f96e6 100644
--- modules/user/user.module
+++ modules/user/user.module
@@ -80,6 +80,7 @@ function user_help($path, $arg) {
  * be passed by reference.
  */
 function user_module_invoke($type, &$edit, $account, $category = NULL) {
+  $account = (object) $account;
   foreach (module_implements('user_' . $type) as $module) {
     $function = $module . '_user_' . $type;
     $function($edit, $account, $category);
D6 backport:
diff --git modules/user/user.module modules/user/user.module
index 2e12c17..39624e9 100644
--- modules/user/user.module
+++ modules/user/user.module
@@ -16,6 +16,7 @@ define('EMAIL_MAX_LENGTH', 64);
  * be passed by reference.
  */
 function user_module_invoke($type, &$array, &$user, $category = NULL) {
+  $user = (object) $user;
   foreach (module_list() as $module) {
     $function = $module .'_user';
     if (function_exists($function)) {

Again, it seems self-evident that the patch in #1/#8 is preferable, especially since the D7 patch above is unnecessary and arguably wrong.

Status: Needs review » Needs work

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

pillarsdotnet’s picture

Status: Needs work » Needs review
FileSize
880 bytes

Trying again...

Status: Needs review » Needs work

The last submitted patch, user_register_validate-720876-12.patch, failed testing.

pillarsdotnet’s picture

Issue tags: -CiviCRM
FileSize
483 bytes

Okay, so the testbot doesn't like it when I follow instructions on how to submit patches.

Trying again with "git diff -no-prefix"

pillarsdotnet’s picture

Status: Needs work » Needs review

Go, testbot!

Status: Needs review » Needs work

The last submitted patch, user_register_validate-720876-14.patch, failed testing.

pillarsdotnet’s picture

Status: Needs work » Needs review
FileSize
880 bytes

oh, tay.
so maybe testbot is trying (once again) to apply D6 patches to D8 core?

pillarsdotnet’s picture

Status: Needs review » Reviewed & tested by the community
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review

Does not seem like anybody tested this in the community beyond the patch creator?!

pillarsdotnet’s picture

I guess nobody cares about fixing long-standing bugs in the d6 api. I get quicker results with d8 bugs, in fact.

Added link on user_register_validate() api page.

pillarsdotnet’s picture

Re-uploading; I think the testbots are fixed...

pillarsdotnet’s picture

Issue tags: -Quick fix, -backport, -quickfix

Status: Needs review » Needs work
Issue tags: +Quick fix, +backport, +quickfix

The last submitted patch, user_register_validate-720876-21.patch, failed testing.

pillarsdotnet’s picture

pillarsdotnet’s picture

pillarsdotnet’s picture

Renaming patch to avoid useless core testing as suggested by ksenzee in IRC.

pillarsdotnet’s picture

pwolanin’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +PHP 5.3 compatibility

So, I've been testing this patch and related code. I think Drupal core masks the error due to the isset() check in function user_user(), so the rest of the code is never called when an anonymous user registers. However, any contrib module implementing hook_user('validate') will see a notice or worse from this bug, and according to the OP PHP 5.3 is less graceful in handling the case of an array accessed as an object.

This test code on PHP 5.2.6 gives me:
FALSE
TRUE

<?php
$x = array();
var_dump(isset($x->uid));
echo "\n";
$x['uid'] = 1;
var_dump(isset($x->uid));
echo "\n";

According to pillarsdotnet PHP 5.3 gives FALSE FALSE which reinforces the fact that 5.2 and 5.3 are different about this.

In any case - the patch is trivial and won't cause any harm and makes core respect its own API.

pillarsdotnet’s picture

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed, pushed, thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix, -backport, -quickfix, -PHP 5.3 compatibility

Automatically closed -- issue fixed for 2 weeks with no activity.