Problem/Motivation

Found frequent occurrences of the following error on a live site using the countryban module:

Parameter 2 to countryban_user() expected to be a reference, value given in .../includes/module.inc on line 497.

This is caused by the following code in the countryban_user() function of countryban/countryban.module:

module_invoke_all('user', 'logout', NULL, $user);

The module_invoke_all() function should never be used to invoke hook_user(). The user_module_invoke() function should be used instead. Additionally, the $edit (second) parameter to hook_user() is required to be an array reference. Passing NULL would cause a PHP warning even if user_module_invoke() were used.

Proposed resolution

The call to module_invoke_all() should be replaced by a call to user_module_invoke() and a temporary $edit variable passed as the second parameter.

Remaining tasks

A patch needs to be written, reviewed, and applied.

User interface changes

None.

API changes

The Drupal API would be used correctly.

CommentFileSizeAuthor
#1 countryban-user_module_invoke-1533026-1.patch1.72 KBpillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Needs review
StatusFileSize
new countryban-user_module_invoke-1533026-1.patch1.72 KB

Trivial patch attached:

--- a/countryban.module
+++ b/countryban.module
@@ -344,7 +344,9 @@ function countryban_user($op, &$edit, &$account, $category = NUL
       case 'login':
         if ( ($banvalue == 1) || ($banvalue == 2) ) {
           // Destroy the current session:
-          module_invoke_all('user', 'logout', NULL, $user);
+          // The $edit parameter to hook_user() must be an array reference.
+          $edit = array();
+          user_module_invoke('logout', $edit, $user);
           // Load the anonymous user
           $user = drupal_anonymous_user();
           drupal_set_message(variable_get('countryban_readonly', "Accounts Disabled
@@ -355,7 +357,9 @@ function countryban_user($op, &$edit, &$account, $category = NUL
         if ($banvalue == 2) {
           // Destroy the current session:
           session_destroy();
-          module_invoke_all('user', 'logout', NULL, $user);
+          // The $edit parameter to hook_user() must be an array reference.
+          $edit = array();
+          user_module_invoke('logout', $edit, $user);
           // Load the anonymous user
           $user = drupal_anonymous_user();
           drupal_set_message(variable_get('countryban_completeban', "Access Denied.
jboukes’s picture

jboukes commented
Assigned: Unassigned » jboukes
Status: Needs review » Fixed

Fixed as of 1.4

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Patch submitted.