filebackend: remove unneeded is_dir() call in FSFileBackend::doCleanInternal()
[lhc/web/wiklou.git] / includes / libs / filebackend / FSFileBackend.php
index c333a5e..9ed7ae3 100644 (file)
@@ -40,6 +40,7 @@
  * @file
  * @ingroup FileBackend
  */
+use Wikimedia\AtEase\AtEase;
 use Wikimedia\Timestamp\ConvertibleTimestamp;
 
 /**
@@ -63,23 +64,22 @@ class FSFileBackend extends FileBackendStore {
        protected $basePath;
 
        /** @var array Map of container names to root paths for custom container paths */
-       protected $containerPaths = [];
+       protected $containerPaths;
 
+       /** @var int Directory permission mode */
+       protected $dirMode;
        /** @var int File permission mode */
        protected $fileMode;
-       /** @var int File permission mode */
-       protected $dirMode;
-
        /** @var string Required OS username to own files */
        protected $fileOwner;
 
-       /** @var bool */
+       /** @var bool Whether the OS is Windows (otherwise assumed Unix-like)*/
        protected $isWindows;
        /** @var string OS username running this script */
        protected $currentUser;
 
-       /** @var array */
-       protected $hadWarningErrors = [];
+       /** @var bool[] Map of (stack index => whether a warning happened) */
+       private $warningTrapStack = [];
 
        /**
         * @see FileBackendStore::__construct()
@@ -102,11 +102,9 @@ class FSFileBackend extends FileBackendStore {
                        $this->basePath = null; // none; containers must have explicit paths
                }
 
-               if ( isset( $config['containerPaths'] ) ) {
-                       $this->containerPaths = (array)$config['containerPaths'];
-                       foreach ( $this->containerPaths as &$path ) {
-                               $path = rtrim( $path, '/' ); // remove trailing slash
-                       }
+               $this->containerPaths = [];
+               foreach ( ( $config['containerPaths'] ?? [] ) as $container => $path ) {
+                       $this->containerPaths[$container] = rtrim( $path, '/' ); // remove trailing slash
                }
 
                $this->fileMode = $config['fileMode'] ?? 0644;
@@ -124,7 +122,7 @@ class FSFileBackend extends FileBackendStore {
                        // See https://www.php.net/manual/en/migration71.windows-support.php
                        return 0;
                } else {
-                       return FileBackend::ATTR_UNICODE_PATHS;
+                       return self::ATTR_UNICODE_PATHS;
                }
        }
 
@@ -228,20 +226,12 @@ class FSFileBackend extends FileBackendStore {
                }
 
                if ( !empty( $params['async'] ) ) { // deferred
-                       $tempFile = $this->tmpFileFactory->newTempFSFile( 'create_', 'tmp' );
+                       $tempFile = $this->stageContentAsTempFile( $params );
                        if ( !$tempFile ) {
                                $status->fatal( 'backend-fail-create', $params['dst'] );
 
                                return $status;
                        }
-                       $this->trapWarnings();
-                       $bytes = file_put_contents( $tempFile->getPath(), $params['content'] );
-                       $this->untrapWarnings();
-                       if ( $bytes === false ) {
-                               $status->fatal( 'backend-fail-create', $params['dst'] );
-
-                               return $status;
-                       }
                        $cmd = implode( ' ', [
                                $this->isWindows ? 'COPY /B /Y' : 'cp', // (binary, overwrite)
                                escapeshellarg( $this->cleanPathSlashes( $tempFile->getPath() ) ),
@@ -376,34 +366,38 @@ class FSFileBackend extends FileBackendStore {
        protected function doMoveInternal( array $params ) {
                $status = $this->newStatus();
 
-               $source = $this->resolveToFSPath( $params['src'] );
-               if ( $source === null ) {
+               $fsSrcPath = $this->resolveToFSPath( $params['src'] );
+               if ( $fsSrcPath === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['src'] );
 
                        return $status;
                }
 
-               $dest = $this->resolveToFSPath( $params['dst'] );
-               if ( $dest === null ) {
+               $fsDstPath = $this->resolveToFSPath( $params['dst'] );
+               if ( $fsDstPath === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['dst'] );
 
                        return $status;
                }
 
-               if ( !is_file( $source ) ) {
-                       if ( empty( $params['ignoreMissingSource'] ) ) {
-                               $status->fatal( 'backend-fail-move', $params['src'] );
-                       }
-
-                       return $status; // do nothing; either OK or bad status
+               if ( $fsSrcPath === $fsDstPath ) {
+                       return $status; // no-op
                }
 
+               $ignoreMissing = !empty( $params['ignoreMissingSource'] );
+
                if ( !empty( $params['async'] ) ) { // deferred
-                       $cmd = implode( ' ', [
-                               $this->isWindows ? 'MOVE /Y' : 'mv', // (overwrite)
-                               escapeshellarg( $this->cleanPathSlashes( $source ) ),
-                               escapeshellarg( $this->cleanPathSlashes( $dest ) )
-                       ] );
+                       // https://manpages.debian.org/buster/coreutils/mv.1.en.html
+                       // https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/move
+                       $encSrc = escapeshellarg( $this->cleanPathSlashes( $fsSrcPath ) );
+                       $encDst = escapeshellarg( $this->cleanPathSlashes( $fsDstPath ) );
+                       if ( $this->isWindows ) {
+                               $writeCmd = "MOVE /Y $encSrc $encDst";
+                               $cmd = $ignoreMissing ? "IF EXIST $encSrc $writeCmd" : $writeCmd;
+                       } else {
+                               $writeCmd = "mv -f $encSrc $encDst";
+                               $cmd = $ignoreMissing ? "test -f $encSrc && $writeCmd" : $writeCmd;
+                       }
                        $handler = function ( $errors, StatusValue $status, array $params, $cmd ) {
                                if ( $errors !== '' && !( $this->isWindows && $errors[0] === " " ) ) {
                                        $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] );
@@ -412,11 +406,13 @@ class FSFileBackend extends FileBackendStore {
                        };
                        $status->value = new FSFileOpHandle( $this, $params, $handler, $cmd );
                } else { // immediate write
-                       $this->trapWarnings();
-                       $ok = ( $source === $dest ) ? true : rename( $source, $dest );
-                       $this->untrapWarnings();
-                       clearstatcache(); // file no longer at source
-                       if ( !$ok ) {
+                       // Use rename() here since (a) this clears xattrs, (b) any threads still reading the
+                       // old inode are unaffected since it writes to a new inode, and (c) this is fast and
+                       // atomic within a file system volume (as is normally the case)
+                       $this->trapWarnings( '/: No such file or directory$/' );
+                       $moved = rename( $fsSrcPath, $fsDstPath );
+                       $hadError = $this->untrapWarnings();
+                       if ( $hadError || ( !$moved && !$ignoreMissing ) ) {
                                $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] );
 
                                return $status;
@@ -429,26 +425,25 @@ class FSFileBackend extends FileBackendStore {
        protected function doDeleteInternal( array $params ) {
                $status = $this->newStatus();
 
-               $source = $this->resolveToFSPath( $params['src'] );
-               if ( $source === null ) {
+               $fsSrcPath = $this->resolveToFSPath( $params['src'] );
+               if ( $fsSrcPath === null ) {
                        $status->fatal( 'backend-fail-invalidpath', $params['src'] );
 
                        return $status;
                }
 
-               if ( !is_file( $source ) ) {
-                       if ( empty( $params['ignoreMissingSource'] ) ) {
-                               $status->fatal( 'backend-fail-delete', $params['src'] );
-                       }
-
-                       return $status; // do nothing; either OK or bad status
-               }
+               $ignoreMissing = !empty( $params['ignoreMissingSource'] );
 
                if ( !empty( $params['async'] ) ) { // deferred
-                       $cmd = implode( ' ', [
-                               $this->isWindows ? 'DEL' : 'unlink',
-                               escapeshellarg( $this->cleanPathSlashes( $source ) )
-                       ] );
+                       // https://manpages.debian.org/buster/coreutils/rm.1.en.html
+                       // https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/del
+                       $encSrc = escapeshellarg( $this->cleanPathSlashes( $fsSrcPath ) );
+                       if ( $this->isWindows ) {
+                               $writeCmd = "DEL /Q $encSrc";
+                               $cmd = $ignoreMissing ? "IF EXIST $encSrc $writeCmd" : $writeCmd;
+                       } else {
+                               $cmd = $ignoreMissing ? "rm -f $encSrc" : "rm $encSrc";
+                       }
                        $handler = function ( $errors, StatusValue $status, array $params, $cmd ) {
                                if ( $errors !== '' && !( $this->isWindows && $errors[0] === " " ) ) {
                                        $status->fatal( 'backend-fail-delete', $params['src'] );
@@ -457,10 +452,10 @@ class FSFileBackend extends FileBackendStore {
                        };
                        $status->value = new FSFileOpHandle( $this, $params, $handler, $cmd );
                } else { // immediate write
-                       $this->trapWarnings();
-                       $ok = unlink( $source );
-                       $this->untrapWarnings();
-                       if ( !$ok ) {
+                       $this->trapWarnings( '/: No such file or directory$/' );
+                       $deleted = unlink( $fsSrcPath );
+                       $hadError = $this->untrapWarnings();
+                       if ( $hadError || ( !$deleted && !$ignoreMissing ) ) {
                                $status->fatal( 'backend-fail-delete', $params['src'] );
 
                                return $status;
@@ -483,7 +478,7 @@ class FSFileBackend extends FileBackendStore {
                $dir = ( $dirRel != '' ) ? "{$contRoot}/{$dirRel}" : $contRoot;
                $existed = is_dir( $dir ); // already there?
                // Create the directory and its parents as needed...
-               $this->trapWarnings();
+               AtEase::suppressWarnings();
                if ( !$existed && !mkdir( $dir, $this->dirMode, true ) && !is_dir( $dir ) ) {
                        $this->logger->error( __METHOD__ . ": cannot create directory $dir" );
                        $status->fatal( 'directorycreateerror', $params['dir'] ); // fails on races
@@ -494,7 +489,7 @@ class FSFileBackend extends FileBackendStore {
                        $this->logger->error( __METHOD__ . ": directory $dir is not readable" );
                        $status->fatal( 'directorynotreadableerror', $params['dir'] );
                }
-               $this->untrapWarnings();
+               AtEase::restoreWarnings();
                // Respect any 'noAccess' or 'noListing' flags...
                if ( is_dir( $dir ) && !$existed ) {
                        $status->merge( $this->doSecureInternal( $fullCont, $dirRel, $params ) );
@@ -519,9 +514,9 @@ class FSFileBackend extends FileBackendStore {
                }
                // Add a .htaccess file to the root of the container...
                if ( !empty( $params['noAccess'] ) && !file_exists( "{$contRoot}/.htaccess" ) ) {
-                       $this->trapWarnings();
+                       AtEase::suppressWarnings();
                        $bytes = file_put_contents( "{$contRoot}/.htaccess", $this->htaccessPrivate() );
-                       $this->untrapWarnings();
+                       AtEase::restoreWarnings();
                        if ( $bytes === false ) {
                                $storeDir = "mwstore://{$this->name}/{$shortCont}";
                                $status->fatal( 'backend-fail-create', "{$storeDir}/.htaccess" );
@@ -539,21 +534,17 @@ class FSFileBackend extends FileBackendStore {
                // Unseed new directories with a blank index.html, to allow crawling...
                if ( !empty( $params['listing'] ) && is_file( "{$dir}/index.html" ) ) {
                        $exists = ( file_get_contents( "{$dir}/index.html" ) === $this->indexHtmlPrivate() );
-                       $this->trapWarnings();
-                       if ( $exists && !unlink( "{$dir}/index.html" ) ) { // reverse secure()
+                       if ( $exists && !$this->unlink( "{$dir}/index.html" ) ) { // reverse secure()
                                $status->fatal( 'backend-fail-delete', $params['dir'] . '/index.html' );
                        }
-                       $this->untrapWarnings();
                }
                // Remove the .htaccess file from the root of the container...
                if ( !empty( $params['access'] ) && is_file( "{$contRoot}/.htaccess" ) ) {
                        $exists = ( file_get_contents( "{$contRoot}/.htaccess" ) === $this->htaccessPrivate() );
-                       $this->trapWarnings();
-                       if ( $exists && !unlink( "{$contRoot}/.htaccess" ) ) { // reverse secure()
+                       if ( $exists && !$this->unlink( "{$contRoot}/.htaccess" ) ) { // reverse secure()
                                $storeDir = "mwstore://{$this->name}/{$shortCont}";
                                $status->fatal( 'backend-fail-delete', "{$storeDir}/.htaccess" );
                        }
-                       $this->untrapWarnings();
                }
 
                return $status;
@@ -564,11 +555,9 @@ class FSFileBackend extends FileBackendStore {
                list( , $shortCont, ) = FileBackend::splitStoragePath( $params['dir'] );
                $contRoot = $this->containerFSRoot( $shortCont, $fullCont ); // must be valid
                $dir = ( $dirRel != '' ) ? "{$contRoot}/{$dirRel}" : $contRoot;
-               $this->trapWarnings();
-               if ( is_dir( $dir ) ) {
-                       rmdir( $dir ); // remove directory if empty
-               }
-               $this->untrapWarnings();
+               AtEase::suppressWarnings();
+               rmdir( $dir ); // remove directory if empty
+               AtEase::restoreWarnings();
 
                return $status;
        }
@@ -724,7 +713,7 @@ class FSFileBackend extends FileBackendStore {
 
                        $tmpPath = $tmpFile->getPath();
                        // Copy the source file over the temp file
-                       $this->trapWarnings();
+                       $this->trapWarnings(); // don't trust 'false' if there were errors
                        $isFile = is_file( $source ); // regular files only
                        $copySuccess = $isFile ? copy( $source, $tmpPath ) : false;
                        $hadError = $this->untrapWarnings();
@@ -755,8 +744,19 @@ class FSFileBackend extends FileBackendStore {
                $statuses = [];
 
                $pipes = [];
+               $octalPermissions = '0' . decoct( $this->fileMode );
                foreach ( $fileOpHandles as $index => $fileOpHandle ) {
-                       $pipes[$index] = popen( "{$fileOpHandle->cmd} 2>&1", 'r' );
+                       $cmd = "{$fileOpHandle->cmd} 2>&1";
+                       // Add a post-operation chmod command for permissions cleanup if applicable
+                       if (
+                               !$this->isWindows &&
+                               $fileOpHandle->chmodPath !== null &&
+                               strlen( $octalPermissions ) == 4
+                       ) {
+                               $encPath = escapeshellarg( $fileOpHandle->chmodPath );
+                               $cmd .= " && chmod $octalPermissions $encPath 2>/dev/null";
+                       }
+                       $pipes[$index] = popen( $cmd, 'r' );
                }
 
                $errs = [];
@@ -772,12 +772,10 @@ class FSFileBackend extends FileBackendStore {
                        $function = $fileOpHandle->call;
                        $function( $errs[$index], $status, $fileOpHandle->params, $fileOpHandle->cmd );
                        $statuses[$index] = $status;
-                       if ( $status->isOK() && $fileOpHandle->chmodPath ) {
-                               $this->chmod( $fileOpHandle->chmodPath );
-                       }
                }
 
                clearstatcache(); // files changed
+
                return $statuses;
        }
 
@@ -788,13 +786,52 @@ class FSFileBackend extends FileBackendStore {
         * @return bool Success
         */
        protected function chmod( $path ) {
-               $this->trapWarnings();
+               if ( $this->isWindows ) {
+                       return true;
+               }
+
+               AtEase::suppressWarnings();
                $ok = chmod( $path, $this->fileMode );
-               $this->untrapWarnings();
+               AtEase::restoreWarnings();
+
+               return $ok;
+       }
+
+       /**
+        * Unlink a file, suppressing the warnings
+        *
+        * @param string $path Absolute file system path
+        * @return bool Success
+        */
+       protected function unlink( $path ) {
+               AtEase::suppressWarnings();
+               $ok = unlink( $path );
+               AtEase::restoreWarnings();
 
                return $ok;
        }
 
