FSFile and TempFSFile cleanups

* Remove wf* function dependencies. This includes wfTempDir().
  Callers now should specify the directory, though it will try to do
  most of the wfTempDir() logic anyway if they do not.
* Update callers to inject wfTempDir() so $wgTmpDirectory is used by
  TempFSFile instead of it probing to find a valid directory itself.
* Move most of the wfTempDir() logic to TempFSFile::getUsableTempDirectory().
* Remove unused getMimeType() method.

Change-Id: Idd55936b07f9448a6c90577708722b7b52b8fe66

includes/GlobalFunctions.php

@@ -2078,35 +2078,7 @@ function wfTempDir() {
 		return $wgTmpDirectory;
 	}
 
-	$tmpDir = array_map( "getenv", [ 'TMPDIR', 'TMP', 'TEMP' ] );
+	return TempFSFile::getUsableTempDirectory();
-	$tmpDir[] = sys_get_temp_dir();
-	$tmpDir[] = ini_get( 'upload_tmp_dir' );
-
-	foreach ( $tmpDir as $tmp ) {
-		if ( $tmp && file_exists( $tmp ) && is_dir( $tmp ) && is_writable( $tmp ) ) {
-			return $tmp;
-		}
-	}
-
-	/**
-	 * PHP on Windows will detect C:\Windows\Temp as not writable even though PHP can write to it
-	 * so create a directory within that called 'mwtmp' with a suffix of the user running the
-	 * current process.
-	 * The user is included as if various scripts are run by different users they will likely
-	 * not be able to access each others temporary files.
-	 */
-	if ( wfIsWindows() ) {
-		$tmp = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'mwtmp' . '-' . get_current_user();
-		if ( !file_exists( $tmp ) ) {
-			mkdir( $tmp );
-		}
-		if ( file_exists( $tmp ) && is_dir( $tmp ) && is_writable( $tmp ) ) {
-			return $tmp;
-		}
-	}
-
-	throw new MWException( 'No writable temporary directory could be found. ' .
-		'Please set $wgTmpDirectory to a writable directory.' );
 }
 
 /**

includes/api/ApiImageRotate.php

@@ -108,7 +108,7 @@ class ApiImageRotate extends ApiBase {
 				continue;
 			}
 			$ext = strtolower( pathinfo( "$srcPath", PATHINFO_EXTENSION ) );
-			$tmpFile = TempFSFile::factory( 'rotate_', $ext );
+			$tmpFile = TempFSFile::factory( 'rotate_', $ext, wfTempDir() );
 			$dstPath = $tmpFile->getPath();
 			$err = $handler->rotate( $file, [
 				'srcPath' => $srcPath,

includes/filebackend/FSFile.php

@@ -85,15 +85,6 @@ class FSFile {
 		return $timestamp;
 	}
 
-	/**
-	 * Guess the MIME type from the file contents alone
-	 *
-	 * @return string
-	 */
-	public function getMimeType() {
-		return MimeMagic::singleton()->guessMimeType( $this->path, false );
-	}
-
 	/**
 	 * Get an associative array containing information about
 	 * a file with the given storage path.
@@ -117,8 +108,6 @@ class FSFile {
 	 * @return array
 	 */
 	public function getProps( $ext = true ) {
-		wfDebug( __METHOD__ . ": Getting file info for $this->path\n" );
-
 		$info = self::placeholderProps();
 		$info['fileExists'] = $this->exists();
 
@@ -131,7 +120,7 @@ class FSFile {
 			}
 
 			# MIME type according to file contents
-			$info['file-mime'] = $this->getMimeType();
+			$info['file-mime'] = $magic->guessMimeType( $this->path, false );
 			# logical MIME type
 			$info['mime'] = $magic->improveTypeFromExtension( $info['file-mime'], $ext );
 
@@ -145,17 +134,15 @@ class FSFile {
 			$handler = MediaHandler::getHandler( $info['mime'] );
 			if ( $handler ) {
 				$tempImage = (object)[]; // XXX (hack for File object)
+				/** @noinspection PhpParamsInspection */
 				$info['metadata'] = $handler->getMetadata( $tempImage, $this->path );
+				/** @noinspection PhpParamsInspection */
 				$gis = $handler->getImageSize( $tempImage, $this->path, $info['metadata'] );
 				if ( is_array( $gis ) ) {
 					$info = $this->extractImageSizeInfo( $gis ) + $info;
 				}
 			}
 			$info['sha1'] = $this->getSha1Base36();
-
-			wfDebug( __METHOD__ . ": $this->path loaded, {$info['size']} bytes, {$info['mime']}.\n" );
-		} else {
-			wfDebug( __METHOD__ . ": $this->path NOT FOUND!\n" );
 		}
 
 		return $info;

includes/filebackend/TempFSFile.php

@@ -48,15 +48,20 @@ class TempFSFile extends FSFile {
 	 * Temporary files may be purged when the file object falls out of scope.
 	 *
 	 * @param string $prefix
-	 * @param string $extension
+	 * @param string $extension Optional file extension
+	 * @param string|null $tmpDirectory Optional parent directory
 	 * @return TempFSFile|null
 	 */
-	public static function factory( $prefix, $extension = '' ) {
+	public static function factory( $prefix, $extension = '', $tmpDirectory = null ) {
 		$ext = ( $extension != '' ) ? ".{$extension}" : '';
 
 		$attempts = 5;
 		while ( $attempts-- ) {
-			$path = wfTempDir() . '/' . $prefix . wfRandomString( 12 ) . $ext;
+			$hex = sprintf( '%06x%06x', mt_rand( 0, 0xffffff ), mt_rand( 0, 0xffffff ) );
+			if ( !is_string( $tmpDirectory ) ) {
+				$tmpDirectory = self::getUsableTempDirectory();
+			}
+			$path = wfTempDir() . '/' . $prefix . $hex . $ext;
 			MediaWiki\suppressWarnings();
 			$newFileHandle = fopen( $path, 'x' );
 			MediaWiki\restoreWarnings();
@@ -73,6 +78,40 @@ class TempFSFile extends FSFile {
 		return null;
 	}
 
+	/**
+	 * @return string Filesystem path to a temporary directory
+	 * @throws RuntimeException
+	 */
+	public static function getUsableTempDirectory() {
+		$tmpDir = array_map( 'getenv', [ 'TMPDIR', 'TMP', 'TEMP' ] );
+		$tmpDir[] = sys_get_temp_dir();
+		$tmpDir[] = ini_get( 'upload_tmp_dir' );
+		foreach ( $tmpDir as $tmp ) {
+			if ( $tmp != '' && is_dir( $tmp ) && is_writable( $tmp ) ) {
+				return $tmp;
+			}
+		}
+
+		// PHP on Windows will detect C:\Windows\Temp as not writable even though PHP can write to
+		// it so create a directory within that called 'mwtmp' with a suffix of the user running
+		// the current process.
+		// The user is included as if various scripts are run by different users they will likely
+		// not be able to access each others temporary files.
+		if ( strtoupper( substr( PHP_OS, 0, 3 ) ) === 'WIN' ) {
+			$tmp = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'mwtmp-' . get_current_user();
+			if ( !file_exists( $tmp ) ) {
+				mkdir( $tmp );
+			}
+			if ( is_dir( $tmp ) && is_writable( $tmp ) ) {
+				return $tmp;
+			}
+		}
+
+		throw new RuntimeException(
+			'No writable temporary directory could be found. ' .
+			'Please explicitly specify a writable directory in configuration.' );
+	}
+
 	/**
 	 * Purge this file off the file system
 	 *

includes/filerepo/file/File.php

@@ -1328,7 +1328,7 @@ abstract class File implements IDBAccessObject {
 	 */
 	protected function makeTransformTmpFile( $thumbPath ) {
 		$thumbExt = FileBackend::extensionFromPath( $thumbPath );
-		return TempFSFile::factory( 'transform_', $thumbExt );
+		return TempFSFile::factory( 'transform_', $thumbExt, wfTempDir() );
 	}
 
 	/**

includes/upload/UploadFromChunks.php

@@ -130,7 +130,7 @@ class UploadFromChunks extends UploadFromFile {
 		// Get the file extension from the last chunk
 		$ext = FileBackend::extensionFromPath( $this->mVirtualTempPath );
 		// Get a 0-byte temp file to perform the concatenation at
-		$tmpFile = TempFSFile::factory( 'chunkedupload_', $ext );
+		$tmpFile = TempFSFile::factory( 'chunkedupload_', $ext, wfTempDir() );
 		$tmpPath = false; // fail in concatenate()
 		if ( $tmpFile ) {
 			// keep alive with $this

includes/upload/UploadFromUrl.php

@@ -202,7 +202,7 @@ class UploadFromUrl extends UploadBase {
 	 * @return string Path to the file
 	 */
 	protected function makeTemporaryFile() {
-		$tmpFile = TempFSFile::factory( 'URL' );
+		$tmpFile = TempFSFile::factory( 'URL', 'urlupload_', wfTempDir() );
 		$tmpFile->bind( $this );
 
 		return $tmpFile->getPath();

tests/phpunit/includes/filebackend/FileBackendTest.php

@@ -268,7 +268,7 @@ class FileBackendTest extends MediaWikiTestCase {
 	public static function provider_testStore() {
 		$cases = [];
 
-		$tmpName = TempFSFile::factory( "unittests_", 'txt' )->getPath();
+		$tmpName = TempFSFile::factory( "unittests_", 'txt', wfTempDir() )->getPath();
 		$toPath = self::baseStorePath() . '/unittest-cont1/e/fun/obj1.txt';
 		$op = [ 'op' => 'store', 'src' => $tmpName, 'dst' => $toPath ];
 		$cases[] = [ $op ];
@@ -1786,9 +1786,9 @@ class FileBackendTest extends MediaWikiTestCase {
 		$fileBContents = 'g-jmq3gpqgt3qtg q3GT ';
 		$fileCContents = 'eigna[ogmewt 3qt g3qg flew[ag';
 
-		$tmpNameA = TempFSFile::factory( "unittests_", 'txt' )->getPath();
+		$tmpNameA = TempFSFile::factory( "unittests_", 'txt', wfTempDir() )->getPath();
-		$tmpNameB = TempFSFile::factory( "unittests_", 'txt' )->getPath();
+		$tmpNameB = TempFSFile::factory( "unittests_", 'txt', wfTempDir() )->getPath();
-		$tmpNameC = TempFSFile::factory( "unittests_", 'txt' )->getPath();
+		$tmpNameC = TempFSFile::factory( "unittests_", 'txt', wfTempDir() )->getPath();
 		$this->addTmpFiles( [ $tmpNameA, $tmpNameB, $tmpNameC ] );
 		file_put_contents( $tmpNameA, $fileAContents );
 		file_put_contents( $tmpNameB, $fileBContents );

tests/phpunit/includes/filerepo/MigrateFileRepoLayoutTest.php

@@ -59,7 +59,8 @@ class MigrateFileRepoLayoutTest extends MediaWikiTestCase {
 			->method( 'getRepo' )
 			->will( $this->returnValue( $repoMock ) );
 
-		$this->tmpFilepath = TempFSFile::factory( 'migratefilelayout-test-', 'png' )->getPath();
+		$this->tmpFilepath = TempFSFile::factory(
+			'migratefilelayout-test-', 'png', wfTempDir() )->getPath();
 
 		file_put_contents( $this->tmpFilepath, $this->text );
 

tests/phpunit/mocks/filebackend/MockFSFile.php

@@ -50,15 +50,11 @@ class MockFSFile extends FSFile {
 		return wfTimestamp( TS_MW );
 	}
 
-	public function getMimeType() {
-		return 'text/mock';
-	}
-
 	public function getProps( $ext = true ) {
 		return [
 			'fileExists' => $this->exists(),
 			'size' => $this->getSize(),
-			'file-mime' => $this->getMimeType(),
+			'file-mime' => 'text/mock',
 			'sha1' => $this->getSha1Base36(),
 		];
 	}