r53173 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r53172‎ | r53173 | r53174 >
Date:15:36, 13 July 2009
Author:tstarling
Status:resolved (Comments)
Tags:
Comment:
Fix dodgy uses of wfMsgHtml() and related HTML escaping
Modified paths:
  • /trunk/phase3/includes/LogPage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDeletedContributions.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserrights.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialUserrights.php
@@ -498,9 +498,13 @@
499499 $ret .= "\t<td style='vertical-align:top;'>\n";
500500 foreach( $column as $group => $checkbox ) {
501501 $attr = $checkbox['disabled'] ? array( 'disabled' => 'disabled' ) : array();
502 - $text = $checkbox['irreversible']
503 - ? wfMsgHtml( 'userrights-irreversible-marker', User::getGroupMember( $group ) )
504 - : User::getGroupMember( $group );
 502+
 503+ if ( $checkbox['irreversible'] ) {
 504+ $text = htmlspecialchars( wfMsg( 'userrights-irreversible-marker',
 505+ User::getGroupMember( $group ) ) );
 506+ } else {
 507+ $text = htmlspecialchars( User::getGroupMember( $group ) );
 508+ }
505509 $checkboxHtml = Xml::checkLabel( $text, "wpGroup-" . $group,
506510 "wpGroup-" . $group, $checkbox['set'], $attr );
507511 $ret .= "\t\t" . ( $checkbox['disabled']
Index: trunk/phase3/includes/specials/SpecialContributions.php
@@ -144,7 +144,7 @@
145145 $sk = $wgUser->getSkin();
146146
147147 if( 0 == $id ) {
148 - $user = $nt->getText();
 148+ $user = htmlspecialchars( $nt->getText() );
149149 } else {
150150 $user = $sk->link( $nt, htmlspecialchars( $nt->getText() ) );
151151 }
Index: trunk/phase3/includes/specials/SpecialDeletedContributions.php
@@ -324,7 +324,7 @@
325325 $sk = $wgUser->getSkin();
326326
327327 if ( 0 == $id ) {
328 - $user = $nt->getText();
 328+ $user = htmlspecialchars( $nt->getText() );
329329 } else {
330330 $user = $sk->link( $nt, htmlspecialchars( $nt->getText() ) );
331331 }
Index: trunk/phase3/includes/LogPage.php
@@ -214,11 +214,12 @@
215215 self::formatBlockFlags( $params[2], is_null( $skin ) ) : '';
216216 // Page protections
217217 } else if ( $type == 'protect' && count($params) == 3 ) {
218 - $details .= " {$params[1]}"; // restrictions and expiries
219218 if( $params[2] ) {
220219 if ( $skin ) {
 220+ $details .= htmlspecialchars( " {$params[1]}" ); // restrictions and expiries
221221 $details .= ' ['.wfMsg('protect-summary-cascade').']';
222222 } else {
 223+ $details .= " {$params[1]}";
223224 $details .= ' ['.wfMsgForContent('protect-summary-cascade').']';
224225 }
225226 }
@@ -245,7 +246,7 @@
246247 $details .= ': '.RevisionDeleter::getLogMessage( $count, $nfield, $ofield, true );
247248 }
248249 if ( $skin ) {
249 - $rv = wfMsgHtml( $wgLogActions[$key], $params ) . $details;
 250+ $rv = htmlspecialchars( wfMsg( $wgLogActions[$key], $params ) ) . $details;
250251 } else {
251252 $rv = wfMsgExt( $wgLogActions[$key], array( 'parsemag', 'escape', 'replaceafter', 'content' ), $params ) . $details;
252253 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r53549(bug 19839) fix for r53173: Comments in log items are no more double escapedialex12:07, 20 July 2009
r53800Allow HTML in message emailnotauthenticated, fixes formatting regression caus...werdna10:32, 27 July 2009
r56564bug 20701: fixed strange breakage from r53173aaron20:20, 17 September 2009

Comments

#Comment by Werdna (talk | contribs)   16:33, 13 July 2009
  1. (Block log); 16:25 . . Abuse filter (Talk | contribs) blocked <a href="https://www.mediawiki.org/test/view/Special:Contributions/119.70.0.0/16" title="Special:Contributions/119.70.0.0/16" class="mw-userlink">119.70.0.0/16</a> (<a href="https://www.mediawiki.org/test/edit/User_talk:119.70.0.0/16?redlink=1" class="new" title="User talk:119.70.0.0/16 (page does not exist)">Talk</a>) with an expiry time of indefinite (account creation disabled, angry-autoblock) (Automatically blocked by abuse filter. Description of matched rule: Spam)

I think there's some double-escaping going on with block logs here :)

#Comment by Bryan (talk | contribs)   19:00, 13 July 2009

Each member of $params should be htmlspecialchar'ed separately.

#Comment by OverlordQ (talk | contribs)   04:04, 20 July 2009

$params are already htmlspecialchar'd in getTitleLink

so on 249 htmlspecialchar turns http://oq.pastebin.com/m5822e0e6 into http://oq.pastebin.com/m2481c256

Status & tagging log