From d63d4248a6c98ee3e9c8772ac9b12e0958a262c8 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Sun, 13 Mar 2005 15:29:43 +0000 Subject: [PATCH] Changed the calling protocol for function wfRunHooks() in Hooks.php. Previously, this function used variable arguments to allow different hooks to pass different parameters. However, var args silently convert reference-calling to value-calling. So a call that used to work like this: # old wfRunHooks('SomeEvent', $param1, &$param2, $param3); ...now works like this: # new wfRunHooks('SomeEvent', array($param1, &$param2, $param3)); Hook functions can now change pass-by-reference parameters correctly (e.g. $param2 in the above example). All calls to wfRunHooks() were changed and tested, and the change was documented in docs/hooks.doc. This change was originally checked in on REL1_4 branch as a bugfix, but per vibber reverted and checked in to HEAD instead. --- docs/hooks.doc | 36 +++++-- includes/Article.php | 18 ++-- includes/EditPage.php | 19 ++-- includes/Hooks.php | 174 ++++++++++++++++----------------- includes/SpecialBlockip.php | 5 +- includes/SpecialEmailuser.php | 4 +- includes/SpecialUserlogin.php | 2 +- includes/SpecialUserlogout.php | 7 +- includes/Title.php | 2 +- 9 files changed, 144 insertions(+), 123 deletions(-) diff --git a/docs/hooks.doc b/docs/hooks.doc index 6f5120e7ee..78b66ba31d 100644 --- a/docs/hooks.doc +++ b/docs/hooks.doc @@ -33,7 +33,7 @@ write extensions. Hooks are a principled alternative to local patches. Consider, for example, two options in MediaWiki. One reverses the order of a title before displaying the article; the other converts the title to all uppercase letters. Currently, in MediaWiki code, we -handle this as follows: +would handle this as follows (note: not real code, here): function showAnArticle($article) { global $wgReverseTitle, $wgCapitalizeTitle; @@ -76,9 +76,12 @@ option-specific stuff in our mainline code. Using hooks, the function becomes: function showAnArticle($article) { - if (wfRunHooks('ArticleShow', $article)) { + + if (wfRunHooks('ArticleShow', array(&$article))) { + # code to actually show the article goes here - wfRunHooks('ArticleShowComplete', $article); + + wfRunHooks('ArticleShowComplete', array(&$article)); } } @@ -89,7 +92,7 @@ on, and make changes or fix bugs. In addition, we can take all the code that deals with the little-used title-reversing options (say) and put it in one place. Instead of -having a little title-reversing if-block spread all over the codebase +having little title-reversing if-blocks spread all over the codebase in showAnArticle, deleteAnArticle, exportArticle, etc., we can concentrate it all in an extension file: @@ -117,8 +120,9 @@ place means that it's easier to read and understand; you don't have to do a grep-find to see where the $wgReverseTitle variable is used, say. If the code is well enough isolated, it can even be excluded when not -used -- making for some slight savings in memory and time at runtime. -Admins who want to have all the reversed titles can add: +used -- making for some slight savings in memory and load-up +performance at runtime. Admins who want to have all the reversed +titles can add: require_once('extensions/ReverseTitle.php'); @@ -179,6 +183,7 @@ This code would result in ircNotify being run twice when an article is saved: once for 'TimStarling', and once for 'brion'. Hooks can return three possible values: + * true: the hook has operated successfully * "some string": an error occurred; processing should stop and the error should be shown to the user @@ -198,7 +203,7 @@ could do: } Returning false makes less sense for events where the action is -complete, and will probably be ignored. +complete, and will normally be ignored. ==Using hooks== @@ -209,9 +214,9 @@ the hooks related to a particular event, like so: # ... function protect() { global $wgUser; - if (wfRunHooks('ArticleProtect', $this, $wgUser)) { + if (wfRunHooks('ArticleProtect', array(&$this, &$wgUser))) { # protect the article - wfRunHooks('ArticleProtectComplete', $this, $wgUser); + wfRunHooks('ArticleProtectComplete', array(&$this, &$wgUser)); } } @@ -221,6 +226,12 @@ if it shouldn't (an error occurred, or one of the hooks handled the action already). Checking the return value matters more for "before" hooks than for "complete" hooks. +Note that hook parameters are passed in an array; this is a necessary +inconvenience to make it possible to pass reference values (that can +be changed) into the hook code. Also note that earlier versions of +wfRunHooks took a variable number of arguments; the array() calling +protocol came about after MediaWiki 1.4rc1. + ==Events and parameters== This is a list of known events and parameters; please add to it if @@ -288,6 +299,13 @@ $from: address of sending user $subject: subject of the mail $text: text of the mail +'TitleMoveComplete': after moving an article (title) +$old: old title +$nt: new title +$user: user who did the move +$oldid: old article database ID +$newid: new article database ID + 'UnknownAction': An unknown "action" has occured (useful for defining your own actions) $action: action name diff --git a/includes/Article.php b/includes/Article.php index 16bee2175e..4e56f69109 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -1194,13 +1194,13 @@ class Article { return; } - if (wfRunHooks('WatchArticle', $wgUser, $this)) { + if (wfRunHooks('WatchArticle', array(&$wgUser, &$this))) { $wgUser->addWatch( $this->mTitle ); $wgUser->saveSettings(); - wfRunHooks('WatchArticleComplete', $wgUser, $this); - + wfRunHooks('WatchArticleComplete', array(&$wgUser, &$this)); + $wgOut->setPagetitle( wfMsg( 'addedwatch' ) ); $wgOut->setRobotpolicy( 'noindex,follow' ); @@ -1229,12 +1229,12 @@ class Article { return; } - if (wfRunHooks('UnwatchArticle', $wgUser, $this)) { + if (wfRunHooks('UnwatchArticle', array(&$wgUser, &$this))) { $wgUser->removeWatch( $this->mTitle ); $wgUser->saveSettings(); - wfRunHooks('UnwatchArticleComplete', $wgUser, $this); + wfRunHooks('UnwatchArticleComplete', array(&$wgUser, &$this)); $wgOut->setPagetitle( wfMsg( 'removedwatch' ) ); $wgOut->setRobotpolicy( 'noindex,follow' ); @@ -1287,7 +1287,7 @@ class Article { if( !$moveonly ) { $restrictions .= ":edit=" . $limit; } - if (wfRunHooks('ArticleProtect', $this, $wgUser, $limit == 'sysop', $reason, $moveonly)) { + if (wfRunHooks('ArticleProtect', array(&$this, &$wgUser, $limit == 'sysop', $reason, $moveonly))) { $dbw =& wfGetDB( DB_MASTER ); $dbw->update( 'page', @@ -1299,7 +1299,7 @@ class Article { ), 'Article::protect' ); - wfRunHooks('ArticleProtectComplete', $this, $wgUser, $limit == 'sysop', $reason, $moveonly); + wfRunHooks('ArticleProtectComplete', array(&$this, &$wgUser, $limit == 'sysop', $reason, $moveonly)); $log = new LogPage( 'protect' ); if ( $limit === '' ) { @@ -1562,7 +1562,7 @@ class Article { $fname = 'Article::doDelete'; wfDebug( $fname."\n" ); - if (wfRunHooks('ArticleDelete', $this, $wgUser, $reason)) { + if (wfRunHooks('ArticleDelete', array(&$this, &$wgUser, &$reason))) { if ( $this->doDeleteArticle( $reason ) ) { $deleted = $this->mTitle->getPrefixedText(); @@ -1578,7 +1578,7 @@ class Article { $wgOut->addHTML( '

