diff --git a/HeaderTrait.php b/HeaderTrait.php index 0767c8f..9ad3379 100644 --- a/HeaderTrait.php +++ b/HeaderTrait.php @@ -12,10 +12,12 @@ namespace Koded\Http; +use InvalidArgumentException; +use Throwable; + trait HeaderTrait { - /** * @var array Message headers. */ @@ -52,9 +54,11 @@ public function getHeaderLine($name): string public function withHeader($name, $value): self { $instance = clone $this; + $name = $instance->normalizeHeaderName($name); $instance->headersMap[strtolower($name)] = $name; - $instance->headers[$name] = (array)$value; + + $instance->headers[$name] = $this->normalizeHeaderValue($name, $value); return $instance; } @@ -73,22 +77,24 @@ public function withHeaders(array $headers): self public function withoutHeader($name): self { $instance = clone $this; - $key = strtolower($name); - unset($instance->headersMap[$key], $instance->headers[$this->headersMap[$key]]); + $name = strtolower($name); + unset($instance->headers[$this->headersMap[$name]], $instance->headersMap[$name]); return $instance; } public function withAddedHeader($name, $value): self { - $value = (array)$value; $instance = clone $this; + $name = $instance->normalizeHeaderName($name); + $value = $instance->normalizeHeaderValue($name, $value); if (isset($instance->headersMap[$header = strtolower($name)])) { - $instance->headers[$name] = array_unique(array_merge((array)$this->headers[$name], $value)); + $header = $instance->headersMap[$header]; + $instance->headers[$header] = array_unique(array_merge((array)$instance->headers[$header], $value)); } else { - $instance->headersMap[strtolower($name)] = $name; - $instance->headers[$name] = $value; + $instance->headersMap[$header] = $name; + $instance->headers[$name] = $value; } return $instance; @@ -149,14 +155,14 @@ public function getCanonicalizedHeaders(array $names = []): string /** * @param string $name - * @param string $value + * @param mixed $value * @param bool $skipKey * * @return void */ protected function normalizeHeader(string $name, $value, bool $skipKey): void { - $name = trim($name); + $name = str_replace(["\r", "\n", "\t"], '', trim($name)); if (false === $skipKey) { $name = ucwords(str_replace('_', '-', strtolower($name)), '-'); @@ -164,7 +170,7 @@ protected function normalizeHeader(string $name, $value, bool $skipKey): void $this->headersMap[strtolower($name)] = $name; - $this->headers[$name] = array_map('trim', (array)$value); + $this->headers[$name] = $this->normalizeHeaderValue($name, $value); } /** @@ -180,4 +186,59 @@ protected function setHeaders(array $headers) return $this; } + + /** + * @param string $name + * + * @return string Normalized header name + */ + protected function normalizeHeaderName($name): string + { + try { + $name = str_replace(["\r", "\n", "\t"], '', trim($name)); + } catch (Throwable $e) { + throw new InvalidArgumentException( + sprintf('Header name must be a string, %s given', gettype($name)), StatusCode::BAD_REQUEST + ); + } + + if ('' === $name) { + throw new InvalidArgumentException('Empty header name', StatusCode::BAD_REQUEST); + } + + return $name; + } + + /** + * @param string $name + * @param string|string[] $value + * + * @return array + */ + protected function normalizeHeaderValue(string $name, $value): array + { + $type = gettype($value); + switch ($type) { + case 'array': + case 'integer': + case 'double': + case 'string': + $value = (array)$value; + break; + default: + throw new InvalidArgumentException( + sprintf('Invalid header value, expects string or array, "%s" given', $type), StatusCode::BAD_REQUEST + ); + } + + if (empty($value = array_map(function($v) { + return trim(preg_replace('/\s+/', ' ', $v)); + }, $value))) { + throw new InvalidArgumentException( + sprintf('The value for header "%s" cannot be empty', $name), StatusCode::BAD_REQUEST + ); + } + + return $value; + } } diff --git a/Tests/HeaderTraitTest.php b/Tests/HeaderTraitTest.php index 3d223ef..a602a57 100644 --- a/Tests/HeaderTraitTest.php +++ b/Tests/HeaderTraitTest.php @@ -174,14 +174,16 @@ public function test_normalizing_headers_key_and_value() { $this->SUT = $this->SUT->withHeaders([ "HTTP/1.1 401 Authorization Required\r\n" => "\r\n", - "cache-control\r\n" => " no-cache, no-store, must-revalidate, pre-check=0, post-check=0\r\n", - "x-xss-protection\r\n" => "0 \r\n" + "cache-control\n" => " no-cache, no-store, must-revalidate, pre-check=0, post-check=0\r\n", + "x-xss-protection\r\n" => "0 \r\n", + " Nasty-\tHeader-\r\nName" => "weird\nvalue\r", ]); $this->assertSame([ 'Http/1.1 401 authorization required' => [''], 'Cache-Control' => ['no-cache, no-store, must-revalidate, pre-check=0, post-check=0'], - 'X-Xss-Protection' => ['0'] + 'X-Xss-Protection' => ['0'], + "Nasty-Header-Name" => ["weird value"], ], $this->SUT->getHeaders()); } diff --git a/Tests/Integration/RequestIntegrationTest.php b/Tests/Integration/RequestIntegrationTest.php index 2bc8704..3044925 100644 --- a/Tests/Integration/RequestIntegrationTest.php +++ b/Tests/Integration/RequestIntegrationTest.php @@ -6,18 +6,26 @@ class RequestIntegrationTest extends \Http\Psr7Test\RequestIntegrationTest { - protected $skippedTests = [ 'testUri' => 'Skipped because of the host requirement', 'testMethod' => 'Implementation uses constants where capitalization matters', 'testMethodWithInvalidArguments' => 'Does not make sense for strict type implementation', - 'testWithHeaderInvalidArguments' => 'Does not make sense for strict type implementation', - 'testWithAddedHeaderInvalidArguments' => 'Does not make sense for strict type implementation', - 'testWithAddedHeaderArrayValueAndKeys' => 'Skipped, the test is weird', - 'testWithAddedHeader' => 'Skipped, the test is weird', 'testUriPreserveHost_NoHost_Host' => 'Skipped because of the host requirement', ]; + /** + * @overridden The header is not merged as the test authors think it should + */ + public function testWithAddedHeaderArrayValueAndKeys() + { + $message = $this->getMessage()->withAddedHeader('content-type', ['foo' => 'text/html']); + $message = $message->withAddedHeader('content-type', ['foo' => 'text/plain', 'bar' => 'application/json']); + $headerLine = $message->getHeaderLine('content-type'); + + $this->assertRegExp('|text/plain|', $headerLine); + $this->assertRegExp('|application/json|', $headerLine); + } + /** * @return RequestInterface that is used in the tests */ @@ -26,5 +34,4 @@ public function createSubject() unset($_SERVER['HTTP_HOST']); return new ClientRequest('GET', ''); } - }