+       /**
+        * @param array $params Operation parameters with 'content' and 'headers' fields
+        * @return TempFSFile|null
+        */
+       protected function stageContentAsTempFile( array $params ) {
+               $content = $params['content'];
+               $tempFile = $this->tmpFileFactory->newTempFSFile( 'create_', 'tmp' );
+               if ( !$tempFile ) {
+                       return null;
+               }
+
+               AtEase::suppressWarnings();
+               $tmpPath = $tempFile->getPath();
+               if ( file_put_contents( $tmpPath, $content ) === false ) {
+                       $tempFile = null;
+               }
+               AtEase::restoreWarnings();
+
+               return $tempFile;
+       }
+
        /**
         * Return the text of an index.html file to hide directory listings
         *
@@ -824,30 +861,29 @@ class FSFileBackend extends FileBackendStore {
        }
 
        /**
-        * Listen for E_WARNING errors and track whether any happen
+        * Listen for E_WARNING errors and track whether any that happen
+        *
+        * @param string|null $regexIgnore Optional regex of errors to ignore
         */
-       protected function trapWarnings() {
-               // push to stack
-               $this->hadWarningErrors[] = false;
-               set_error_handler( function ( $errno, $errstr ) {
-                       // more detailed error logging
-                       $this->logger->error( $errstr );
-                       $this->hadWarningErrors[count( $this->hadWarningErrors ) - 1] = true;
-
-                       // suppress from PHP handler
-                       return true;
+       protected function trapWarnings( $regexIgnore = null ) {
+               $this->warningTrapStack[] = false;
+               set_error_handler( function ( $errno, $errstr ) use ( $regexIgnore ) {
+                       if ( $regexIgnore === null || !preg_match( $regexIgnore, $errstr ) ) {
+                               $this->logger->error( $errstr );
+                               $this->warningTrapStack[count( $this->warningTrapStack ) - 1] = true;
+                       }
+                       return true; // suppress from PHP handler
                }, E_WARNING );
        }
 
        /**
-        * Stop listening for E_WARNING errors and return true if any happened
+        * Stop listening for E_WARNING errors and get whether any happened
         *
-        * @return bool
+        * @return bool Whether any warnings happened
         */
        protected function untrapWarnings() {
-               // restore previous handler
                restore_error_handler();
-               // pop from stack
-               return array_pop( $this->hadWarningErrors );
+
+               return array_pop( $this->warningTrapStack );
        }
 }