r103751 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103750‎ | r103751 | r103752 >
Date:11:29, 20 November 2011
Author:catrope
Status:resolved (Comments)
Tags:
Comment:
In the spirit of r103745, guard against invalid user names in SpecialContributions and SpecialDeletedContributions by checking for invalidity early, then passing User objects around. Loosely based on Søren Løvborg's patch on bug 26854
Modified paths:
  • /trunk/phase3/includes/specials/SpecialContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDeletedContributions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialContributions.php
@@ -74,21 +74,19 @@
7575 $this->opts['target'] = $target;
7676 $this->opts['topOnly'] = $request->getBool( 'topOnly' );
7777
78 - $nt = Title::makeTitleSafe( NS_USER, $target );
79 - if( !$nt ) {
 78+ $userObj = User::newFromName( $target, false );
 79+ if( !$userObj ) {
8080 $out->addHTML( $this->getForm() );
8181 return;
8282 }
83 - $id = User::idFromName( $nt->getText() );
 83+ $nt = $userObj->getUserPage();
 84+ $id = $userObj->getID();
8485
8586 if( $this->opts['contribs'] != 'newbie' ) {
8687 $target = $nt->getText();
87 - $out->addSubtitle( $this->contributionsSub( $nt, $id ) );
 88+ $out->addSubtitle( $this->contributionsSub( $userObj ) );
8889 $out->setHTMLTitle( $this->msg( 'pagetitle', wfMsgExt( 'contributions-title', array( 'parsemag' ), $target ) ) );
89 - $userObj = User::newFromName( $target, false );
90 - if ( is_object( $userObj ) ) {
91 - $this->getSkin()->setRelevantUser( $userObj );
92 - }
 90+ $this->getSkin()->setRelevantUser( $userObj );
9391 } else {
9492 $out->addSubtitle( $this->msg( 'sp-contributions-newbies-sub') );
9593 $out->setHTMLTitle( $this->msg( 'pagetitle', wfMsg( 'sp-contributions-newbies-title' ) ) );
@@ -193,8 +191,7 @@
194192 if ( IP::isIPAddress( $target ) ) {
195193 $message = 'sp-contributions-footer-anon';
196194 } else {
197 - $userObj = User::newFromName( $target );
198 - if ( !$userObj || $userObj->isAnon() ) {
 195+ if ( $userObj->isAnon() ) {
199196 // No message for non-existing users
200197 return;
201198 }
@@ -211,19 +208,18 @@
212209
213210 /**
214211 * Generates the subheading with links
215 - * @param $nt Title object for the target
216 - * @param $id Integer: User ID for the target
 212+ * @param $userObj User object for the target
217213 * @return String: appropriately-escaped HTML to be output literally
218214 * @todo FIXME: Almost the same as getSubTitle in SpecialDeletedContributions.php. Could be combined.
219215 */
220 - protected function contributionsSub( $nt, $id ) {
221 - if ( $id === null ) {
222 - $user = htmlspecialchars( $nt->getText() );
 216+ protected function contributionsSub( $userObj ) {
 217+ if ( $userObj->isAnon() ) {
 218+ $user = htmlspecialchars( $userObj->getName() );
223219 } else {
224 - $user = Linker::link( $nt, htmlspecialchars( $nt->getText() ) );
 220+ $user = Linker::link( $userObj->getUserPage(), htmlspecialchars( $userObj->getName() ) );
225221 }
226 - $userObj = User::newFromName( $nt->getText(), /* check for username validity not needed */ false );
227 - $talk = $nt->getTalkPage();
 222+ $nt = $userObj->getUserPage();
 223+ $talk = $userObj->getTalkPage();
228224 if( $talk ) {
229225 $tools = self::getUserLinks( $nt, $talk, $userObj, $this->getUser() );
230226 $links = $this->getLang()->pipeList( $tools );
@@ -243,7 +239,7 @@
244240 $userObj->isAnon() ?
245241 'sp-contributions-blocked-notice-anon' :
246242 'sp-contributions-blocked-notice',
247 - $nt->getText() # Support GENDER in 'sp-contributions-blocked-notice'
 243+ $userObj->getName() # Support GENDER in 'sp-contributions-blocked-notice'
248244 ),
249245 'offset' => '' # don't use WebRequest parameter offset
250246 )
Index: trunk/phase3/includes/specials/SpecialDeletedContributions.php
@@ -287,15 +287,16 @@
288288 $options['limit'] = $request->getInt( 'limit', $wgQueryPageDefaultLimit );
289289 $options['target'] = $target;
290290
291 - $nt = Title::makeTitleSafe( NS_USER, $target );
292 - if ( !$nt ) {
 291+ $userObj = User::newFromName( $target );
 292+ if ( !$userObj ) {
293293 $out->addHTML( $this->getForm( '' ) );
294294 return;
295295 }
296 - $id = User::idFromName( $nt->getText() );
 296+ $nt = $userObj->getUserPage();
 297+ $id = $userObj->getID();
297298
298 - $target = $nt->getText();
299 - $out->addSubtitle( $this->getSubTitle( $nt, $id ) );
 299+ $target = $userObj->getName();
 300+ $out->addSubtitle( $this->getSubTitle( $userObj ) );
300301
301302 if ( ( $ns = $request->getVal( 'namespace', null ) ) !== null && $ns !== '' ) {
302303 $options['namespace'] = intval( $ns );
@@ -336,18 +337,18 @@
337338
338339 /**
339340 * Generates the subheading with links
340 - * @param $nt Title object for the target
341 - * @param $id Integer: User ID for the target
 341+ * @param $userObj User object for the target
342342 * @return String: appropriately-escaped HTML to be output literally
343343 * @todo FIXME: Almost the same as contributionsSub in SpecialContributions.php. Could be combined.
344344 */
345 - function getSubTitle( $nt, $id ) {
346 - if ( $id === null ) {
347 - $user = htmlspecialchars( $nt->getText() );
 345+ function getSubTitle( $userObj ) {
 346+ if ( $userObj->isAnon() ) {
 347+ $user = htmlspecialchars( $userObj->getName() );
348348 } else {
349 - $user = Linker::link( $nt, htmlspecialchars( $nt->getText() ) );
 349+ $user = Linker::link( $userObj->getPage(), htmlspecialchars( $userObj->getText() ) );
350350 }
351 - $userObj = User::newFromName( $nt->getText(), /* check for username validity not needed */ false );
 351+ $nt = $userObj->getUserPage();
 352+ $id = $userObj->getID();
352353 $talk = $nt->getTalkPage();
353354 if( $talk ) {
354355 # Talk page link

Follow-up revisions

RevisionCommit summaryAuthorDate
r103753Fix typo in r103751catrope13:02, 20 November 2011
r104013Fix typo in r103751catrope10:25, 23 November 2011
r113819* (bug 34889) User name should be normalized on Special:Contributions. Fixes ...maxsem16:31, 14 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r103745(bug 26854) Invalid user names go unchecked. Applied most of the patch submit...catrope10:55, 20 November 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   01:22, 23 November 2011
$user = Linker::link( $userObj->getPage(), htmlspecialchars( $userObj->getText() ) );

User doesn't have a getText() method. Did you mean getName()?

Fatal error: Call to undefined method User::getText() in /var/www/trunk/includes/specials/SpecialDeletedContributions.php on line 348

Call Stack:
    0.0004     659272   1. {main}() /var/www/trunk/index.php:0
    0.0868   19330848   2. MediaWiki->run() /var/www/trunk/index.php:58
    0.0868   19330848   3. MediaWiki->main() /var/www/trunk/includes/Wiki.php:524
    0.0868   19330984   4. MediaWiki->performRequest() /var/www/trunk/includes/Wiki.php:605
    0.0937   19964632   5. SpecialPageFactory::executePath() /var/www/trunk/includes/Wiki.php:237
    0.1093   21222416   6. DeletedContributionsPage->execute() /var/www/trunk/includes/SpecialPageFactory.php:470
    0.1640   31870560   7. DeletedContributionsPage->getSubTitle() /var/www/trunk/includes/specials/SpecialDeletedContributions.php:299

#Comment by Catrope (talk | contribs)   10:25, 23 November 2011

Grr, yes. And to think I fixed something very close to that in r103753. Fixed in r104013.

#Comment by Duplicatebug (talk | contribs)   10:04, 3 March 2012

The use of User::newFromName does not remove leading/trailing whitespaces, please have a look at bug 34889. Thanks.

#Comment by MaxSem (talk | contribs)   19:44, 8 March 2012

Fixmed per #c31764.

#Comment by MaxSem (talk | contribs)   16:32, 14 March 2012

Resolved in r113819.

Status & tagging log