MediaWiki r110871 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r110870‎ | r110871 (on ViewVC)‎ | r110872 >
Date:20:47, 7 February 2012
Author:hashar
Status:resolved (Comments)
Tags:
Comment:
simplify FileCacheBase::fetchText

Implements a proposal by Tim on r98405 CR
Modified paths:

Diff [purge]

Index: trunk/phase3/includes/cache/FileCacheBase.php
——@@ -116,12 +116,9 @@
117117 * @return string
118118 */
119119 public function fetchText() {
120 - if ( $this->useGzip() ) {
121 - /* Why is there no gzfile_get_contents() or gzdecode()? */
122 - return implode( '', gzfile( $this->cachePath() ) );
123 - } else {
124 - return file_get_contents( $this->cachePath() );
125 - }
 120+ // gzopen can transparently read from gziped or plain text
 121+ $fh = gzopen( $this->cachePath(), 'r' );
 122+ return stream_get_contents( $fh );
126123 }
127124
128125 /**
——@@ -141,6 +138,7 @@
142139
143140 $this->checkCacheDirs(); // build parent dir
144141 if ( !file_put_contents( $this->cachePath(), $text, LOCK_EX ) ) {
 142+ wfDebug( __METHOD__ . "() failed saving ". $this->cachePath() . "\n");
145143 $this->mCached = null;
146144 return false;
147145 }

Follow-up revisions

Rev.Commit summaryAuthorDate
r110916make gzopen() portable by using binary flag...hashar07:03, 8 February 2012
r111324Fix some injection from r110871hashar19:51, 12 February 2012

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r98405HTMLFileCache refactoring:...aaron08:18, 29 September 2011

Comments

#Comment by Aaron Schulz (Talk | contribs)   21:26, 7 February 2012

Can you add the 'b' flag to gzopen() for good measure? ("$fh = gzopen( $this->cachePath(), 'rb' );")

#Comment by Hashar (Talk | contribs)   07:03, 8 February 2012

binary flag added with r110916.

#Comment by Tim Starling (Talk | contribs)   01:55, 10 February 2012

Calling gzopen() on a plain text file seems rather scary to me, since with the resource loader, the user-supplied input can start from the first character, and there's no obvious reason why a malicious user couldn't supply a string starting with the GZIP magic signature.

#Comment by Hashar (Talk | contribs)   19:51, 12 February 2012

I have put back the file_get_contents() with r111324

Status & tagging log

Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/110871"
Personal tools
Namespaces

Variants
Views
Actions
Navigation
Support
Download
Development
Communication
Toolbox