[20220307] - Core - Variable Tampering on JInput $_REQUEST data

@@ -92,7 +92,7 @@ class Input implements \Countable
 	 */
 	public function __construct($source = null, array $options = [])
 	{
-		$this->data    = empty($source) ? $_REQUEST : $source;
+		$this->data    = $source ?? $_REQUEST;
 		$this->filter  = $options['filter'] ?? new Filter\InputFilter;
 		$this->options = $options;
 	}

@@ -17,13 +17,6 @@
  */
 class InputTest extends TestCase
 {
-	/**
-	 * The test class.
-	 *
-	 * @var  Input
-	 */
-	private $instance;
-
 	/**
 	 * The mock filter object
 	 *
@@ -47,21 +40,22 @@ protected function setUp(): void
 	/**
 	 * Get an Input object populated with passed in data
 	 *
-	 * @param   array  $data  Optional source data. If omitted, a copy of the server variable '_REQUEST' is used.
+	 * @param   array|null  $data  Optional source data. If omitted, a copy of the server variable '_REQUEST' is used.
 	 *
 	 * @return  Input
 	 */
-	protected function getInputObject($data = null)
+	protected function getInputObject(?array $data = null): Input
 	{
 		return new Input($data, ['filter' => $this->filterMock]);
 	}
 
 	/**
-	 * @testdox  Tests the default constructor behavior
+	 * @testdox  Default constructor behavior
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
+	 * @throws \ReflectionException
 	 */
-	public function test__constructDefaultBehaviour()
+	public function test__constructDefaultBehaviour(): void
 	{
 		$instance = new Input;
 
@@ -70,11 +64,12 @@ public function test__constructDefaultBehaviour()
 	}
 
 	/**
-	 * @testdox  Tests the constructor with injected data
+	 * @testdox  Constructor with injected data
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
+	 * @throws \ReflectionException
 	 */
-	public function test__constructDependencyInjection()
+	public function test__constructDependencyInjection(): void
 	{
 		$instance = $this->getInputObject($_GET);
 
@@ -83,11 +78,11 @@ public function test__constructDependencyInjection()
 	}
 
 	/**
-	 * @testdox  Tests convenience methods are proxied
+	 * @testdox  Convenience methods are proxied
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function test__callProxiesToTheGetMethod()
+	public function test__callProxiesToTheGetMethod(): void
 	{
 		$this->filterMock->expects($this->once())
 			->method('clean')
@@ -99,23 +94,25 @@ public function test__callProxiesToTheGetMethod()
 	}
 
 	/**
-	 * @testdox  Tests an error is thrown if an undefined method is called
+	 * @testdox  An error is thrown if an undefined method is called
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function test__callThrowsAnErrorIfAnUndefinedMethodIsCalled()
+	public function test__callThrowsAnErrorIfAnUndefinedMethodIsCalled(): void
 	{
 		$this->expectError();
 
-		$instance = $this->getInputObject()->setRaw();
+		/** @noinspection PhpUndefinedMethodInspection */
+		$this->getInputObject()->setRaw();
 	}
 
 	/**
-	 * @testdox   Tests the magic get method correctly proxies to another global
+	 * @testdox   Magic get method correctly proxies to another global
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers    \Joomla\Input\Input
+	 * @throws \ReflectionException
 	 */
-	public function test__get()
+	public function test__get(): void
 	{
 		$instance = $this->getInputObject();
 
@@ -124,58 +121,59 @@ public function test__get()
 	}
 
 	/**
-	 * @testdox   Tests the magic get method correctly proxies to another global represented by the Input class and returns the same instance
+	 * @testdox   Magic get method correctly proxies to another global represented by the Input class and returns the same instance
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function test__getCachedInstances()
+	public function test__getCachedInstances(): void
 	{
 		$instance = $this->getInputObject();
 
 		$this->assertSame($instance->get, $instance->get, 'The same Input instance should be returned');
 	}
 
 	/**
-	 * @testdox   Tests the magic get method correctly proxies to another global represented by a Input subclass and returns the same instance
+	 * @testdox   Magic get method correctly proxies to another global represented by an Input subclass and returns the same instance
 	 *
-	 * @covers   Joomla\Input\Input
-	 * @uses     Joomla\Input\Files
+	 * @covers   \Joomla\Input\Input
+	 * @uses     \Joomla\Input\Files
 	 */
