Create FauxRequestUpload to fake uploads in tests

And use it in core
Avoid direct use of super global $_FILES

This can breaks all FauxRequest relaying on $_FILES
in tests or production code via FauxRequest::getUpload.
Falls back to $_FILES for the moment

Bug: T48163
Change-Id: I7392acc9bb682ec6b7025dbed0734c142f45c91a

autoload.php

@@ -491,6 +491,7 @@ $wgAutoloadLocalClasses = [
 	'FallbackContentHandler' => __DIR__ . '/includes/content/FallbackContentHandler.php',
 	'FatalError' => __DIR__ . '/includes/exception/FatalError.php',
 	'FauxRequest' => __DIR__ . '/includes/FauxRequest.php',
+	'FauxRequestUpload' => __DIR__ . '/includes/FauxRequestUpload.php',
 	'FauxResponse' => __DIR__ . '/includes/FauxResponse.php',
 	'FauxSearchResult' => __DIR__ . '/includes/search/FauxSearchResult.php',
 	'FauxSearchResultSet' => __DIR__ . '/includes/search/FauxSearchResultSet.php',

includes/DerivativeRequest.php

@@ -90,6 +90,10 @@ class DerivativeRequest extends FauxRequest {
 		return $this->base->getProtocol();
 	}
 
+	public function getUpload( $key ) {
+		return $this->base->getUpload( $key );
+	}
+
 	public function getElapsedTime() {
 		return $this->base->getElapsedTime();
 	}

includes/FauxRequest.php

@@ -36,6 +36,8 @@ class FauxRequest extends WebRequest {
 	private $wasPosted = false;
 	private $requestUrl;
 	protected $cookies = [];
+	/** @var array */
+	private $uploadData = [];
 
 	/**
 	 * @stable to call
@@ -147,6 +149,56 @@ class FauxRequest extends WebRequest {
 		}
 	}
 
+	/**
+	 * Set fake upload data for all files
+	 *
+	 * @since 1.37
+	 * @param (array|WebRequestUpload)[] $uploadData
+	 */
+	public function setUploadData( $uploadData ) {
+		foreach ( $uploadData as $key => $data ) {
+			$this->setUpload( $key, $data );
+		}
+	}
+
+	/**
+	 * Set fake upload data for one file with specific key
+	 *
+	 * @since 1.37
+	 * @param string $key
+	 * @param array|WebRequestUpload $data
+	 */
+	public function setUpload( $key, $data ) {
+		if ( $data instanceof WebRequestUpload ) {
+			// cannot reuse WebRequestUpload, because it contains the original web request object
+			$data = [
+				'name' => $data->getName(),
+				'type' => $data->getType(),
+				'tmp_name' => $data->getTempName(),
+				'size' => $data->getSize(),
+				'error' => $data->getError(),
+			];
+		}
+		// Check if everything is provided
+		if ( !is_array( $data ) ||
+			array_diff( WebRequestUpload::REQUIRED_FILEINFO_KEYS, array_keys( $data ) ) !== []
+		) {
+			throw new MWException( __METHOD__ . ' got bogus data' );
+		}
+		$this->uploadData[$key] = $data;
+	}
+
+	/**
+	 * Return a FauxRequestUpload object corresponding to the key
+	 *
+	 * @param string $key
+	 * @return FauxRequestUpload
+	 */
+	public function getUpload( $key ) {
+		// $_FILES is used for backward compatibility
+		return new FauxRequestUpload( $this->uploadData + $_FILES, $this, $key );
+	}
+
 	/**
 	 * @since 1.25
 	 * @param string $url

includes/FauxRequestUpload.php

@@ -0,0 +1,47 @@
+<?php
+/**
+ * Object to access a fake $_FILES array for testing purpose
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Object to fake the $_FILES array
+ *
+ * @ingroup HTTP
+ * @since 1.37
+ */
+class FauxRequestUpload extends WebRequestUpload {
+
+	/**
+	 * Constructor. Should only be called by FauxRequest
+	 *
+	 * @param array $data Array of *non*-urlencoded key => value pairs, the
+	 *   fake (whole) FILES values
+	 * @param FauxRequest $request The associated faux request
+	 * @param string $key name of upload param
+	 */
+	public function __construct( $data, $request, $key ) {
+		$this->request = $request;
+		$this->doesExist = isset( $data[$key] );
+		if ( $this->doesExist ) {
+			$this->fileInfo = $data[$key];
+		}
+	}
+
+}

includes/WebRequest.php

@@ -1038,8 +1038,7 @@ class WebRequest {
 	 * @return string|null String or null if no such file.
 	 */
 	public function getFileTempname( $key ) {
-		$file = new WebRequestUpload( $this, $key );
-		return $file->getTempName();
+		return $this->getUpload( $key )->getTempName();
 	}
 
 	/**
@@ -1049,8 +1048,7 @@ class WebRequest {
 	 * @return int
 	 */
 	public function getUploadError( $key ) {
-		$file = new WebRequestUpload( $this, $key );
-		return $file->getError();
+		return $this->getUpload( $key )->getError();
 	}
 
 	/**
@@ -1065,8 +1063,7 @@ class WebRequest {
 	 * @return string|null String or null if no such file.
 	 */
 	public function getFileName( $key ) {
-		$file = new WebRequestUpload( $this, $key );
-		return $file->getName();
+		return $this->getUpload( $key )->getName();
 	}
 
 	/**

includes/WebRequestUpload.php

@@ -31,6 +31,8 @@ use MediaWiki\MediaWikiServices;
  * @ingroup HTTP
  */
 class WebRequestUpload {
+	/** All keys a fileinfo has to specific to work with this class */
+	public const REQUIRED_FILEINFO_KEYS = [ 'name', 'size', 'tmp_name', 'type', 'error', ];
 	/** @var WebRequest */
 	protected $request;
 	/** @var bool */

tests/phpunit/includes/api/ApiTestCase.php

@@ -105,7 +105,7 @@ abstract class ApiTestCase extends MediaWikiLangTestCase {
 			)->toString();
 		}
 
-		$wgRequest = new FauxRequest( $params, true, $sessionObj );
+		$wgRequest = $this->buildFauxRequest( $params, $sessionObj );
 		RequestContext::getMain()->setRequest( $wgRequest );
 		RequestContext::getMain()->setUser( $contextUser );
 		MediaWiki\Auth\AuthManager::resetCache();
@@ -132,6 +132,16 @@ abstract class ApiTestCase extends MediaWikiLangTestCase {
 		return $results;
 	}
 
+	/**
+	 * @since 1.37
+	 * @param array $params
+	 * @param MediaWiki\Session\Session|array|null $session
+	 * @return FauxRequest
+	 */
+	protected function buildFauxRequest( $params, $session ) {
+		return new FauxRequest( $params, true, $session );
+	}
+
 	/**
 	 * Convenience function to access the token parameter of doApiRequest()
 	 * more succinctly.

tests/phpunit/includes/api/ApiUploadTestCase.php

@@ -6,6 +6,13 @@ use MediaWiki\MediaWikiServices;
  * Abstract class to support upload tests
  */
 abstract class ApiUploadTestCase extends ApiTestCase {
+
+	/**
+	 * @since 1.37
+	 * @var array Used to fake $_FILES in tests and given to FauxRequest
+	 */
+	protected $requestDataFiles = [];
+
 	/**
 	 * Fixture -- run before every test
 	 */
@@ -115,7 +122,7 @@ abstract class ApiUploadTestCase extends ApiTestCase {
 			throw new Exception( "couldn't stat $tmpName" );
 		}
 
-		$_FILES[$fieldName] = [
+		$this->requestDataFiles[$fieldName] = [
 			'name' => $fileName,
 			'type' => $type,
 			'tmp_name' => $tmpName,
@@ -139,7 +146,7 @@ abstract class ApiUploadTestCase extends ApiTestCase {
 			throw new Exception( "couldn't stat $tmpName" );
 		}
 
-		$_FILES[$fieldName] = [
+		$this->requestDataFiles[$fieldName] = [
 			'name' => $fileName,
 			'type' => $type,
 			'tmp_name' => $tmpName,
@@ -148,10 +155,17 @@ abstract class ApiUploadTestCase extends ApiTestCase {
 		];
 	}
 
+	/** @inheritDoc */
+	protected function buildFauxRequest( $params, $session ) {
+		$request = parent::buildFauxRequest( $params, $session );
+		$request->setUploadData( $this->requestDataFiles );
+		return $request;
+	}
+
 	/**
 	 * Remove traces of previous fake uploads
 	 */
 	public function clearFakeUploads() {
-		$_FILES = [];
+		$this->requestDataFiles = [];
 	}
 }

tests/phpunit/includes/api/Validator/ApiParamValidatorCallbacksTest.php

@@ -92,7 +92,7 @@ class ApiParamValidatorCallbacksTest extends ApiUploadTestCase {
 		$filePath = $this->filePath( 'yuv420.jpg' );
 		$this->fakeUploadFile( 'file', $fileName, $mimeType, $filePath );
 
-		$_FILES['file2'] = [
+		$this->requestDataFiles['file2'] = [
 			'name' => '',
 			'type' => '',
 			'tmp_name' => '',
@@ -100,7 +100,7 @@ class ApiParamValidatorCallbacksTest extends ApiUploadTestCase {
 			'error' => UPLOAD_ERR_NO_FILE,
 		];
 
-		$_FILES['file3'] = [
+		$this->requestDataFiles['file3'] = [
 			'name' => 'xxx.png',
 			'type' => '',
 			'tmp_name' => '',
@@ -112,10 +112,12 @@ class ApiParamValidatorCallbacksTest extends ApiUploadTestCase {
 	public function testHasUpload() : void {
 		$this->setupUploads();
 
-		[ $callbacks, $main ] = $this->getCallbacks( new FauxRequest( [
+		$request = new FauxRequest( [
 			'foo' => '1',
 			'bar' => '',
-		] ) );
+		] );
+		$request->setUploadData( $this->requestDataFiles );
+		[ $callbacks, $main ] = $this->getCallbacks( $request );
 
 		$this->assertFalse( $callbacks->hasUpload( 'foo', [] ) );
 		$this->assertFalse( $callbacks->hasUpload( 'bar', [] ) );
@@ -133,10 +135,12 @@ class ApiParamValidatorCallbacksTest extends ApiUploadTestCase {
 	public function testGetUploadedFile() : void {
 		$this->setupUploads();
 
-		[ $callbacks, $main ] = $this->getCallbacks( new FauxRequest( [
+		$request = new FauxRequest( [
 			'foo' => '1',
 			'bar' => '',
-		] ) );
+		] );
+		$request->setUploadData( $this->requestDataFiles );
+		[ $callbacks, $main ] = $this->getCallbacks( $request );
 
 		$this->assertNull( $callbacks->getUploadedFile( 'foo', [] ) );
 		$this->assertNull( $callbacks->getUploadedFile( 'bar', [] ) );