Browse Source

Deprecate Exception::responseHeader() in favor of HttpException

Corey Taylor 5 years ago
parent
commit
7013b5ed8e

+ 1 - 1
src/Auth/BasicAuthenticate.php

@@ -95,7 +95,7 @@ class BasicAuthenticate extends BaseAuthenticate
     public function unauthenticated(ServerRequest $request, Response $response)
     {
         $unauthorizedException = new UnauthorizedException();
-        $unauthorizedException->responseHeader($this->loginHeaders($request));
+        $unauthorizedException->setHeaders($this->loginHeaders($request));
 
         throw $unauthorizedException;
     }

+ 8 - 0
src/Core/Exception/Exception.php

@@ -92,12 +92,20 @@ class Exception extends RuntimeException
      *   array of "header name" => "header value"
      * @param string|null $value The header value.
      * @return array|null
+     * @deprecated 4.2.0 Use `HttpException::setHeaders()` instead. Response headers
+     *   should be set for HttpException only.
      */
     public function responseHeader($header = null, $value = null): ?array
     {
         if ($header === null) {
             return $this->_responseHeaders;
         }
+
+        deprecationWarning(
+            'Setting HTTP response headers from Exception directly is deprecated. ' .
+            'If your exceptions extend Exception, they must now extend HttpException. ' .
+            'You should only set HTTP headers on HttpException instances via the `setHeaders()` method.'
+        );
         if (is_array($header)) {
             return $this->_responseHeaders = $header;
         }

+ 6 - 0
src/Error/ExceptionRenderer.php

@@ -230,10 +230,16 @@ class ExceptionRenderer implements ExceptionRendererInterface
         $response = $this->controller->getResponse();
 
         if ($exception instanceof CakeException) {
+            /** @psalm-suppress DeprecatedMethod */
             foreach ((array)$exception->responseHeader() as $key => $value) {
                 $response = $response->withHeader($key, $value);
             }
         }
+        if ($exception instanceof HttpException) {
+            foreach ($exception->getHeaders() as $name => $value) {
+                $response = $response->withHeader($name, $value);
+            }
+        }
         $response = $response->withStatus($code);
 
         $viewVars = [

+ 38 - 0
src/Http/Exception/HttpException.php

@@ -30,4 +30,42 @@ class HttpException extends Exception
      * @inheritDoc
      */
     protected $_defaultCode = 500;
+
+    /**
+     * @var array
+     */
+    protected $headers = [];
+
+    /**
+     * Set a single HTTP response header.
+     *
+     * @param string $header Header name
+     * @param string|string[]|null $value Header value
+     * @return void
+     */
+    public function setHeader(string $header, $value = null): void
+    {
+        $this->headers[$header] = $value;
+    }
+
+    /**
+     * Sets HTTP response headers.
+     *
+     * @param array $headers Array of header name and value pairs.
+     * @return void
+     */
+    public function setHeaders(array $headers): void
+    {
+        $this->headers = $headers;
+    }
+
+    /**
+     * Returns array of response headers.
+     *
+     * @return array
+     */
+    public function getHeaders(): array
+    {
+        return $this->headers;
+    }
 }

+ 1 - 1
src/Http/Middleware/CsrfProtectionMiddleware.php

@@ -292,7 +292,7 @@ class CsrfProtectionMiddleware implements MiddlewareInterface
             $exception = new InvalidCsrfTokenException(__d('cake', 'Missing or invalid CSRF cookie.'));
 
             $expiredCookie = $this->_createCookie('', $request)->withExpired();
-            $exception->responseHeader('Set-Cookie', $expiredCookie->toHeaderValue());
+            $exception->setHeader('Set-Cookie', $expiredCookie->toHeaderValue());
 
             throw $exception;
         }

+ 1 - 1
src/Http/ServerRequest.php

@@ -1507,7 +1507,7 @@ class ServerRequest implements ServerRequestInterface
         }
         $allowed = strtoupper(implode(', ', $methods));
         $e = new MethodNotAllowedException();
-        $e->responseHeader('Allow', $allowed);
+        $e->setHeader('Allow', $allowed);
         throw $e;
     }
 

+ 1 - 1
tests/TestCase/Auth/BasicAuthenticateTest.php

@@ -191,7 +191,7 @@ class BasicAuthenticateTest extends TestCase
         $this->assertNotEmpty($e);
 
         $expected = ['WWW-Authenticate' => 'Basic realm="localhost"'];
