Problem/Motivation

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

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

This is caused by the following code in the legal_user() function of legal/legal.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 legal-user_module_invoke-1533018-1.patch1.09 KBpillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new legal-user_module_invoke-1533018-1.patch1.09 KB

Trivial patch attached:

--- a/legal.module
+++ b/legal.module
@@ -286,7 +286,9 @@ function legal_user($op, &$edit, &$account, $category = FALSE) {
       // Destroy the current session.
       session_destroy();
       session_set_save_handler('sess_open', 'sess_close', 'sess_read', 'sess_write', 'sess_destroy_sid', 'sess_gc');
-      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);
 
       // We have to use $GLOBALS to unset a global variable.
       $user = user_load(array('uid' => 0));
pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Needs review
thedavidmeister’s picture

thedavidmeister commented
Status: Needs review » Reviewed & tested by the community

Errors are gone for me with patch in #1. Thank god.

Patch makes sense to me too and rolls nicely against 6.x-8.5

thedavidmeister’s picture

thedavidmeister commented
Issue summary: View changes

Patch submitted.

pillarsdotnet’s picture

pillarsdotnet commented

Note that user_module_invoke() will be removed in Drupal 8.
See #200344: Remove user_module_invoke() and rename user_login_block() to user_login_block_form()

thedavidmeister’s picture

thedavidmeister commented

sure, good to know. This patch should still work well for d6 and d7 though. I was seeing a warning error for every single module that implements hook_user() (which is quite a few).

thedavidmeister’s picture

thedavidmeister commented
Issue summary: View changes

Patch reviewed.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: legal-user_module_invoke-1533018-1.patch, failed testing.

robert castelo’s picture

robert castelo commented
Issue summary: View changes
Status: Needs work » Closed (outdated)

No longer supporting Drupal 6 version of Legal, so closing this old issue.