From 95f31706ee451a773c8f0b81ef26ab3c25855453 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 19 Jan 2012 02:07:48 +0000 Subject: [PATCH] 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. --- includes/AutoLoader.php | 1 + includes/filerepo/backend/FileBackend.php | 29 +++++++++-- includes/filerepo/backend/FileOp.php | 62 +++++++++++++++++++++++ languages/messages/MessagesEn.php | 1 + maintenance/language/messages.inc | 3 +- 5 files changed, 91 insertions(+), 5 deletions(-) diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index c7d4632cbd..85158857cd 100644 --- 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 ded152bc4d..9fc15b50fc 100644 --- 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 cd20097f2a..4a5e0c7154 100644 --- 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 8670658f54..aad39feb14 100644 --- 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 5dfacc62be..02fe7aebd3 100644 --- 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( -- 2.20.1