r100750 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r100749‎ | r100750 | r100751 >
Date:21:51, 25 October 2011
Author:reedy
Status:ok
Tags:
Comment:
Modified paths:
  • /branches/REL1_18/phase3 (modified) (history)
  • /branches/REL1_18/phase3/CREDITS (modified) (history)
  • /branches/REL1_18/phase3/docs/hooks.txt (modified) (history)
  • /branches/REL1_18/phase3/includes (modified) (history)
  • /branches/REL1_18/phase3/includes/ChangesList.php (modified) (history)
  • /branches/REL1_18/phase3/includes/GlobalFunctions.php (modified) (history)
  • /branches/REL1_18/phase3/includes/api (modified) (history)
  • /branches/REL1_18/phase3/includes/api/ApiQueryCategoryMembers.php (modified) (history)
  • /branches/REL1_18/phase3/includes/api/ApiQueryRevisions.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/BMP.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/GIFMetadataExtractor.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/JpegMetadataExtractor.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/PNGMetadataExtractor.php (modified) (history)
  • /branches/REL1_18/phase3/includes/resourceloader/ResourceLoaderUserGroupsModule.php (modified) (history)
  • /branches/REL1_18/phase3/includes/specials (modified) (history)
  • /branches/REL1_18/phase3/includes/specials/SpecialUpload.php (modified) (history)
  • /branches/REL1_18/phase3/maintenance/install.php (modified) (history)
  • /branches/REL1_18/phase3/tests/phpunit/includes/parser/MagicVariableTest.php (modified) (history)

Diff [purge]

Index: branches/REL1_18/phase3/maintenance/install.php
@@ -40,7 +40,7 @@
4141
4242 $this->addArg( 'admin', 'The username of the wiki administrator (WikiSysop)', true );
4343 $this->addOption( 'pass', 'The password for the wiki administrator. You will be prompted for this if it isn\'t provided', false, true );
44 - $this->addOption( 'email', 'The email for the wiki administrator', false, true );
 44+ /* $this->addOption( 'email', 'The email for the wiki administrator', false, true ); */
4545 $this->addOption( 'scriptpath', 'The relative path of the wiki in the web server (/wiki)', false, true );
4646
4747 $this->addOption( 'lang', 'The language to use (en)', false, true );
Index: branches/REL1_18/phase3/docs/hooks.txt
@@ -1590,6 +1590,10 @@
15911591 'SkinTemplateToolboxEnd': Called by SkinTemplate skins after toolbox links have
15921592 been rendered (useful for adding more)
15931593 $sk: The QuickTemplate based skin template running the hook.
 1594+$dummy: Called when SkinTemplateToolboxEnd is used from a BaseTemplate skin,
 1595+ extensions that add support for BaseTemplateToolbox should watch for this dummy
 1596+ parameter with "$dummy=false" in their code and return without echoing any html
 1597+ to avoid creating duplicate toolbox items.
15941598
15951599 'SoftwareInfo': Called by Special:Version for returning information about
15961600 the software
Property changes on: branches/REL1_18/phase3/docs/hooks.txt
___________________________________________________________________
Modified: svn:mergeinfo
15971601 Merged /trunk/phase3/docs/hooks.txt:r99370,99700,100239,100242,100347,100510,100572,100592
Index: branches/REL1_18/phase3/CREDITS
@@ -11,6 +11,7 @@
1212 * Ashar Voultoiz
1313 * Brian Wolff
1414 * Bertrand Grondin
 15+* Brad Jorsch
1516 * Brion Vibber
1617 * Bryan Tong Minh
1718 * Chad Horohoe
Index: branches/REL1_18/phase3/tests/phpunit/includes/parser/MagicVariableTest.php
@@ -111,6 +111,24 @@
112112 $this->assertUnPadded( 'revisionmonth1', $month );
113113 }
114114
 115+ /**
 116+ * Rough tests for {{SERVERNAME}} magic word
 117+ * Bug 31176
 118+ */
 119+ function testServernameFromDifferentProtocols() {
 120+ global $wgServer;
 121+ $saved_wgServer= $wgServer;
 122+
 123+ $wgServer = 'http://localhost/';
 124+ $this->assertMagic( 'localhost', 'servername' );
 125+ $wgServer = 'https://localhost/';
 126+ $this->assertMagic( 'localhost', 'servername' );
 127+ $wgServer = '//localhost/'; # bug 31176
 128+ $this->assertMagic( 'localhost', 'servername' );
 129+
 130+ $wgServer = $saved_wgServer;
 131+ }
 132+