-        $this->assertEquals($expected, $e->responseHeader());
+        $this->assertEquals($expected, $e->getHeaders());
     }
 
     /**

+ 2 - 2
tests/TestCase/Auth/DigestAuthenticateTest.php

@@ -150,7 +150,7 @@ class DigestAuthenticateTest extends TestCase
 
         $this->assertNotEmpty($e);
 
-        $header = $e->responseHeader();
+        $header = $e->getHeaders();
         $this->assertMatchesRegularExpression(
             '/^Digest realm="localhost",qop="auth",nonce="[a-zA-Z0-9=]+",opaque="123abc"$/',
             $header['WWW-Authenticate']
@@ -184,7 +184,7 @@ class DigestAuthenticateTest extends TestCase
         }
         $this->assertNotEmpty($e);
 
-        $header = $e->responseHeader()['WWW-Authenticate'];
+        $header = $e->getHeaders()['WWW-Authenticate'];
         $this->assertStringContainsString('stale=true', $header);
     }
 

+ 26 - 2
tests/TestCase/Error/ExceptionRendererTest.php

@@ -451,12 +451,36 @@ class ExceptionRendererTest extends TestCase
     public function testExceptionResponseHeader()
     {
         $exception = new MethodNotAllowedException('Only allowing POST and DELETE');
-        $exception->responseHeader(['Allow' => 'POST, DELETE']);
+        $exception->setHeader('Allow', ['POST', 'DELETE']);
         $ExceptionRenderer = new ExceptionRenderer($exception);
 
         $result = $ExceptionRenderer->render();
         $this->assertTrue($result->hasHeader('Allow'));
-        $this->assertSame('POST, DELETE', $result->getHeaderLine('Allow'));
+        $this->assertSame('POST,DELETE', $result->getHeaderLine('Allow'));
+
+        $exception->setHeaders(['Allow' => 'GET']);
+        $result = $ExceptionRenderer->render();
+        $this->assertTrue($result->hasHeader('Allow'));
+        $this->assertSame('GET', $result->getHeaderLine('Allow'));
+    }
+
+    /**
+     * Tests setting exception response headers through core Exception
+     *
+     * @return void
+     */
+    public function testExceptionDeprecatedResponseHeader()
+    {
+        $this->deprecated(function () {
+            $exception = new CakeException('Should Not Set Headers');
+            $exception->responseHeader(['Allow' => 'POST, DELETE']);
+
+            $ExceptionRenderer = new ExceptionRenderer($exception);
+
+            $result = $ExceptionRenderer->render();
+            $this->assertTrue($result->hasHeader('Allow'));
+            $this->assertSame('POST, DELETE', $result->getHeaderLine('Allow'));
+        });
     }
 
     /**

+ 3 - 3
tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php

@@ -232,7 +232,7 @@ class CsrfProtectionMiddlewareTest extends TestCase
 
             $this->fail();
         } catch (InvalidCsrfTokenException $exception) {
-            $responseHeaders = $exception->responseHeader();
+            $responseHeaders = $exception->getHeaders();
 
             $this->assertArrayHasKey('Set-Cookie', $responseHeaders);
 
@@ -313,7 +313,7 @@ class CsrfProtectionMiddlewareTest extends TestCase
 
             $this->fail();
         } catch (InvalidCsrfTokenException $exception) {
-            $responseHeaders = $exception->responseHeader();
+            $responseHeaders = $exception->getHeaders();
             $this->assertArrayHasKey('Set-Cookie', $responseHeaders);
 
             $cookie = Cookie::createFromHeaderString($responseHeaders['Set-Cookie']);
@@ -386,7 +386,7 @@ class CsrfProtectionMiddlewareTest extends TestCase
 
             $this->fail();
         } catch (InvalidCsrfTokenException $exception) {
-            $responseHeaders = $exception->responseHeader();
+            $responseHeaders = $exception->getHeaders();
             $this->assertEmpty($responseHeaders, 'Should not send any header');
         }
     }

+ 1 - 1
tests/TestCase/Http/ServerRequestTest.php

@@ -1833,7 +1833,7 @@ XML;
             $request->allowMethod(['POST', 'DELETE']);
             $this->fail('An expected exception has not been raised.');
         } catch (MethodNotAllowedException $e) {
-            $this->assertEquals(['Allow' => 'POST, DELETE'], $e->responseHeader());
+            $this->assertEquals(['Allow' => 'POST, DELETE'], $e->getHeaders());
         }
 
         $this->expectException(MethodNotAllowedException::class);