r102879 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r102878‎ | r102879 | r102880 >
Date:07:25, 13 November 2011
Author:ialex
Status:ok
Tags:
Comment:
Removed usage of $wgUser in block and unblock processing:
* Made HTMLFormField pass the HTMLForm object to the validation and filter callbacks (so that they can get a context)
* Added new parameter to SpecialBlock::checkUnblockSelf() to pass the user doing the request
* SpecialBlock::processForm() and SpecialUnblock::processUnblock() now require a context as second parameter; added SpecialBlock::processUIForm() and SpecialUnblock::processUIUnblock() as adaptators from HTMLForm as second parameter to context
Modified paths:
  • /trunk/phase3/includes/HTMLForm.php (modified) (history)
  • /trunk/phase3/includes/api/ApiBlock.php (modified) (history)
  • /trunk/phase3/includes/api/ApiUnblock.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialBlock.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUnblock.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HTMLForm.php
——@@ -891,7 +891,7 @@
892892 }
893893
894894 if ( isset( $this->mValidationCallback ) ) {
895 - return call_user_func( $this->mValidationCallback, $value, $alldata );
 895+ return call_user_func( $this->mValidationCallback, $value, $alldata, $this->mParent );
896896 }
897897
898898 return true;
——@@ -899,7 +899,7 @@
900900
901901 function filter( $value, $alldata ) {
902902 if ( isset( $this->mFilterCallback ) ) {
903 - $value = call_user_func( $this->mFilterCallback, $value, $alldata );
 903+ $value = call_user_func( $this->mFilterCallback, $value, $alldata, $this->mParent );
904904 }
905905
906906 return $value;
Index: trunk/phase3/includes/api/ApiUnblock.php
——@@ -66,7 +66,7 @@
6767 }
6868 # bug 15810: blocked admins should have limited access here
6969 if ( $user->isBlocked() ) {
70 - $status = SpecialBlock::checkUnblockSelf( $params['user'] );
 70+ $status = SpecialBlock::checkUnblockSelf( $params['user'], $user );
7171 if ( $status !== true ) {
7272 $this->dieUsageMsg( $status );
7373 }
——@@ -77,7 +77,7 @@
7878 'Reason' => is_null( $params['reason'] ) ? '' : $params['reason']
7979 );
8080 $block = Block::newFromTarget( $data['Target'] );
81 - $retval = SpecialUnblock::processUnblock( $data );
 81+ $retval = SpecialUnblock::processUnblock( $data, $this->getContext() );
8282 if ( $retval !== true ) {
8383 $this->dieUsageMsg( $retval[0] );
8484 }
Index: trunk/phase3/includes/api/ApiBlock.php
——@@ -62,7 +62,7 @@
6363 }
6464 # bug 15810: blocked admins should have limited access here
6565 if ( $user->isBlocked() ) {
66 - $status = SpecialBlock::checkUnblockSelf( $params['user'] );
 66+ $status = SpecialBlock::checkUnblockSelf( $params['user'], $user );
6767 if ( $status !== true ) {
6868 $this->dieUsageMsg( array( $status ) );
6969 }
——@@ -93,7 +93,7 @@
9494 'Confirm' => true,
9595 );
9696
97 - $retval = SpecialBlock::processForm( $data );
 97+ $retval = SpecialBlock::processForm( $data, $this->getContext() );