115133 ############### HELPERS ############################################
116134
117135 /** assertion helper expecting a magic output which is zero padded */
Index: branches/REL1_18/phase3/includes/GlobalFunctions.php
@@ -3620,3 +3620,39 @@
36213621 function wfRunHooks( $event, $args = array() ) {
36223622 return Hooks::run( $event, $args );
36233623 }
 3624+
 3625+/**
 3626+ * Wrapper around php's unpack.
 3627+ *
 3628+ * @param $format String: The format string (See php's docs)
 3629+ * @param $data: A binary string of binary data
 3630+ * @param $length integer or false: The minimun length of $data. This is to
 3631+ * prevent reading beyond the end of $data. false to disable the check.
 3632+ *
 3633+ * Also be careful when using this function to read unsigned 32 bit integer
 3634+ * because php might make it negative.
 3635+ *
 3636+ * @throws MWException if $data not long enough, or if unpack fails
 3637+ * @return Associative array of the extracted data
 3638+ */
 3639+function wfUnpack( $format, $data, $length=false ) {
 3640+ if ( $length !== false ) {
 3641+ $realLen = strlen( $data );
 3642+ if ( $realLen < $length ) {
 3643+ throw new MWException( "Tried to use wfUnpack on a "
 3644+ . "string of length $realLen, but needed one "
 3645+ . "of at least length $length."
 3646+ );
 3647+ }
 3648+ }
 3649+
 3650+ wfSuppressWarnings();
 3651+ $result = unpack( $format, $data );
 3652+ wfRestoreWarnings();
 3653+
 3654+ if ( $result === false ) {
 3655+ // If it cannot extract the packed data.
 3656+ throw new MWException( "unpack could not unpack binary data" );
 3657+ }
 3658+ return $result;
 3659+}
Property changes on: branches/REL1_18/phase3/includes/GlobalFunctions.php
___________________________________________________________________
Modified: svn:mergeinfo
36243660 Merged /trunk/phase3/includes/GlobalFunctions.php:r100572,100592
Index: branches/REL1_18/phase3/includes/api/ApiQueryCategoryMembers.php
@@ -143,7 +143,9 @@
144144 $contWhere = "cl_sortkey $op $escSortkey OR " .
145145 "(cl_sortkey = $escSortkey AND " .
146146 "cl_from $op= $from)";
147 -
 147+ // The below produces ORDER BY cl_sortkey, cl_from, possibly with DESC added to each of them
 148+ $this->addWhereRange( 'cl_sortkey', $dir, null, null );
 149+ $this->addWhereRange( 'cl_from', $dir, null, null );