' . $text . "

\n" ); $wgOut->returnToMain( false ); - wfRunHooks('ArticleDeleteComplete', $this, $wgUser, $reason); + wfRunHooks('ArticleDeleteComplete', array(&$this, &$wgUser, $reason)); } else { $wgOut->fatalError( wfMsg( 'cannotdelete' ) ); } diff --git a/includes/EditPage.php b/includes/EditPage.php index 544e2bcb4e..36a6d01b3b 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -336,13 +336,14 @@ class EditPage { $wgOut->redirect( $this->mTitle->getFullURL() ); return; } - if (wfRunHooks('ArticleSave', $this->mArticle, $wgUser, $this->textbox1, - $this->summary, $this->minoredit, $this->watchthis, NULL)) + if (wfRunHooks('ArticleSave', array(&$this->mArticle, &$wgUser, &$this->textbox1, + &$this->summary, &$this->minoredit, &$this->watchthis, NULL))) { $this->mArticle->insertNewArticle( $this->textbox1, $this->summary, $this->minoredit, $this->watchthis ); - wfRunHooks('ArticleSaveComplete', $this->mArticle, $wgUser, $this->textbox1, - $this->summary, $this->minoredit, $this->watchthis, NULL); + wfRunHooks('ArticleSaveComplete', array(&$this->mArticle, &$wgUser, $this->textbox1, + $this->summary, $this->minoredit, + $this->watchthis, NULL)); } return; } @@ -403,15 +404,17 @@ class EditPage { } } - if (wfRunHooks('ArticleSave', $this->mArticle, $wgUser, $text, $this->summary, - $this->minoredit, $this->watchthis, $sectionanchor)) + if (wfRunHooks('ArticleSave', array(&$this->mArticle, &$wgUser, &$text, + &$this->summary, &$this->minoredit, + &$this->watchthis, &$sectionanchor))) { # update the article here if($this->mArticle->updateArticle( $text, $this->summary, $this->minoredit, $this->watchthis, '', $sectionanchor )) { - wfRunHooks('ArticleSaveComplete', $this->mArticle, $wgUser, $text, $this->summary, - $this->minoredit, $this->watchthis, $sectionanchor); + wfRunHooks('ArticleSaveComplete', array(&$this->mArticle, &$wgUser, $text, + $this->summary, $this->minoredit, + $this->watchthis, $sectionanchor)); return; } else diff --git a/includes/Hooks.php b/includes/Hooks.php index b156a6a673..9198d00433 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -1,7 +1,7 @@ . + * Copyright 2004, 2005 Evan Prodromou . * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -24,105 +24,103 @@ if (defined('MEDIAWIKI')) { -/** - * Because programmers assign to $wgHooks, we need to be very - * careful about its contents. So, there's a lot more error-checking - * in here than would normally be necessary. - */ -function wfRunHooks() { + /** + * Because programmers assign to $wgHooks, we need to be very + * careful about its contents. So, there's a lot more error-checking + * in here than would normally be necessary. + */ + + function wfRunHooks($event, $args) { - global $wgHooks; - - if (!is_array($wgHooks)) { - wfDieDebugBacktrace("Global hooks array is not an array!\n"); - return false; - } - - $args = func_get_args(); - - if (count($args) < 1) { - wfDieDebugBacktrace("No event name given for wfRunHooks().\n"); - return false; - } - - $event = array_shift($args); - - if (!array_key_exists($event, $wgHooks)) { - return true; - } + global $wgHooks; - if (!is_array($wgHooks[$event])) { - wfDieDebugBacktrace("Hooks array for event '$event' is not an array!\n"); - return false; - } + if (!is_array($wgHooks)) { + wfDieDebugBacktrace("Global hooks array is not an array!\n"); + return false; + } - foreach ($wgHooks[$event] as $hook) { + if (!array_key_exists($event, $wgHooks)) { + return true; + } - $object = NULL; - $method = NULL; - $func = NULL; - $data = NULL; - $have_data = false; - - /* $hook can be: a function, an object, an array of $function and $data, - * an array of just a function, an array of object and method, or an - * array of object, method, and data. - */ + if (!is_array($wgHooks[$event])) { + wfDieDebugBacktrace("Hooks array for event '$event' is not an array!\n"); + return false; + } - if (is_array($hook)) { - if (count($hook) < 1) { - wfDieDebugBacktrace("Empty array in hooks for " . $event . "\n"); - } else if (is_object($hook[0])) { - $object = $hook[0]; - if (count($hook) < 2) { - $method = "on" . $event; - } else { - $method = $hook[1]; - if (count($hook) > 2) { - $data = $hook[2]; + foreach ($wgHooks[$event] as $hook) { + + $object = NULL; + $method = NULL; + $func = NULL; + $data = NULL; + $have_data = false; + + /* $hook can be: a function, an object, an array of $function and $data, + * an array of just a function, an array of object and method, or an + * array of object, method, and data. + */ + + if (is_array($hook)) { + if (count($hook) < 1) { + wfDieDebugBacktrace("Empty array in hooks for " . $event . "\n"); + } else if (is_object($hook[0])) { + $object = $hook[0]; + if (count($hook) < 2) { + $method = "on" . $event; + } else { + $method = $hook[1]; + if (count($hook) > 2) { + $data = $hook[2]; + $have_data = true; + } + } + } else if (is_string($hook[0])) { + $func = $hook[0]; + if (count($hook) > 1) { + $data = $hook[1]; $have_data = true; } + } else { + wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n"); } - } else if (is_string($hook[0])) { - $func = $hook[0]; - if (count($hook) > 1) { - $data = $hook[1]; - $have_data = true; - } + } else if (is_string($hook)) { # functions look like strings, too + $func = $hook; + } else if (is_object($hook)) { + $object = $hook; + $method = "on" . $event; } else { wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n"); } - } else if (is_string($hook)) { # functions look like strings, too - $func = $hook; - } else if (is_object($hook)) { - $object = $hook; - $method = "on" . $event; - } else { - wfDieDebugBacktrace("Unknown datatype in hooks for " . $event . "\n"); - } - - if ($have_data) { - $hook_args = array_merge(array($data), $args); - } else { - $hook_args = $args; - } - - if ($object) { - $retval = call_user_func_array(array($object, $method), $hook_args); - } else { - $retval = call_user_func_array($func, $hook_args); + + /* We put the first data element on, if needed. */ + + if ($have_data) { + $hook_args = array_merge(array($data), $args); + } else { + $hook_args = $args; + } + + /* Call the hook. */ + + if ($object) { + $retval = call_user_func_array(array($object, $method), $hook_args); + } else { + $retval = call_user_func_array($func, $hook_args); + } + + /* String return is an error; false return means stop processing. */ + + if (is_string($retval)) { + global $wgOut; + $wgOut->fatalError($retval); + return false; + } else if (!$retval) { + return false; + } } - if (is_string($retval)) { - global $wgOut; - $wgOut->fatalError($retval); - return false; - } else if (!$retval) { - return false; - } + return true; } - - return true; -} } /* if defined(MEDIAWIKI) */ -?> \ No newline at end of file +?> \ No newline at end of file diff --git a/includes/SpecialBlockip.php b/includes/SpecialBlockip.php index 9c60423b30..fe34d41ca9 100644 --- a/includes/SpecialBlockip.php +++ b/includes/SpecialBlockip.php @@ -172,10 +172,11 @@ class IPBlockForm { $ban = new Block( $this->BlockAddress, $userId, $wgUser->getID(), $this->BlockReason, wfTimestampNow(), 0, $expiry ); - if (wfRunHooks('BlockIp', $ban, $wgUser)) { + if (wfRunHooks('BlockIp', array(&$ban, &$wgUser))) { + $ban->insert(); - wfRunHooks('BlockIpComplete', $ban, $wgUser); + wfRunHooks('BlockIpComplete', array($ban, $wgUser)); # Make log entry $log = new LogPage( 'block' ); diff --git a/includes/SpecialEmailuser.php b/includes/SpecialEmailuser.php index 7bb4a6177a..577a8e9366 100644 --- a/includes/SpecialEmailuser.php +++ b/includes/SpecialEmailuser.php @@ -143,7 +143,7 @@ class EmailUserForm { $from = wfQuotedPrintable( $wgUser->getName() ) . " <" . $wgUser->getEmail() . ">"; $subject = wfQuotedPrintable( $this->subject ); - if (wfRunHooks('EmailUser', $this->mAddress, $from, $subject, $this->text)) { + if (wfRunHooks('EmailUser', array(&$this->mAddress, &$from, &$subject, &$this->text))) { $mailResult = userMailer( $this->mAddress, $from, $subject, $this->text ); @@ -151,7 +151,7 @@ class EmailUserForm { $titleObj = Title::makeTitle( NS_SPECIAL, "Emailuser" ); $encTarget = wfUrlencode( $this->target ); $wgOut->redirect( $titleObj->getFullURL( "target={$encTarget}&action=success" ) ); - wfRunHooks('EmailUserComplete', $this->mAddress, $from, $subject, $this->text); + wfRunHooks('EmailUserComplete', array($this->mAddress, $from, $subject, $this->text)); } else { $wgOut->addHTML( wfMsg( "usermailererror" ) . $mailResult); } diff --git a/includes/SpecialUserlogin.php b/includes/SpecialUserlogin.php index 28bb93cdd3..76db17bb81 100644 --- a/includes/SpecialUserlogin.php +++ b/includes/SpecialUserlogin.php @@ -461,7 +461,7 @@ class LoginForm { # Run any hooks; ignore results - wfRunHooks('UserLoginComplete', $wgUser); + wfRunHooks('UserLoginComplete', array(&$wgUser)); $wgOut->setPageTitle( wfMsg( 'loginsuccesstitle' ) ); $wgOut->setRobotpolicy( 'noindex,nofollow' ); diff --git a/includes/SpecialUserlogout.php b/includes/SpecialUserlogout.php index 6c9a8882e0..0a34a8e50a 100644 --- a/includes/SpecialUserlogout.php +++ b/includes/SpecialUserlogout.php @@ -11,16 +11,17 @@ function wfSpecialUserlogout() { global $wgUser, $wgOut, $returnto; - if (wfRunHooks('UserLogout', $wgUser)) { + if (wfRunHooks('UserLogout', array(&$wgUser))) { $wgUser->logout(); - + + wfRunHooks('UserLogoutComplete', array(&$wgUser)); + $wgOut->mCookies = array(); $wgOut->setRobotpolicy( 'noindex,nofollow' ); $wgOut->addHTML( wfMsg( 'logouttext' ) ); $wgOut->returnToMain(); - wfRunHooks('UserLogoutComplete', $wgUser); } } diff --git a/includes/Title.php b/includes/Title.php index 0de9cd1e5e..dcfe6b2464 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1443,7 +1443,7 @@ class Title { $u = new SearchUpdate( $newid, $this->getPrefixedDBkey(), '' ); $u->doUpdate(); - wfRunHooks( 'TitleMoveComplete', $this, $nt, $wgUser, $oldid, $newid ); + wfRunHooks( 'TitleMoveComplete', array(&$this, &$nt, &$wgUser, $oldid, $newid) ); return true; } -- 2.20.1