Response\Headers: add input validation + more defensive coding

src/Response/Headers.php

@@ -8,6 +8,7 @@
 namespace WpOrg\Requests\Response;
 
 use WpOrg\Requests\Exception;
+use WpOrg\Requests\Exception\InvalidArgument;
 use WpOrg\Requests\Utility\CaseInsensitiveDictionary;
 use WpOrg\Requests\Utility\FilteredIterator;
 
@@ -30,7 +31,10 @@ class Headers extends CaseInsensitiveDictionary {
 	 * @return string|null Header value
 	 */
 	public function offsetGet($offset) {
-		$offset = strtolower($offset);
+		if (is_string($offset)) {
+			$offset = strtolower($offset);
+		}
+
 		if (!isset($this->data[$offset])) {
 			return null;
 		}
@@ -51,7 +55,9 @@ public function offsetSet($offset, $value) {
 			throw new Exception('Object is a dictionary, not a list', 'invalidset');
 		}
 
-		$offset = strtolower($offset);
+		if (is_string($offset)) {
+			$offset = strtolower($offset);
+		}
 
 		if (!isset($this->data[$offset])) {
 			$this->data[$offset] = array();
@@ -65,8 +71,14 @@ public function offsetSet($offset, $value) {
 	 *
 	 * @param string $offset
 	 * @return array|null Header values
+	 *
+	 * @throws \WpOrg\Requests\Exception\InvalidArgument When the passed argument is not valid as an array key.
 	 */
 	public function getValues($offset) {
+		if (!is_string($offset) && !is_int($offset)) {
+			throw InvalidArgument::create(1, '$offset', 'string|int', gettype($offset));
+		}
+
 		$offset = strtolower($offset);
 		if (!isset($this->data[$offset])) {
 			return null;
@@ -83,13 +95,19 @@ public function getValues($offset) {
 	 *
 	 * @param string|array $value Value to flatten
 	 * @return string Flattened value
+	 *
+	 * @throws \WpOrg\Requests\Exception\InvalidArgument When the passed argument is not a string or an array.
 	 */
 	public function flatten($value) {
+		if (is_string($value)) {
+			return $value;
+		}
+
 		if (is_array($value)) {
-			$value = implode(',', $value);
+			return implode(',', $value);
 		}
 
-		return $value;
+		throw InvalidArgument::create(1, '$value', 'string|array', gettype($value));
 	}
 
 	/**

tests/Response/HeadersTest.php

@@ -2,7 +2,9 @@
 
 namespace WpOrg\Requests\Tests\Response;
 
+use stdClass;
 use WpOrg\Requests\Exception;
+use WpOrg\Requests\Exception\InvalidArgument;
 use WpOrg\Requests\Response\Headers;
 use WpOrg\Requests\Tests\TestCase;
 
@@ -76,18 +78,87 @@ public function testMultipleHeaders() {
 		$this->assertSame('text/html;q=1.0,*/*;q=0.1', $headers['Accept']);
 	}
 
+	/**
+	 * Test that non-string array keys are handled correctly.
+	 *
+	 * @covers ::offsetSet
+	 *
+	 * @dataProvider dataOffsetSetDoesNotTryToLowercaseNonStringKeys
+	 *
+	 * @param mixed      $key         Key to set.
+	 * @param string|int $request_key Key to retrieve if different.
+	 *
+	 * @return void
+	 */
+	public function testOffsetSetDoesNotTryToLowercaseNonStringKeys($key, $request_key = null) {
+		$headers       = new Headers();
+		$headers[$key] = 'value';
+
+		if (!isset($request_key)) {
+			$request_key = $key;
+		}
+
+		$this->assertSame('value', $headers[$request_key]);
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array
+	 */
+	public function dataOffsetSetDoesNotTryToLowercaseNonStringKeys() {
+		return array(
+			'integer key'       => array(10),
+			'boolean false key' => array(false, 0),
+		);
+	}
+
+	/**
+	 * Test that multiple headers can be registered on a non-string key.
+	 *
+	 * @covers ::offsetGet
+	 * @covers ::offsetSet
+	 *
+	 * @return void
+	 */
+	public function testOffsetSetRegisterMultipleHeadersOnIntegerKey() {
+		$headers     = new Headers();
+		$headers[10] = 'value1';
+		$headers[10] = 'value2';
+
+		$this->assertSame('value1,value2', $headers[10]);
+	}
+
 	/**
 	 * Test that null is returned when a non-registered header is requested.
 	 *
 	 * @covers ::offsetGet
 	 *
+	 * @dataProvider dataOffsetGetReturnsNullForNonRegisteredHeader
+	 *
+	 * @param mixed $key Key to request.
+	 *
 	 * @return void
 	 */
-	public function testOffsetGetReturnsNullForNonRegisteredHeader() {
+	public function testOffsetGetReturnsNullForNonRegisteredHeader($key) {
 		$headers                 = new Headers();
 		$headers['Content-Type'] = 'text/plain';
 
-		$this->assertNull($headers['not-content-type']);
+		$this->assertNull($headers[$key]);
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array
+	 */
+	public function dataOffsetGetReturnsNullForNonRegisteredHeader() {
+		return array(
+			// This test case also tests that no "passing null to non-nullable" deprecation is thrown in PHP 8.1.
+			'null'                       => array(null),
+			'non-registered integer key' => array(10),
+			'non-registred string key'   => array('not-content-type'),
+		);
 	}
 
 	/**
@@ -149,6 +220,37 @@ public function dataGetValues() {
 		);
 	}
 
+	/**
+	 * Tests receiving an exception when an invalid offset is passed to getValues().
+	 *
+	 * @covers ::getValues
+	 *
+	 * @dataProvider dataGetValuesInvalidOffset
+	 *
+	 * @param mixed $key Requested offset.
+	 *
+	 * @return void
+	 */
+	public function testGetValuesInvalidOffset($key) {
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage('Argument #1 ($offset) must be of type string|int');
+
+		$headers = new Headers();
+		$headers->getValues($key);
+	}
+
+	/**
+	 * Data Provider.
+	 *
+	 * @return array
+	 */
+	public function dataGetValuesInvalidOffset() {
+		return array(
+			'null'          => array(null),
+			'boolean false' => array(false),
+		);
+	}
+
 	/**
 	 * Test iterator access for the object is supported.
 	 *
@@ -184,4 +286,66 @@ public function testIteration() {
 			}
 		}
 	}
+
+	/**
+	 * Tests flattening of data.
+	 *
+	 * @covers ::flatten
+	 *
+	 * @dataProvider dataFlatten
+	 *
+	 * @param string|array $input    Value to flatten.
+	 * @param string       $expected Expected output value.
+	 *
+	 * @return void
+	 */
+	public function testFlatten($input, $expected) {
+		$headers = new Headers();
+		$this->assertSame($expected, $headers->flatten($input));
+	}
+
+	/**
+	 * Data Provider.
+	 *
+	 * @return array
+	 */
+	public function dataFlatten() {
+		return array(
+			'string'            => array('text', 'text'),
+			'empty array'       => array(array(), ''),
+			'array with values' => array(array('text', 10, 'more text'), 'text,10,more text'),
+		);
+	}
+
+	/**
+	 * Tests receiving an exception when an invalid value is passed to flatten().
+	 *
+	 * @covers ::flatten
+	 *
+	 * @dataProvider dataFlattenInvalidValue
+	 *
+	 * @param mixed $input Value to flatten.
+	 *
+	 * @return void
+	 */
+	public function testFlattenInvalidValue($input) {
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage('Argument #1 ($value) must be of type string|array');
+
+		$headers = new Headers();
+		$headers->flatten($input);
+	}
+
+	/**
+	 * Data Provider.
+	 *
+	 * @return array
+	 */
+	public function dataFlattenInvalidValue() {
+		return array(
+			'null'          => array(null),
+			'boolean false' => array(false),
+			'plain object'  => array(new stdClass()),
+		);
+	}
 }