148150 } else {
149151 // The below produces ORDER BY cl_sortkey, cl_from, possibly with DESC added to each of them
150152 $this->addWhereRange( 'cl_sortkey',
Index: branches/REL1_18/phase3/includes/api/ApiQueryRevisions.php
@@ -610,9 +610,9 @@
611611 'endid' => 'Stop revision enumeration on this revid (enum)',
612612 'start' => 'From which revision timestamp to start enumeration (enum)',
613613 'end' => 'Enumerate up to this timestamp (enum)',
614 - 'dir' => $this->getDirectionDescription( $p ),
615 - 'user' => 'Only include revisions made by user',
616 - 'excludeuser' => 'Exclude revisions made by user',
 614+ 'dir' => $this->getDirectionDescription( $p, ' (enum)' ),
 615+ 'user' => 'Only include revisions made by user (enum)',
 616+ 'excludeuser' => 'Exclude revisions made by user (enum)',
617617 'expandtemplates' => 'Expand templates in revision content',
618618 'generatexml' => 'Generate XML parse tree for revision content',
619619 'parse' => 'Parse revision content. For performance reasons if this option is used, rvlimit is enforced to 1.',
Property changes on: branches/REL1_18/phase3/includes/api
___________________________________________________________________
Modified: svn:mergeinfo
620620 Merged /trunk/phase3/includes/api:r98997,99118,99370,99700,100239,100242,100347,100510,100572,100592
Index: branches/REL1_18/phase3/includes/resourceloader/ResourceLoaderUserGroupsModule.php
@@ -33,15 +33,14 @@
3434 protected function getPages( ResourceLoaderContext $context ) {
3535 if ( $context->getUser() ) {
3636 $user = User::newFromName( $context->getUser() );
37 - if( $user instanceof User ){
 37+ if ( $user instanceof User ) {
3838 $pages = array();
39 - foreach( $user->getEffectiveGroups() as $group ){
40 - if( in_array( $group, array( '*', 'user' ) ) ){
 39+ foreach( $user->getEffectiveGroups() as $group ) {
 40+ if ( in_array( $group, array( '*', 'user' ) ) ) {
4141 continue;
4242 }
43 - $g = ucfirst( $group );
44 - $pages["MediaWiki:Group-$g.js"] = array( 'type' => 'script' );
45 - $pages["MediaWiki:Group-$g.css"] = array( 'type' => 'style' );
 43+ $pages["MediaWiki:Group-$group.js"] = array( 'type' => 'script' );
 44+ $pages["MediaWiki:Group-$group.css"] = array( 'type' => 'style' );
4645 }
4746 return $pages;
4847 }
Index: branches/REL1_18/phase3/includes/media/GIFMetadataExtractor.php
@@ -50,7 +50,7 @@
5151 throw new Exception( "File $filename does not exist" );
5252 }
5353
54 - $fh = fopen( $filename, 'r' );
 54+ $fh = fopen( $filename, 'rb' );
5555
5656 if ( !$fh ) {
5757 throw new Exception( "Unable to open file $filename" );
@@ -95,6 +95,7 @@
9696 self::skipBlock( $fh );
9797 } elseif ( $buf == self::$gif_extension_sep ) {
9898 $buf = fread( $fh, 1 );
 99+ if ( strlen( $buf ) < 1 ) throw new Exception( "Ran out of input" );
99100 $extension_code = unpack( 'C', $buf );
100101 $extension_code = $extension_code[1];
101102
@@ -105,6 +106,7 @@
106107 fread( $fh, 1 ); // Transparency, disposal method, user input
107108
108109 $buf = fread( $fh, 2 ); // Delay, in hundredths of seconds.
 110+ if ( strlen( $buf ) < 2 ) throw new Exception( "Ran out of input" );
109111 $delay = unpack( 'v', $buf );
110112 $delay = $delay[1];
111113 $duration += $delay * 0.01;
@@ -112,6 +114,7 @@
113115 fread( $fh, 1 ); // Transparent colour index
114116
115117 $term = fread( $fh, 1 ); // Should be a terminator
 118+ if ( strlen( $term ) < 1 ) throw new Exception( "Ran out of input" );
116119 $term = unpack( 'C', $term );
117120 $term = $term[1];
118121 if ($term != 0 ) {
@@ -150,6 +153,7 @@
151154 // Application extension (Netscape info about the animated gif)
152155 // or XMP (or theoretically any other type of extension block)
153156 $blockLength = fread( $fh, 1 );
 157+ if ( strlen( $blockLength ) < 1 ) throw new Exception( "Ran out of input" );
154158 $blockLength = unpack( 'C', $blockLength );
155159 $blockLength = $blockLength[1];
156160 $data = fread( $fh, $blockLength );
@@ -172,6 +176,7 @@
173177
174178 // Unsigned little-endian integer, loop count or zero for "forever"
175179 $loopData = fread( $fh, 2 );
 180+ if ( strlen( $loopData ) < 2 ) throw new Exception( "Ran out of input" );
176181 $loopData = unpack( 'v', $loopData );
177182 $loopCount = $loopData[1];
178183
@@ -209,6 +214,7 @@
210215 } elseif ( $buf == self::$gif_term ) {
211216 break;
212217 } else {
 218+ if ( strlen( $buf ) < 1 ) throw new Exception( "Ran out of input" );
213219 $byte = unpack( 'C', $buf );
214220 $byte = $byte[1];
215221 throw new Exception( "At position: ".ftell($fh). ", Unknown byte ".$byte );
@@ -242,6 +248,7 @@
243249 * @return int
244250 */
245251 static function decodeBPP( $data ) {
 252+ if ( strlen( $data ) < 1 ) throw new Exception( "Ran out of input" );
246253 $buf = unpack( 'C', $data );
247254 $buf = $buf[1];
248255 $bpp = ( $buf & 7 ) + 1;
@@ -259,6 +266,7 @@
260267 static function skipBlock( $fh ) {
261268 while ( !feof( $fh ) ) {
262269 $buf = fread( $fh, 1 );
 270+ if ( strlen( $buf ) < 1 ) throw new Exception( "Ran out of input" );
263271 $block_len = unpack( 'C', $buf );
264272 $block_len = $block_len[1];
265273 if ($block_len == 0) {
Index: branches/REL1_18/phase3/includes/media/BMP.php
@@ -42,7 +42,7 @@
4343 * @return array
4444 */
4545 function getImageSize( $image, $filename ) {
46 - $f = fopen( $filename, 'r' );
 46+ $f = fopen( $filename, 'rb' );
4747 if( !$f ) {
4848 return false;
4949 }
@@ -54,8 +54,12 @@
5555 $h = substr( $header, 22, 4);
5656
5757 // Convert the unsigned long 32 bits (little endian):
58 - $w = unpack( 'V' , $w );
59 - $h = unpack( 'V' , $h );
 58+ try {
 59+ $w = wfUnpack( 'V', $w, 4 );
 60+ $h = wfUnpack( 'V', $h, 4 );
 61+ } catch ( MWException $e ) {
 62+ return false;
 63+ }
6064 return array( $w[1], $h[1] );
6165 }
6266 }
Index: branches/REL1_18/phase3/includes/media/PNGMetadataExtractor.php
@@ -66,7 +66,7 @@
6767 throw new Exception( __METHOD__ . ": File $filename does not exist" );
6868 }
6969
70 - $fh = fopen( $filename, 'r' );
 70+ $fh = fopen( $filename, 'rb' );
7171
7272 if ( !$fh ) {
7373 throw new Exception( __METHOD__ . ": Unable to open file $filename" );
@@ -81,20 +81,24 @@
8282 // Read chunks
8383 while ( !feof( $fh ) ) {
8484 $buf = fread( $fh, 4 );
85 - if ( !$buf ) {
 85+ if ( !$buf || strlen( $buf ) < 4 ) {
8686 throw new Exception( __METHOD__ . ": Read error" );
8787 }
8888 $chunk_size = unpack( "N", $buf );
8989 $chunk_size = $chunk_size[1];
9090
 91+ if ( $chunk_size < 0 ) {
 92+ throw new Exception( __METHOD__ . ": Chunk size too big for unpack" );
 93+ }
 94+
9195 $chunk_type = fread( $fh, 4 );
92 - if ( !$chunk_type ) {
 96+ if ( !$chunk_type || strlen( $chunk_type ) < 4 ) {
9397 throw new Exception( __METHOD__ . ": Read error" );
9498 }
9599
96100 if ( $chunk_type == "IHDR" ) {
97101 $buf = self::read( $fh, $chunk_size );
98 - if ( !$buf ) {
 102+ if ( !$buf || strlen( $buf ) < $chunk_size ) {
99103 throw new Exception( __METHOD__ . ": Read error" );
100104 }
101105 $bitDepth = ord( substr( $buf, 8, 1 ) );
@@ -122,7 +126,7 @@
123127 }
124128 } elseif ( $chunk_type == "acTL" ) {
125129 $buf = fread( $fh, $chunk_size );
126 - if( !$buf ) {
 130+ if( !$buf || strlen( $buf ) < $chunk_size || $chunk_size < 4 ) {
127131 throw new Exception( __METHOD__ . ": Read error" );
128132 }
129133
@@ -131,10 +135,13 @@
132136 $loopCount = $actl['plays'];
133137 } elseif ( $chunk_type == "fcTL" ) {
134138 $buf = self::read( $fh, $chunk_size );
135 - if ( !$buf ) {
 139+ if ( !$buf || strlen( $buf ) < $chunk_size ) {
136140 throw new Exception( __METHOD__ . ": Read error" );
137141 }
138142 $buf = substr( $buf, 20 );
 143+ if ( strlen( $buf ) < 4 ) {
 144+ throw new Exception( __METHOD__ . ": Read error" );
 145+ }
139146
140147 $fctldur = unpack( "ndelay_num/ndelay_den", $buf );
141148 if ( $fctldur['delay_den'] == 0 ) {
@@ -294,7 +301,7 @@
295302 throw new Exception( __METHOD__ . ": tIME wrong size" );
296303 }
297304 $buf = self::read( $fh, $chunk_size );
298 - if ( !$buf ) {
 305+ if ( !$buf || strlen( $buf ) < $chunk_size ) {
299306 throw new Exception( __METHOD__ . ": Read error" );
300307 }
301308
@@ -317,20 +324,24 @@
318325 }
319326
320327 $buf = self::read( $fh, $chunk_size );
321 - if ( !$buf ) {
 328+ if ( !$buf || strlen( $buf ) < $chunk_size ) {
322329 throw new Exception( __METHOD__ . ": Read error" );
323330 }
324331
325332 $dim = unpack( "Nwidth/Nheight/Cunit", $buf );
326333 if ( $dim['unit'] == 1 ) {
327 - // unit is meters
328 - // (as opposed to 0 = undefined )
329 - $text['XResolution'] = $dim['width']
330 - . '/100';
331 - $text['YResolution'] = $dim['height']
332 - . '/100';
333 - $text['ResolutionUnit'] = 3;
334 - // 3 = dots per cm (from Exif).
 334+ // Need to check for negative because php
 335+ // doesn't deal with super-large unsigned 32-bit ints well
 336+ if ( $dim['width'] > 0 && $dim['height'] > 0 ) {
 337+ // unit is meters
 338+ // (as opposed to 0 = undefined )
 339+ $text['XResolution'] = $dim['width']
 340+ . '/100';
 341+ $text['YResolution'] = $dim['height']
 342+ . '/100';
 343+ $text['ResolutionUnit'] = 3;
 344+ // 3 = dots per cm (from Exif).
 345+ }
335346 }
336347
337348 } elseif ( $chunk_type == "IEND" ) {
Index: branches/REL1_18/phase3/includes/media/JpegMetadataExtractor.php
@@ -125,7 +125,7 @@
126126 return $segments;
127127 } else {
128128 // segment we don't care about, so skip
129 - $size = unpack( "nint", fread( $fh, 2 ) );
 129+ $size = wfUnpack( "nint", fread( $fh, 2 ), 2 );
130130 if ( $size['int'] <= 2 ) throw new MWException( "invalid marker size in jpeg" );
131131 fseek( $fh, $size['int'] - 2, SEEK_CUR );
132132 }
@@ -141,9 +141,11 @@
142142 * @return data content of segment.
143143 */
144144 private static function jpegExtractMarker( &$fh ) {
145 - $size = unpack( "nint", fread( $fh, 2 ) );
 145+ $size = wfUnpack( "nint", fread( $fh, 2 ), 2 );
146146 if ( $size['int'] <= 2 ) throw new MWException( "invalid marker size in jpeg" );
147 - return fread( $fh, $size['int'] - 2 );
 147+ $segment = fread( $fh, $size['int'] - 2 );
 148+ if ( strlen( $segment ) !== $size['int'] - 2 ) throw new MWException( "Segment shorter than expected" );
 149+ return $segment;
148150 }
149151
150152 /**
@@ -202,7 +204,12 @@
203205 $offset += $lenName;
204206
205207 // now length of data (unsigned long big endian)
206 - $lenData = unpack( 'Nlen', substr( $app13, $offset, 4 ) );
 208+ $lenData = wfUnpack( 'Nlen', substr( $app13, $offset, 4 ), 4 );
 209+ // PHP can take issue with very large unsigned ints and make them negative.
 210+ // Which should never ever happen, as this has to be inside a segment
 211+ // which is limited to a 16 bit number.
 212+ if ( $lenData['len'] < 0 ) throw new MWException( "Too big PSIR (" . $lenData['len'] . ')' );
 213+
207214 $offset += 4; // 4bytes length field;
208215
209216 // this should not happen, but check.
Index: branches/REL1_18/phase3/includes/ChangesList.php
@@ -1133,7 +1133,7 @@
11341134 } else {
11351135 $r .= $this->recentChangesFlags( array(
11361136 'newpage' => $type == RC_NEW,
1137 - 'mino' => $rcObj->mAttribs['rc_minor'],
 1137+ 'minor' => $rcObj->mAttribs['rc_minor'],
11381138 'unpatrolled' => $rcObj->unpatrolled,
11391139 'bot' => $rcObj->mAttribs['rc_bot'],
11401140 ) );
Property changes on: branches/REL1_18/phase3/includes/ChangesList.php
___________________________________________________________________
Modified: svn:mergeinfo
11411141 Merged /trunk/phase3/includes/ChangesList.php:r100510,100572,100592
Index: branches/REL1_18/phase3/includes/specials/SpecialUpload.php
@@ -342,7 +342,7 @@
343343 */
344344 protected function showRecoverableUploadError( $message ) {
345345 $sessionKey = $this->mUpload->stashSession();
346 - $message = '<h2>' . wfMsgHtml( 'uploadwarning' ) . "</h2>\n" .
 346+ $message = '<h2>' . wfMsgHtml( 'uploaderror' ) . "</h2>\n" .
347347 '<div class="error">' . $message . "</div>\n";
348348
349349 $form = $this->getUploadForm( $message, $sessionKey );
Property changes on: branches/REL1_18/phase3/includes/specials
___________________________________________________________________
Modified: svn:mergeinfo
350350 Merged /trunk/phase3/includes/specials:r99700,100239,100242,100347,100510,100572,100592
Property changes on: branches/REL1_18/phase3/includes
___________________________________________________________________
Modified: svn:mergeinfo
351351 Merged /trunk/phase3/includes:r98997,99118,99370,99700,100239,100242,100347,100510,100572,100592
Property changes on: branches/REL1_18/phase3
___________________________________________________________________
Modified: svn:mergeinfo
352352 Merged /trunk/phase3:r98997,99118,99370,99700,100239,100242,100347,100510,100572,100592

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r98997Fix bug in r83814 reported on IRC: categorymembers did not set an ORDER BY wh...catrope13:15, 5 October 2011
r99118Follow-up r68645: back out --email, still not usedmaxsem17:00, 6 October 2011
r99370Followup r98212; Hook documentation.dantman19:42, 9 October 2011
r99700Fixes bug 31496. Patch by Tomer A. I tested it by trying to upload a file wit...amire8016:48, 13 October 2011
r100239Follow-up r82285: we should not apply case conversion to these names, because...happy-melon17:09, 19 October 2011
r100242* Fixed undefined var $g error...aaron17:30, 19 October 2011
r100347(bug 31345) document API Revisions parameters as enum...hashar14:28, 20 October 2011
r100510Follow up to r79085. Fixes bug 31408....hartman19:50, 22 October 2011
r100572(bug 31740) JpegMetadataExtractor and friends weren't checking for unexpected...bawolff02:19, 24 October 2011
r100592test {{SERVERNAME}} with relative URLS...hashar09:32, 24 October 2011

Status & tagging log