-	public function test__getCachedInstancesSubclasses()
+	public function test__getCachedInstancesSubclasses(): void
 	{
 		$instance = $this->getInputObject();
 
 		$this->assertSame($instance->files, $instance->files, 'The same Files instance should be returned');
 	}
 
 	/**
-	 * @testdox   Tests an error is thrown if an undefined property is called
+	 * @testdox   An error is thrown if an undefined property is called
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function test__getThrowsAnErrorIfAnUndefinedPropertyIsCalled()
+	public function test__getThrowsAnErrorIfAnUndefinedPropertyIsCalled(): void
 	{
 		$this->expectError();
 
-		$instance = $this->getInputObject()->put;
+		/** @noinspection PhpUndefinedFieldInspection */
+		$this->getInputObject()->put;
 	}
 
 	/**
-	 * @testdox   Tests the data store is counted
+	 * @testdox   Data store is counted
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testCount()
+	public function testCount(): void
 	{
 		$this->assertCount(3, $this->getInputObject(['foo' => 2, 'bar' => 3, 'gamma' => 4]));
 	}
 
 	/**
-	 * @testdox  Tests the data source is correctly read
+	 * @testdox  Data source is correctly read
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testGet()
+	public function testGet(): void
 	{
 		$this->filterMock->expects($this->once())
 			->method('clean')
@@ -187,11 +185,11 @@ public function testGet()
 	}
 
 	/**
-	 * @testdox  Tests a key is not redefined if already present
+	 * @testdox  A key is not redefined if already present
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testDefNotReadWhenValueExists()
+	public function testDefNotReadWhenValueExists(): void
 	{
 		$this->filterMock->expects($this->once())
 			->method('clean')
@@ -205,11 +203,11 @@ public function testDefNotReadWhenValueExists()
 	}
 
 	/**
-	 * @testdox  Tests a key is defined when not present
+	 * @testdox  A key is defined when not present
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testDefRead()
+	public function testDefRead(): void
 	{
 		$this->filterMock->expects($this->once())
 			->method('clean')
@@ -223,11 +221,11 @@ public function testDefRead()
 	}
 
 	/**
-	 * @testdox  Tests a key is added or overwritten in the data source
+	 * @testdox  A key is added or overwritten in the data source
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testSet()
+	public function testSet(): void
 	{
 		$this->filterMock->expects($this->once())
 			->method('clean')
@@ -241,25 +239,25 @@ public function testSet()
 	}
 
 	/**
-	 * @testdox  Tests for a key's existence in the data source
+	 * @testdox  For a key's existence in the data source
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testExists()
+	public function testExists(): void
 	{
 		$instance = $this->getInputObject(['foo' => 'bar']);
 
 		$this->assertTrue($instance->exists('foo'));
 	}
 
 	/**
-	 * @testdox  Tests that an array of keys are read from the data source
+	 * @testdox  An array of keys is read from the data source
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testGetArray()
+	public function testGetArray(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -281,13 +279,13 @@ public function testGetArray()
 	}
 
 	/**
-	 * @testdox  Tests that the full data array is read from the data source
+	 * @testdox  Full data array is read from the data source
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testGetArrayWithoutSpecifiedVariables()
+	public function testGetArrayWithoutSpecifiedVariables(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -306,15 +304,15 @@ public function testGetArrayWithoutSpecifiedVariables()
 	}
 
 	/**
-	 * @testdox  Tests that the request method is returned
+	 * @testdox  Request method is returned
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 *
 	 * @backupGlobals enabled
 	 */
-	public function testGetMethod()
+	public function testGetMethod(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -326,15 +324,15 @@ public function testGetMethod()
 	}
 
 	/**
-	 * @testdox  Tests that the Input object for the request method is returned on a GET request
+	 * @testdox  Input object for the request method is returned on a GET request
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 *
 	 * @backupGlobals enabled
 	 */
-	public function testGetInputForRequestMethodWithGetRequest()
+	public function testGetInputForRequestMethodWithGetRequest(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -347,15 +345,15 @@ public function testGetInputForRequestMethodWithGetRequest()
 	}
 
 	/**
-	 * @testdox  Tests that the Input object for the request method is returned on a POST request
+	 * @testdox  Input object for the request method is returned on a POST request
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 *
 	 * @backupGlobals enabled
 	 */
-	public function testGetInputForRequestMethodWithPostRequest()
+	public function testGetInputForRequestMethodWithPostRequest(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -368,15 +366,15 @@ public function testGetInputForRequestMethodWithPostRequest()
 	}
 
 	/**
-	 * @testdox  Tests that the Input object for the request method is returned on a PUT request
+	 * @testdox  Input object for the request method is returned on a PUT request
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 *
 	 * @backupGlobals enabled
 	 */
-	public function testGetInputForRequestMethodWithPutRequest()
+	public function testGetInputForRequestMethodWithPutRequest(): void
 	{
-		$this->filterMock->expects($this->any())
+		$this->filterMock
 			->method('clean')
 			->willReturnArgument(0);
 
@@ -389,17 +387,51 @@ public function testGetInputForRequestMethodWithPutRequest()
 	}
 
 	/**
-	 * @testdox  Tests that the get method disallows access to non-whitelisted globals
+	 * @testdox  Get method disallows access to non-whitelisted globals
 	 *
-	 * @covers   Joomla\Input\Input
+	 * @covers   \Joomla\Input\Input
 	 */
-	public function testGetDoesNotSupportNonWhitelistedGlobals()
+	public function testGetDoesNotSupportNonWhitelistedGlobals(): void
+	{
+		$this->expectError();
+		$this->getInputObject()->_phpunit_configuration_file;
+	}
+
+	public function constructorCases(): \Generator
 	{
-		$this->markTestSkipped('Update to account for notice being raised.');
+		yield 'no source' => [
+			'constructor-arg' => null,
+			'expected' => 'value',
+		];
 
-		$this->assertNull(
-			$this->getInputObject()->_phpunit_configuration_file,
-			'Access to library defined globals is restricted'
-		);
+		yield 'empty source' => [
+			'constructor-arg' => [],
+			'expected' => null,
+		];
+
+		yield 'non-empty source' => [
+			'constructor-arg' => ['foo' => 'bar'],
+			'expected' => null,
+		];
+
+		yield 'same key' => [
+			'constructor-arg' => ['var' => 'bar'],
+			'expected' => 'bar',
+		];
+	}
+
+	/**
+	 * @testdox If no source is provided ($source === null), $_REQUEST is used. If any source is provided ($source !== null), $_REQUEST is ignored.
+	 *
+	 * @dataProvider constructorCases
+	 * @return void
+	 */
+	public function testConstructorUsesRequestIfNeeded($constructorArgs, $expected): void
+	{
+		$_REQUEST = ['var' => 'value'];
+
+		$input = new Input($constructorArgs);
+
+		$this->assertEquals($expected, $input->get('var'));
 	}
 }