Last modified: 2014-08-15 20:13:40 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T32821, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 30821 - Title::checkPermissionHooks docs for "return true" with "$result = false" state that user should not be allowed to proceed
Title::checkPermissionHooks docs for "return true" with "$result = false" sta...
Status: REOPENED
Product: MediaWiki
Classification: Unclassified
Page protection (Other open bugs)
1.17.x
All All
: Normal minor (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-08 20:31 UTC by ju.ju2004
Modified: 2014-08-15 20:13 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description ju.ju2004 2011-09-08 20:31:27 UTC
I had some suprising results with 'userCan' hook: user was allowed to edit a page although the $result value returned by the function hooked was false. So I took a look at the code. Title::UserCan calls Title::getUserPermissionErrorsInternal which calls Title::checkPermissionHooks, which runs 'UserCan' hook. Here is a piece of this code :

private function checkPermissionHooks( $action, $user, $errors, $doExpensiveQueries, $short ) {
	...
	if ( !wfRunHooks( 'userCan', array( &$this, &$user, $action, &$result ) ) ) {
		return $result ? array() : array( array( 'badaccess-group0' ) );
	}
	...
}

If the function called by hook 'userCan' (or 'getUserPermissionsErrors' or 'getUserPermissionsErrorsExpensive') returns true, the value of $result is never considered. That means that a false "$result" will have no effect on the result of Title::UserCan. Is this conscious ? Should my hook return $result instead of true ?
Comment 1 Brion Vibber 2011-09-08 23:21:58 UTC
Returning true usually indicates that you are passing regular control back to the caller, as if you were never there.

Returning false short-circuits further processing, and in a case like this you have a chance to also return some override value through the $result outparam. You can then return either true or false in there, as I understand.
Comment 2 ju.ju2004 2011-09-09 07:16:15 UTC
Okay, but I expected something like what was described in http://www.mediawiki.org/wiki/Manual:Hooks/userCan#Table_of_combinations :
* return true: 	
** $result = true: User should be allowed to proceed. Later functions can override. 	
** $result = false: User should not be allowed to proceed. Later functions can override.
* return false:
** $result = true: User should be allowed to proceed. Later functions not consulted.
** $result = false: User should not be allowed to proceed. Later functions not consulted.

But the real table of combinations seems to be this one :
* return true:
** $result = true: User is allowed to proceed. Later functions can override. 	
** $result = false: USER IS ALLOWED TO PROCEED. Later functions can override.
* return false:
** $result = true: User is allowed to proceed. Later functions not consulted.
** $result = false: User is not allowed to proceed. Later functions not consulted.
Is there a reason for this ?
Comment 3 Mark A. Hershberger 2011-09-10 01:09:04 UTC
resolving invalid since this is just a question, not a bug report.
Comment 4 ju.ju2004 2011-09-12 06:36:43 UTC
(In reply to comment #3)
> resolving invalid since this is just a question, not a bug report.

I still think this is a bug! But I'm not sure (and I'm polite): that's why I'm asking...
Comment 5 ju.ju2004 2011-09-13 06:38:53 UTC
Excuse me, but there are no official specifications for MW. The most I can say is: IT SEEMS REALLY TO BE A BUG. Please read...

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links