From 3cd02f7a38ac093a8160db3ac666a1e096a5865c Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 9 Jan 2012 00:20:28 +0000 Subject: [PATCH] * r107986: Added readOnly checks to prepare(), secure(), and clean() in FileBackendBase. * Added some prepare()/clean() tests. --- includes/filerepo/backend/FSFileBackend.php | 34 ++++----- includes/filerepo/backend/FileBackend.php | 72 +++++++++++++------ .../backend/FileBackendMultiWrite.php | 18 ++--- .../includes/filerepo/FileBackendTest.php | 48 ++++++++++++- 4 files changed, 123 insertions(+), 49 deletions(-) diff --git a/includes/filerepo/backend/FSFileBackend.php b/includes/filerepo/backend/FSFileBackend.php index 7c1cca4ed0..a0caebea8d 100644 --- a/includes/filerepo/backend/FSFileBackend.php +++ b/includes/filerepo/backend/FSFileBackend.php @@ -274,9 +274,9 @@ class FSFileBackend extends FileBackend { } /** - * @see FileBackend::doPrepare() + * @see FileBackend::doPrepareInternal() */ - protected function doPrepare( $container, $dir, array $params ) { + protected function doPrepareInternal( $container, $dir, array $params ) { $status = Status::newGood(); if ( !wfMkdirParents( $dir ) ) { $status->fatal( 'directorycreateerror', $params['dir'] ); @@ -289,9 +289,9 @@ class FSFileBackend extends FileBackend { } /** - * @see FileBackend::doSecure() + * @see FileBackend::doSecureInternal() */ - protected function doSecure( $container, $dir, array $params ) { + protected function doSecureInternal( $container, $dir, array $params ) { $status = Status::newGood(); if ( !wfMkdirParents( $dir ) ) { $status->fatal( 'directorycreateerror', $params['dir'] ); @@ -308,25 +308,27 @@ class FSFileBackend extends FileBackend { } } // Add a .htaccess file to the root of the container... - list( $b, $container, $r ) = FileBackend::splitStoragePath( $params['dir'] ); - $dirRoot = $this->containerPaths[$container]; // real path - if ( !empty( $params['noAccess'] ) && !file_exists( "{$dirRoot}/.htaccess" ) ) { - wfSuppressWarnings(); - $ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" ); - wfRestoreWarnings(); - if ( !$ok ) { - $storeDir = "mwstore://{$this->name}/{$container}"; - $status->fatal( 'backend-fail-create', "$storeDir/.htaccess" ); - return $status; + if ( !empty( $params['noAccess'] ) ) { + list( $b, $container, $r ) = FileBackend::splitStoragePath( $params['dir'] ); + $dirRoot = $this->containerPaths[$container]; // real path + if ( !file_exists( "{$dirRoot}/.htaccess" ) ) { + wfSuppressWarnings(); + $ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" ); + wfRestoreWarnings(); + if ( !$ok ) { + $storeDir = "mwstore://{$this->name}/{$container}"; + $status->fatal( 'backend-fail-create', "$storeDir/.htaccess" ); + return $status; + } } } return $status; } /** - * @see FileBackend::doClean() + * @see FileBackend::doCleanInternal() */ - protected function doClean( $container, $dir, array $params ) { + protected function doCleanInternal( $container, $dir, array $params ) { $status = Status::newGood(); wfSuppressWarnings(); if ( is_dir( $dir ) ) { diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 5befbe0c38..fefb2c9d84 100644 --- a/includes/filerepo/backend/FileBackend.php +++ b/includes/filerepo/backend/FileBackend.php @@ -292,7 +292,17 @@ abstract class FileBackendBase { * @param $params Array * @return Status */ - abstract public function prepare( array $params ); + final public function prepare( array $params ) { + if ( $this->readOnly != '' ) { + return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly ); + } + return $this->doPrepare( $params ); + } + + /** + * @see FileBackendBase::prepare() + */ + abstract protected function doPrepare( array $params ); /** * Take measures to block web access to a directory and @@ -309,7 +319,17 @@ abstract class FileBackendBase { * @param $params Array * @return Status */ - abstract public function secure( array $params ); + final public function secure( array $params ) { + if ( $this->readOnly != '' ) { + return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly ); + } + return $this->doSecure( $params ); + } + + /** + * @see FileBackendBase::secure() + */ + abstract protected function doSecure( array $params ); /** * Clean up an empty storage directory. @@ -321,7 +341,17 @@ abstract class FileBackendBase { * @param $params Array * @return Status */ - abstract public function clean( array $params ); + final public function clean( array $params ) { + if ( $this->readOnly != '' ) { + return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly ); + } + return $this->doClean( $params ); + } + + /** + * @see FileBackendBase::clean() + */ + abstract protected function doClean( array $params ); /** * Check if a file exists at a storage path in the backend. @@ -747,9 +777,9 @@ abstract class FileBackend extends FileBackendBase { } /** - * @see FileBackendBase::prepare() + * @see FileBackendBase::doPrepare() */ - final public function prepare( array $params ) { + final protected function doPrepare( array $params ) { $status = Status::newGood(); list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] ); if ( $dir === null ) { @@ -757,28 +787,28 @@ abstract class FileBackend extends FileBackendBase { return $status; // invalid storage path } if ( $shard !== null ) { // confined to a single container/shard - $status->merge( $this->doPrepare( $fullCont, $dir, $params ) ); + $status->merge( $this->doPrepareInternal( $fullCont, $dir, $params ) ); } else { // directory is on several shards wfDebug( __METHOD__ . ": iterating over all container shards.\n" ); list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] ); foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) { - $status->merge( $this->doPrepare( "{$fullCont}{$suffix}", $dir, $params ) ); + $status->merge( $this->doPrepareInternal( "{$fullCont}{$suffix}", $dir, $params ) ); } } return $status; } /** - * @see FileBackend::prepare() + * @see FileBackend::doPrepare() */ - protected function doPrepare( $container, $dir, array $params ) { + protected function doPrepareInternal( $container, $dir, array $params ) { return Status::newGood(); } /** - * @see FileBackendBase::secure() + * @see FileBackendBase::doSecure() */ - final public function secure( array $params ) { + final protected function doSecure( array $params ) { $status = Status::newGood(); list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] ); if ( $dir === null ) { @@ -786,28 +816,28 @@ abstract class FileBackend extends FileBackendBase { return $status; // invalid storage path } if ( $shard !== null ) { // confined to a single container/shard - $status->merge( $this->doSecure( $fullCont, $dir, $params ) ); + $status->merge( $this->doSecureInternal( $fullCont, $dir, $params ) ); } else { // directory is on several shards wfDebug( __METHOD__ . ": iterating over all container shards.\n" ); list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] ); foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) { - $status->merge( $this->doSecure( "{$fullCont}{$suffix}", $dir, $params ) ); + $status->merge( $this->doSecureInternal( "{$fullCont}{$suffix}", $dir, $params ) ); } } return $status; } /** - * @see FileBackend::secure() + * @see FileBackend::doSecure() */ - protected function doSecure( $container, $dir, array $params ) { + protected function doSecureInternal( $container, $dir, array $params ) { return Status::newGood(); } /** - * @see FileBackendBase::clean() + * @see FileBackendBase::doClean() */ - final public function clean( array $params ) { + final protected function doClean( array $params ) { $status = Status::newGood(); list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] ); if ( $dir === null ) { @@ -815,21 +845,21 @@ abstract class FileBackend extends FileBackendBase { return $status; // invalid storage path } if ( $shard !== null ) { // confined to a single container/shard - $status->merge( $this->doClean( $fullCont, $dir, $params ) ); + $status->merge( $this->doCleanInternal( $fullCont, $dir, $params ) ); } else { // directory is on several shards wfDebug( __METHOD__ . ": iterating over all container shards.\n" ); list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] ); foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) { - $status->merge( $this->doClean( "{$fullCont}{$suffix}", $dir, $params ) ); + $status->merge( $this->doCleanInternal( "{$fullCont}{$suffix}", $dir, $params ) ); } } return $status; } /** - * @see FileBackend::clean() + * @see FileBackend::doClean() */ - protected function doClean( $container, $dir, array $params ) { + protected function doCleanInternal( $container, $dir, array $params ) { return Status::newGood(); } diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php index b71b631030..4258d13543 100644 --- a/includes/filerepo/backend/FileBackendMultiWrite.php +++ b/includes/filerepo/backend/FileBackendMultiWrite.php @@ -251,37 +251,37 @@ class FileBackendMultiWrite extends FileBackendBase { } /** - * @see FileBackendBase::prepare() + * @see FileBackendBase::doPrepare() */ - public function prepare( array $params ) { + public function doPrepare( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->prepare( $realParams ) ); + $status->merge( $backend->doPrepare( $realParams ) ); } return $status; } /** - * @see FileBackendBase::secure() + * @see FileBackendBase::doSecure() */ - public function secure( array $params ) { + public function doSecure( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->secure( $realParams ) ); + $status->merge( $backend->doSecure( $realParams ) ); } return $status; } /** - * @see FileBackendBase::clean() + * @see FileBackendBase::doClean() */ - public function clean( array $params ) { + public function doClean( array $params ) { $status = Status::newGood(); foreach ( $this->backends as $backend ) { $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->clean( $realParams ) ); + $status->merge( $backend->doClean( $realParams ) ); } return $status; } diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index c9f5f3a90a..df9651d78b 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -690,11 +690,53 @@ class FileBackendTest extends MediaWikiTestCase { return $cases; } - // @TODO: testPrepare + /** + * @dataProvider provider_testPrepareAndClean + */ + public function testPrepareAndClean( $path, $isOK ) { + $this->backend = $this->singleBackend; + $this->doTestPrepareAndClean( $path, $isOK ); - // @TODO: testSecure + $this->backend = $this->multiBackend; + $this->doTestPrepareAndClean( $path, $isOK ); + } + + function provider_testPrepareAndClean() { + $base = $this->baseStorePath(); + return array( + array( "$base/cont1/a/z/some_file1.txt", true ), + array( "$base/cont2/a/z/some_file2.txt", true ), + array( "$base/cont3/a/z/some_file3.txt", false ), + ); + } + + function doTestPrepareAndClean( $path, $isOK ) { + $backendName = $this->backendClass(); + + $status = $this->backend->prepare( array( 'dir' => $path ) ); + if ( $isOK ) { + $this->assertEquals( array(), $status->errors, + "Preparing dir $path succeeded without warnings ($backendName)." ); + $this->assertEquals( true, $status->isOK(), + "Preparing dir $path succeeded ($backendName)." ); + } else { + $this->assertEquals( false, $status->isOK(), + "Preparing dir $path failed ($backendName)." ); + } - // @TODO: testClean + $status = $this->backend->clean( array( 'dir' => $path ) ); + if ( $isOK ) { + $this->assertEquals( array(), $status->errors, + "Cleaning dir $path succeeded without warnings ($backendName)." ); + $this->assertEquals( true, $status->isOK(), + "Cleaning dir $path succeeded ($backendName)." ); + } else { + $this->assertEquals( false, $status->isOK(), + "Cleaning dir $path failed ($backendName)." ); + } + } + + // @TODO: testSecure // @TODO: testDoOperations -- 2.20.1