9898 if ( $retval !== true ) {
9999 // We don't care about multiple errors, just report one of them
100100 $this->dieUsageMsg( $retval );
Index: trunk/phase3/includes/specials/SpecialUnblock.php
——@@ -58,7 +58,7 @@
5959
6060 $form = new HTMLForm( $this->getFields(), $this->getContext() );
6161 $form->setWrapperLegend( wfMsg( 'unblockip' ) );
62 - $form->setSubmitCallback( array( __CLASS__, 'processUnblock' ) );
 62+ $form->setSubmitCallback( array( __CLASS__, 'processUIUnblock' ) );
6363 $form->setSubmitText( wfMsg( 'ipusubmit' ) );
6464 $form->addPreText( wfMsgExt( 'unblockiptext', 'parse' ) );
6565
——@@ -143,12 +143,21 @@
144144 }
145145
146146 /**
 147+ * Submit callback for an HTMLForm object
 148+ */
 149+ public static function processUIUnblock( array $data, HTMLForm $form ) {
 150+ return self::processUnblock( $data, $form->getContext() );
 151+ }
 152+
 153+ /**
147154 * Process the form
 155+ *
 156+ * @param $data Array
 157+ * @param $context IContextSource
148158 * @return Array( Array(message key, parameters) ) on failure, True on success
149159 */
150 - public static function processUnblock( array $data ){
151 - global $wgUser;
152 -
 160+ public static function processUnblock( array $data, IContextSource $context ){
 161+ $performer = $context->getUser();
153162 $target = $data['Target'];
154163 $block = Block::newFromTarget( $data['Target'] );
155164
——@@ -159,7 +168,7 @@
160169 # bug 15810: blocked admins should have limited access here. This
161170 # won't allow sysops to remove autoblocks on themselves, but they
162171 # should have ipblock-exempt anyway
163 - $status = SpecialBlock::checkUnblockSelf( $target );
 172+ $status = SpecialBlock::checkUnblockSelf( $target, $performer );
164173 if ( $status !== true ) {
165174 throw new ErrorPageError( 'badaccess', $status );
166175 }
——@@ -174,7 +183,7 @@
175184
176185 # If the name was hidden and the blocking user cannot hide
177186 # names, then don't allow any block removals...
178 - if( !$wgUser->isAllowed( 'hideuser' ) && $block->mHideName ) {
 187+ if( !$performer->isAllowed( 'hideuser' ) && $block->mHideName ) {
179188 return array( 'unblock-hideuser' );
180189 }
181190
Index: trunk/phase3/includes/specials/SpecialBlock.php
——@@ -57,7 +57,8 @@
5858
5959 public function execute( $par ) {
6060 # Permission check
61 - if( !$this->userCanExecute( $this->getUser() ) ) {
 61+ $user = $this->getUser();
 62+ if( !$this->userCanExecute( $user ) ) {
6263 $this->displayRestrictionError();
6364 return;
6465 }
——@@ -82,7 +83,7 @@
8384 $this->requestedHideUser = $request->getBool( 'wpHideUser' );
8485
8586 # bug 15810: blocked admins should have limited access here
86 - $status = self::checkUnblockSelf( $this->target );
 87+ $status = self::checkUnblockSelf( $this->target, $user );
8788 if ( $status !== true ) {
8889 throw new ErrorPageError( 'badaccess', $status );
8990 }
——@@ -99,7 +100,7 @@
100101
101102 $form = new HTMLForm( $fields, $this->getContext() );
102103 $form->setWrapperLegend( wfMsg( 'blockip-legend' ) );
103 - $form->setSubmitCallback( array( __CLASS__, 'processForm' ) );
 104+ $form->setSubmitCallback( array( __CLASS__, 'processUIForm' ) );
104105
105106 $t = $this->alreadyBlocked
106107 ? wfMsg( 'ipb-change-block' )
——@@ -492,7 +493,7 @@
493494 * @param $alldata Array
494495 * @return Message
495496 */
496 - public static function validateTargetField( $value, $alldata = null ) {
 497+ public static function validateTargetField( $value, $alldata, $form ) {
497498 global $wgBlockCIDRLimit;
498499
499500 list( $target, $type ) = self::getTargetAndType( $value );
——@@ -500,13 +501,13 @@
501502 if( $type == Block::TYPE_USER ){
502503 # TODO: why do we not have a User->exists() method?
503504 if( !$target->getId() ){
504 - return wfMessage( 'nosuchusershort',
 505+ return $form->msg( 'nosuchusershort',
505506 wfEscapeWikiText( $target->getName() ) );
506507 }
507508
508 - $status = self::checkUnblockSelf( $target );
 509+ $status = self::checkUnblockSelf( $target, $form->getUser() );
509510 if ( $status !== true ) {
510 - return wfMessage( 'badaccess', $status );
 511+ return $form->msg( 'badaccess', $status );
511512 }
512513
513514 } elseif( $type == Block::TYPE_RANGE ){
——@@ -516,40 +517,49 @@
517518 || ( IP::isIPv6( $ip ) && $wgBlockCIDRLimit['IPv6'] == 128 ) )
518519 {
519520 # Range block effectively disabled
520 - return wfMessage( 'range_block_disabled' );
 521+ return $form->msg( 'range_block_disabled' );
521522 }
522523
523524 if( ( IP::isIPv4( $ip ) && $range > 32 )
524525 || ( IP::isIPv6( $ip ) && $range > 128 ) )
525526 {
526527 # Dodgy range
527 - return wfMessage( 'ip_range_invalid' );
 528+ return $form->msg( 'ip_range_invalid' );
528529 }
529530
530531 if( IP::isIPv4( $ip ) && $range < $wgBlockCIDRLimit['IPv4'] ) {
531 - return wfMessage( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv4'] );
 532+ return $form->msg( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv4'] );
532533 }
533534
534535 if( IP::isIPv6( $ip ) && $range < $wgBlockCIDRLimit['IPv6'] ) {
535 - return wfMessage( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv6'] );
 536+ return $form->msg( 'ip_range_toolarge', $wgBlockCIDRLimit['IPv6'] );
536537 }
537538 } elseif( $type == Block::TYPE_IP ){
538539 # All is well
539540 } else {
540 - return wfMessage( 'badipaddress' );
 541+ return $form->msg( 'badipaddress' );
541542 }
542543
543544 return true;
544545 }
545546
546547 /**
 548+ * Submit callback for an HTMLForm object, will simply pass
 549+ */
 550+ public static function processUIForm( array $data, HTMLForm $form ) {
 551+ return self::processForm( $data, $form->getContext() );
 552+ }
 553+
 554+ /**
547555 * Given the form data, actually implement a block
548556 * @param $data Array
549557 * @return Bool|String
550558 */
551 - public static function processForm( array $data ){
552 - global $wgUser, $wgBlockAllowsUTEdit;
 559+ public static function processForm( array $data, IContextSource $context ){
 560+ global $wgBlockAllowsUTEdit;
553561
 562+ $performer = $context->getUser();
 563+
554564 // Handled by field validator callback
555565 // self::validateTargetField( $data['Target'] );
556566
——@@ -566,7 +576,7 @@
567577 # Give admins a heads-up before they go and block themselves. Much messier
568578 # to do this for IPs, but it's pretty unlikely they'd ever get the 'block'
569579 # permission anyway, although the code does allow for it
570 - if( $target === $wgUser->getName() &&
 580+ if( $target === $performer->getName() &&
571581 ( $data['PreviousTarget'] !== $data['Target'] || !$data['Confirm'] ) )
572582 {
573583 return array( 'ipb-blockingself' );
——@@ -598,7 +608,7 @@
599609 }
600610
601611 if( $data['HideUser'] ) {
602 - if( !$wgUser->isAllowed('hideuser') ){
 612+ if( !$performer->isAllowed('hideuser') ){
603613 # this codepath is unreachable except by a malicious user spoofing forms,
604614 # or by race conditions (user has oversight and sysop, loads block form,
605615 # and is de-oversighted before submission); so need to fail completely
——@@ -624,7 +634,7 @@
625635 # Create block object.
626636 $block = new Block();
627637 $block->setTarget( $target );
628 - $block->setBlocker( $wgUser );
 638+ $block->setBlocker( $performer );
629639 $block->mReason = $data['Reason'][0];
630640 $block->mExpiry = self::parseExpiryInput( $data['Expiry'] );
631641 $block->prevents( 'createaccount', $data['CreateAccount'] );
——@@ -634,7 +644,7 @@
635645 $block->isAutoblocking( $data['AutoBlock'] );
636646 $block->mHideName = $data['HideUser'];
637647
638 - if( !wfRunHooks( 'BlockIp', array( &$block, &$wgUser ) ) ) {
 648+ if( !wfRunHooks( 'BlockIp', array( &$block, &$performer ) ) ) {
639649 return array( 'hookaborted' );
640650 }
641651
——@@ -658,7 +668,7 @@
659669
660670 # If the name was hidden and the blocking user cannot hide
661671 # names, then don't allow any block changes...
662 - if( $currentBlock->mHideName && !$wgUser->isAllowed( 'hideuser' ) ) {
 672+ if( $currentBlock->mHideName && !$performer->isAllowed( 'hideuser' ) ) {
663673 return array( 'cant-see-hidden-user' );
664674 }
665675
——@@ -680,7 +690,7 @@
681691 $logaction = 'block';
682692 }
683693
684 - wfRunHooks( 'BlockIpComplete', array( $block, $wgUser ) );
 694+ wfRunHooks( 'BlockIpComplete', array( $block, $performer ) );
685695
686696 # Set *_deleted fields if requested
687697 if( $data['HideUser'] ) {
——@@ -689,7 +699,7 @@
690700
691701 # Can't watch a rangeblock
692702 if( $type != Block::TYPE_RANGE && $data['Watch'] ) {
693 - $wgUser->addWatch( Title::makeTitle( NS_USER, $target ) );
 703+ $performer->addWatch( Title::makeTitle( NS_USER, $target ) );
694704 }
695705
696706 # Block constructor sanitizes certain block options on insert
——@@ -791,24 +801,23 @@
792802 * others, and probably shouldn't be able to unblock themselves
793803 * either.
794804 * @param $user User|Int|String
 805+ * @param $performer User user doing the request
795806 * @return Bool|String true or error message key
796807 */
797 - public static function checkUnblockSelf( $user ) {
798 - global $wgUser;
799 -
 808+ public static function checkUnblockSelf( $user, User $performer ) {
800809 if ( is_int( $user ) ) {
801810 $user = User::newFromId( $user );
802811 } elseif ( is_string( $user ) ) {
803812 $user = User::newFromName( $user );
804813 }
805814
806 - if( $wgUser->isBlocked() ){
807 - if( $user instanceof User && $user->getId() == $wgUser->getId() ) {
 815+ if( $performer->isBlocked() ){
 816+ if( $user instanceof User && $user->getId() == $performer->getId() ) {
808817 # User is trying to unblock themselves
809 - if ( $wgUser->isAllowed( 'unblockself' ) ) {
 818+ if ( $performer->isAllowed( 'unblockself' ) ) {
810819 return true;
811820 # User blocked themselves and is now trying to reverse it
812 - } elseif ( $wgUser->blockedBy() === $wgUser->getName() ) {
 821+ } elseif ( $performer->blockedBy() === $performer->getName() ) {
813822 return true;
814823 } else {
815824 return 'ipbnounblockself';

Follow-up revisions

RevisionCommit summaryAuthorDate
r103449* Removed Preferences::validateEmail() to that e-mail address modification is...ialex10:06, 17 November 2011

Status & tagging log