- improved header methods (PSR-7)

- unit tests updated
kodedphp · Mar 26, 2020 · c9d5c42 · c9d5c42
1 parent 9258440
commit c9d5c42
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 20 deletions.
diff --git 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,22 +155,22 @@ 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)), '-');
         }
 
         $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
@@ -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
@@ -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', '');
     }
-
 }