HeaderUtil::normalize(): remove CRLF from header names and values (GHSA-wxmh-65f7-jcvw)

Author: codemasher

Diff for: src/HeaderUtil.php

@@ -12,8 +12,9 @@
 
 namespace chillerlan\HTTP\Utils;
 
-use function array_keys, array_values, count, explode, implode,
-	is_array, is_numeric, is_string, strtolower, trim, ucfirst;
+use InvalidArgumentException;
+use function array_keys, array_values, count, explode, implode, is_array,
+	is_numeric, is_scalar, is_string, str_replace, strtolower, trim, ucfirst;
 
 /**
  *
@@ -48,27 +49,29 @@ public static function normalize(iterable $headers):array{
 
 				// array received from Message::getHeaders()
 				if(is_array($val)){
-					foreach($val as $line){
+					foreach(self::trimValues($val) as $line){
 						$normalized[$key][$name($line)] = trim($line);
 					}
 				}
 				else{
-					$normalized[$key][$name($val)] = trim($val);
+					$val = self::trimValues([$val])[0];
+
+					$normalized[$key][$name($val)] = $val;
 				}
 			}
 			// combine header fields with the same name
 			// https://datatracker.ietf.org/doc/html/rfc7230#section-3.2
 			else{
 
-				// the key is named, so we assume $val holds the header values only, either as string or array
-				if(is_array($val)){
-					$val = implode(', ', array_values($val));
+				if(!is_array($val)){
+					$val = [$val];
 				}
 
-				$val = trim((string)($val ?? ''));
+				/** @noinspection PhpParamsInspection */
+				$val = implode(', ', array_values(self::trimValues($val)));
 
 				// skip if the header already exists but the current value is empty
-				if(isset($normalized[$key]) && empty($val)){
+				if(isset($normalized[$key]) && $val === ''){
 					continue;
 				}
 
@@ -117,25 +120,33 @@ protected static function normalizeKV(mixed $value):array{
 	 * OWS          = *( SP / HTAB )
 	 *
 	 * @see https://tools.ietf.org/html/rfc7230#section-3.2.4
+	 * @see https://github.com/advisories/GHSA-wxmh-65f7-jcvw
 	 */
 	public static function trimValues(iterable $values):iterable{
 
 		foreach($values as &$value){
-			$value = trim((string)($value ?? ''), " \t");
+
+			if(!is_scalar($value) && $value !== null){
+				throw new InvalidArgumentException('value is expected to be scalar or null');
+			}
+
+			$value = trim(str_replace(["\r", "\n"], '', (string)($value ?? '')));
 		}
 
 		return $values;
 	}
 
 	/**
 	 * Normalizes a header name, e.g. "con TENT- lenGTh" -> "Content-Length"
+	 *
+	 * @see https://github.com/advisories/GHSA-wxmh-65f7-jcvw
 	 */
 	public static function normalizeHeaderName(string $name):string{
-		$parts = explode('-', $name);
+		// we'll remove any spaces as well as CRLF in the name, e.g. "con tent" -> "content"
+		$parts = explode('-', str_replace([' ', "\r", "\n"], '', $name));
 
 		foreach($parts as &$part){
-			// we'll remove any spaces in the name part, e.g. "con tent" -> "content"
-			$part = ucfirst(strtolower(str_replace(' ', '', trim($part))));
+			$part = ucfirst(strtolower(trim($part)));
 		}
 
 		return implode('-', $parts);

Diff for: tests/HeaderUtilTest.php

@@ -40,6 +40,7 @@ public static function headerDataProvider():array{
 			'empty value'   => [['empty-value' => ''], ['Empty-Value' => '']],
 			'null value'    => [['null-value' => null], ['Null-Value' => '']],
 			'space in name' => [['space name - header' => 'nope'], ['Spacename-Header' => 'nope']],
+			'CRLF'          => [["CR\rLF-\nin-Na\r\n\r\nme" => " CR\rLF-\nin-va\r\n\r\nlue "], ['Crlf-In-Name' => 'CRLF-in-value']],
 		];
 	}
 
@@ -133,6 +134,7 @@ public static function headerNameProvider():array{
 			'UPPERCASEKEY' => ['UPPERCASEKEY', 'Uppercasekey'],
 			'mIxEdCaSeKey' => ['mIxEdCaSeKey', 'Mixedcasekey'],
 			'31i71casekey' => ['31i71casekey', '31i71casekey'],
+			'CRLF-In-Name' => ["CR\rLF-\nin-Na\r\n\r\nme", 'Crlf-In-Name'],
 		];
 	}
 
@@ -141,8 +143,20 @@ public function testNormalizeHeaderName(string $name, string $expected):void{
 		$this::assertSame($expected, HeaderUtil::normalizeHeaderName($name));
 	}
 
-	public function testTrimValues():void{
-		$this::assertSame(['69', '420'], HeaderUtil::trimValues([69, '	420 ']));
+	public static function headerValueProvider():array{
+		return [
+			'boolean'                => [true, '1'],
+			'float'                  => [69.420, '69.42'],
+			'int'                    => [69, '69'],
+			'numeric string'         => ['69.420', '69.420'],
+			'string with whitespace' => ['	 hello	 ', 'hello'],
+			'CRLF-In-Value'          => [" CR\rLF-\nIn-Va\r\n\r\nlue ", 'CRLF-In-Value'],
+		];
+	}
+
+	#[DataProvider('headerValueProvider')]
+	public function testTrimValues(string|int|float|bool $value, string $expected):void{
+		$this::assertSame([$expected], HeaderUtil::trimValues([$value]));
 	}
 
 }