In FileBackend/FileOp:

* Added a sane default max file size to FileBackend. Operation batches need to check this before trying anything.
* Temporarily adjust the PHP execution time limit in attemptBatch() to reduce the chance of dying in the middle of it. Also added a maximum batch size limit.
* Added some code comments.

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index c7d4632..8515885 100644 (file)
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -511,6 +511,7 @@ $wgAutoloadLocalClasses = array(
        'MySqlLockManager'=> 'includes/filerepo/backend/lockmanager/DBLockManager.php',
        'NullLockManager' => 'includes/filerepo/backend/lockmanager/LockManager.php',
        'FileOp' => 'includes/filerepo/backend/FileOp.php',
+       'FileOpScopedPHPTimeout' => 'includes/filerepo/backend/FileOp.php',
        'StoreFileOp' => 'includes/filerepo/backend/FileOp.php',
        'CopyFileOp' => 'includes/filerepo/backend/FileOp.php',
        'MoveFileOp' => 'includes/filerepo/backend/FileOp.php',

diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php
index ded152b..9fc15b5 100644 (file)
--- a/includes/filerepo/backend/FileBackend.php
+++ b/includes/filerepo/backend/FileBackend.php
@@ -592,6 +592,19 @@ abstract class FileBackend extends FileBackendBase {
        /** @var Array */
        protected $shardViaHashLevels = array(); // (container name => integer)
 
+       protected $maxFileSize = 1000000000; // integer bytes (1GB)
+
+       /**
+        * Get the maximum allowable file size given backend
+        * medium restrictions and basic performance constraints.
+        * Do not call this function from places outside FileBackend and FileOp.
+        * 
+        * @return integer Bytes 
+        */
+       final public function maxFileSizeInternal() {
+               return $this->maxFileSize;
+       }
+
        /**
         * Create a file in the backend with the given contents.
         * Do not call this function from places outside FileBackend and FileOp.
@@ -605,8 +618,12 @@ abstract class FileBackend extends FileBackendBase {
         * @return Status
         */
        final public function createInternal( array $params ) {
-               $status = $this->doCreateInternal( $params );
-               $this->clearCache( array( $params['dst'] ) );
+               if ( strlen( $params['content'] ) > $this->maxFileSizeInternal() ) {
+                       $status = Status::newFatal( 'backend-fail-create', $params['dst'] );
+               } else {
+                       $status = $this->doCreateInternal( $params );
+                       $this->clearCache( array( $params['dst'] ) );
+               }
                return $status;
        }
 
@@ -628,8 +645,12 @@ abstract class FileBackend extends FileBackendBase {
         * @return Status
         */
        final public function storeInternal( array $params ) {
-               $status = $this->doStoreInternal( $params );
-               $this->clearCache( array( $params['dst'] ) );
+               if ( filesize( $params['src'] ) > $this->maxFileSizeInternal() ) {
+                       $status = Status::newFatal( 'backend-fail-store', $params['dst'] );
+               } else {
+                       $status = $this->doStoreInternal( $params );
+                       $this->clearCache( array( $params['dst'] ) );
+               }
                return $status;
        }
 

diff --git a/includes/filerepo/backend/FileOp.php b/includes/filerepo/backend/FileOp.php
index cd20097..4a5e0c7 100644 (file)
--- a/includes/filerepo/backend/FileOp.php
+++ b/includes/filerepo/backend/FileOp.php
@@ -34,6 +34,10 @@ abstract class FileOp {
        const STATE_CHECKED = 2;
        const STATE_ATTEMPTED = 3;
 
+       /* Timeout related parameters */
+       const MAX_BATCH_SIZE = 1000;
+       const TIME_LIMIT_SEC = 300; // 5 minutes
+
        /**
         * Build a new file operation transaction
         *
@@ -52,6 +56,10 @@ abstract class FileOp {
 
        /**
         * Allow stale data for file reads and existence checks.
+        * 
+        * Note that we don't want to mix stale and non-stale reads
+        * because stat calls are cached: if we read X without 'latest'
+        * and then read it with 'latest', the data may still be stale.
         *
         * @return void
         */
@@ -80,6 +88,12 @@ abstract class FileOp {
                $allowStale = !empty( $opts['allowStale'] );
                $ignoreErrors = !empty( $opts['force'] );
 
+               $n = count( $performOps );
+               if ( $n > self::MAX_BATCH_SIZE ) {
+                       $status->fatal( 'backend-fail-batchsize', $n, self::MAX_BATCH_SIZE );
+                       return $status;
+               }
+
                $predicates = FileOp::newPredicates(); // account for previous op in prechecks
                // Do pre-checks for each operation; abort on failure...
                foreach ( $performOps as $index => $fileOp ) {
@@ -97,6 +111,11 @@ abstract class FileOp {
                        }
                }
 
+               // Restart PHP's execution timer and set the timeout to safe amount.
+               // This handles cases where the operations take a long time or where we are
+               // already running low on time left. The old timeout is restored afterwards.
+               $scopedTimeLimit = new FileOpScopedPHPTimeout( self::TIME_LIMIT_SEC );
+
                // Attempt each operation...
                foreach ( $performOps as $index => $fileOp ) {
                        if ( $fileOp->failed() ) {
@@ -325,6 +344,39 @@ abstract class FileOp {
        }
 }
 
+/**
+ * FileOp helper class to expand PHP execution time for a function.
+ * On construction, set_time_limit() is called and set to $seconds.
+ * When the object goes out of scope, the timer is restarted, with
+ * the original time limit minus the time the object existed.
+ */
+class FileOpScopedPHPTimeout {
+       protected $startTime; // integer seconds
+       protected $oldTimeout; // integer seconds
+
+       /**
+        * @param $seconds integer
+        */
+       public function __construct( $seconds ) {
+               if ( ini_get( 'max_execution_time' ) > 0 ) { // CLI uses 0
+                       $this->oldTimeout = ini_set( 'max_execution_time', $seconds );
+               }
+               $this->startTime = time();
+       }
+
+       /*
+        * Restore the original timeout.
+        * This does not account for the timer value on __construct().
+        */
+       public function __destruct() {
+               if ( $this->oldTimeout ) {
+                       $elapsed = time() - $this->startTime;
+                       // Note: a limit of 0 is treated as "forever"
+                       set_time_limit( max( 1, $this->oldTimeout - $elapsed ) );
+               }
+       }
+}
+
 /**
  * Store a file into the backend from a file on the file system.
  * Parameters similar to FileBackend::storeInternal(), which include:
@@ -345,6 +397,11 @@ class StoreFileOp extends FileOp {
                        $status->fatal( 'backend-fail-notexists', $this->params['src'] );
                        return $status;
                }
+               // Check if the source file is too big
+               if ( filesize( $this->params['src'] ) > $this->backend->maxFileSizeInternal() ) {
+                       $status->fatal( 'backend-fail-store', $this->params['dst'] );
+                       return $status;
+               }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
                if ( !$status->isOK() ) {
@@ -395,6 +452,11 @@ class CreateFileOp extends FileOp {
 
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
+               // Check if the source data is too big
+               if ( strlen( $this->params['content'] ) > $this->backend->maxFileSizeInternal() ) {
+                       $status->fatal( 'backend-fail-create', $this->params['dst'] );
+                       return $status;
+               }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
                if ( !$status->isOK() ) {

diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index 8670658..aad39fe 100644 (file)
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2258,6 +2258,7 @@ If the problem persists, contact an [[Special:ListUsers/sysop|administrator]].',
 'backend-fail-connect'       => 'Could not connect to file backend "$1".',
 'backend-fail-internal'      => 'An unknown error occurred in file backend "$1".',
 'backend-fail-contenttype'   => 'Could not determine the content type of file to store at "$1".',
+'backend-fail-batchsize'     => 'Backend given a batch of $1 file {{PLURAL:$1|operation|operations}}; the limit is $2.',
 
 # Lock manager
 'lockmanager-notlocked'        => 'Could not unlock "$1"; it is not locked.',

diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc
index 5dfacc6..02fe7ae 100644 (file)
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -1367,7 +1367,8 @@ $wgMessageStructure = array(
                'backend-fail-synced',
                'backend-fail-connect',
                'backend-fail-internal',
-               'backend-fail-contenttype'
+               'backend-fail-contenttype',
+               'backend-fail-batchsize'
        ),
 
        'lockmanager-errors' => array(