feature #19822 [HttpKernel] Deprecate X-Status-Code for better alternative (jameshalsall)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[HttpKernel] Deprecate X-Status-Code for better alternative
| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #12343 |
| License | MIT |
| Doc PR | https://github.com/symfony/symfony-docs/pull/6948 |
This marks the X-Status-Code header method of setting a custom response status
code in exception listeners for a better alternative. There is now a new method
on the `GetResponseForExceptionEvent` that allows successful status codes in
the response sent to the client.
The old method of setting the X-Status-Code header will now throw a deprecation warning.
Instead, in your exception listener you simply call `GetResponseForExceptionEvent::allowCustomResponseCode()` which will tell the Kernel not to override the status code of the event's response object.
Currenty the `X-Status-Code` header will still be removed, so as not to change the existing behaviour, but this is something we can remove in 4.0.
TODO:
- [x] Replace usage of X-Status-Code in `FormAuthenticationEntryPoint`
- [x] Open Silex issue
- [x] Rename method on the response
- [x] Ensure correct response code is set in `AuthenticationEntryPointInterface` implementations
- [x] Ensure the exception listeners are marking `GetResponseForExceptionEvent` as allowing a custom response code
- [x] In the Security component we should only use the new method of setting a custom response code if it is available, and fall back to the `X-Status-Code` method
Commits
-------
cc0ef282cd
[HttpKernel] Deprecate X-Status-Code for better alternative
This commit is contained in:
commit
28a00dac0c
@ -139,15 +139,20 @@ FrameworkBundle
|
|||||||
deprecated and will be removed in 4.0. Use `Symfony\Component\Config\DependencyInjection\ConfigCachePass`
|
deprecated and will be removed in 4.0. Use `Symfony\Component\Config\DependencyInjection\ConfigCachePass`
|
||||||
class instead.
|
class instead.
|
||||||
|
|
||||||
|
|
||||||
HttpKernel
|
HttpKernel
|
||||||
-----------
|
-----------
|
||||||
|
|
||||||
* The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array of pools indexed
|
* The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array
|
||||||
by name to the constructor instead.
|
of pools indexed by name to the constructor instead.
|
||||||
|
|
||||||
* The `LazyLoadingFragmentHandler::addRendererService()` method has been deprecated and
|
* The `LazyLoadingFragmentHandler::addRendererService()` method has been
|
||||||
will be removed in 4.0.
|
deprecated and will be removed in 4.0.
|
||||||
|
|
||||||
|
* The `X-Status-Code` header method of setting a custom status code in the
|
||||||
|
response when handling exceptions has been removed. There is now a new
|
||||||
|
`GetResponseForExceptionEvent::allowCustomResponseCode()` method instead,
|
||||||
|
which will tell the Kernel to use the response code set on the event's
|
||||||
|
response object.
|
||||||
|
|
||||||
Process
|
Process
|
||||||
-------
|
-------
|
||||||
|
@ -243,6 +243,12 @@ HttpKernel
|
|||||||
|
|
||||||
* The `LazyLoadingFragmentHandler::addRendererService()` method has been removed.
|
* The `LazyLoadingFragmentHandler::addRendererService()` method has been removed.
|
||||||
|
|
||||||
|
* The `X-Status-Code` header method of setting a custom status code in the
|
||||||
|
response when handling exceptions has been removed. There is now a new
|
||||||
|
`GetResponseForExceptionEvent::allowCustomResponseCode()` method instead,
|
||||||
|
which will tell the Kernel to use the response code set on the event's
|
||||||
|
response object.
|
||||||
|
|
||||||
Ldap
|
Ldap
|
||||||
----
|
----
|
||||||
|
|
||||||
|
@ -36,6 +36,11 @@ class GetResponseForExceptionEvent extends GetResponseEvent
|
|||||||
*/
|
*/
|
||||||
private $exception;
|
private $exception;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @var bool
|
||||||
|
*/
|
||||||
|
private $allowCustomResponseCode = false;
|
||||||
|
|
||||||
public function __construct(HttpKernelInterface $kernel, Request $request, $requestType, \Exception $e)
|
public function __construct(HttpKernelInterface $kernel, Request $request, $requestType, \Exception $e)
|
||||||
{
|
{
|
||||||
parent::__construct($kernel, $request, $requestType);
|
parent::__construct($kernel, $request, $requestType);
|
||||||
@ -64,4 +69,22 @@ class GetResponseForExceptionEvent extends GetResponseEvent
|
|||||||
{
|
{
|
||||||
$this->exception = $exception;
|
$this->exception = $exception;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mark the event as allowing a custom response code.
|
||||||
|
*/
|
||||||
|
public function allowCustomResponseCode()
|
||||||
|
{
|
||||||
|
$this->allowCustomResponseCode = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns true if the event allows a custom response code.
|
||||||
|
*
|
||||||
|
* @return bool
|
||||||
|
*/
|
||||||
|
public function isAllowingCustomResponseCode()
|
||||||
|
{
|
||||||
|
return $this->allowCustomResponseCode;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -242,10 +242,12 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface
|
|||||||
|
|
||||||
// the developer asked for a specific status code
|
// the developer asked for a specific status code
|
||||||
if ($response->headers->has('X-Status-Code')) {
|
if ($response->headers->has('X-Status-Code')) {
|
||||||
|
@trigger_error(sprintf('Using the X-Status-Code header is deprecated since version 3.3 and will be removed in 4.0. Use %s::allowCustomResponseCode() instead.', GetResponseForExceptionEvent::class), E_USER_DEPRECATED);
|
||||||
|
|
||||||
$response->setStatusCode($response->headers->get('X-Status-Code'));
|
$response->setStatusCode($response->headers->get('X-Status-Code'));
|
||||||
|
|
||||||
$response->headers->remove('X-Status-Code');
|
$response->headers->remove('X-Status-Code');
|
||||||
} elseif (!$response->isClientError() && !$response->isServerError() && !$response->isRedirect()) {
|
} elseif (!$event->isAllowingCustomResponseCode() && !$response->isClientError() && !$response->isServerError() && !$response->isRedirect()) {
|
||||||
// ensure that we actually have an error response
|
// ensure that we actually have an error response
|
||||||
if ($e instanceof HttpExceptionInterface) {
|
if ($e instanceof HttpExceptionInterface) {
|
||||||
// keep the HTTP status code and headers
|
// keep the HTTP status code and headers
|
||||||
|
@ -0,0 +1,27 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
/*
|
||||||
|
* This file is part of the Symfony package.
|
||||||
|
*
|
||||||
|
* (c) Fabien Potencier <fabien@symfony.com>
|
||||||
|
*
|
||||||
|
* For the full copyright and license information, please view the LICENSE
|
||||||
|
* file that was distributed with this source code.
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace Symfony\Component\HttpKernel\Tests\Event;
|
||||||
|
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Component\HttpFoundation\Request;
|
||||||
|
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
|
||||||
|
use Symfony\Component\HttpKernel\Tests\TestHttpKernel;
|
||||||
|
|
||||||
|
class GetResponseForExceptionEventTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testAllowSuccessfulResponseIsFalseByDefault()
|
||||||
|
{
|
||||||
|
$event = new GetResponseForExceptionEvent(new TestHttpKernel(), new Request(), 1, new \Exception());
|
||||||
|
|
||||||
|
$this->assertFalse($event->isAllowingCustomResponseCode());
|
||||||
|
}
|
||||||
|
}
|
@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\RequestStack;
|
|||||||
use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface;
|
use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface;
|
||||||
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
|
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
|
||||||
use Symfony\Component\HttpKernel\Event\FilterControllerArgumentsEvent;
|
use Symfony\Component\HttpKernel\Event\FilterControllerArgumentsEvent;
|
||||||
|
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
|
||||||
use Symfony\Component\HttpKernel\HttpKernel;
|
use Symfony\Component\HttpKernel\HttpKernel;
|
||||||
use Symfony\Component\HttpKernel\HttpKernelInterface;
|
use Symfony\Component\HttpKernel\HttpKernelInterface;
|
||||||
use Symfony\Component\HttpKernel\KernelEvents;
|
use Symfony\Component\HttpKernel\KernelEvents;
|
||||||
@ -111,9 +112,10 @@ class HttpKernelTest extends TestCase
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
* @group legacy
|
||||||
* @dataProvider getStatusCodes
|
* @dataProvider getStatusCodes
|
||||||
*/
|
*/
|
||||||
public function testHandleWhenAnExceptionIsHandledWithASpecificStatusCode($responseStatusCode, $expectedStatusCode)
|
public function testLegacyHandleWhenAnExceptionIsHandledWithASpecificStatusCode($responseStatusCode, $expectedStatusCode)
|
||||||
{
|
{
|
||||||
$dispatcher = new EventDispatcher();
|
$dispatcher = new EventDispatcher();
|
||||||
$dispatcher->addListener(KernelEvents::EXCEPTION, function ($event) use ($responseStatusCode, $expectedStatusCode) {
|
$dispatcher->addListener(KernelEvents::EXCEPTION, function ($event) use ($responseStatusCode, $expectedStatusCode) {
|
||||||
@ -137,6 +139,32 @@ class HttpKernelTest extends TestCase
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider getSpecificStatusCodes
|
||||||
|
*/
|
||||||
|
public function testHandleWhenAnExceptionIsHandledWithASpecificStatusCode($expectedStatusCode)
|
||||||
|
{
|
||||||
|
$dispatcher = new EventDispatcher();
|
||||||
|
$dispatcher->addListener(KernelEvents::EXCEPTION, function (GetResponseForExceptionEvent $event) use ($expectedStatusCode) {
|
||||||
|
$event->allowCustomResponseCode();
|
||||||
|
$event->setResponse(new Response('', $expectedStatusCode));
|
||||||
|
});
|
||||||
|
|
||||||
|
$kernel = $this->getHttpKernel($dispatcher, function () { throw new \RuntimeException(); });
|
||||||
|
$response = $kernel->handle(new Request());
|
||||||
|
|
||||||
|
$this->assertEquals($expectedStatusCode, $response->getStatusCode());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getSpecificStatusCodes()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
array(200),
|
||||||
|
array(302),
|
||||||
|
array(403),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
public function testHandleWhenAListenerReturnsAResponse()
|
public function testHandleWhenAListenerReturnsAResponse()
|
||||||
{
|
{
|
||||||
$dispatcher = new EventDispatcher();
|
$dispatcher = new EventDispatcher();
|
||||||
|
@ -54,7 +54,7 @@ class FormAuthenticationEntryPoint implements AuthenticationEntryPointInterface
|
|||||||
|
|
||||||
$response = $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
|
$response = $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
|
||||||
if (200 === $response->getStatusCode()) {
|
if (200 === $response->getStatusCode()) {
|
||||||
$response->headers->set('X-Status-Code', 401);
|
$response->setStatusCode(401);
|
||||||
}
|
}
|
||||||
|
|
||||||
return $response;
|
return $response;
|
||||||
|
@ -112,6 +112,7 @@ class ExceptionListener
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
$event->setResponse($this->startAuthentication($event->getRequest(), $exception));
|
$event->setResponse($this->startAuthentication($event->getRequest(), $exception));
|
||||||
|
$event->allowCustomResponseCode();
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
$event->setException($e);
|
$event->setException($e);
|
||||||
}
|
}
|
||||||
@ -155,6 +156,7 @@ class ExceptionListener
|
|||||||
$subRequest->attributes->set(Security::ACCESS_DENIED_ERROR, $exception);
|
$subRequest->attributes->set(Security::ACCESS_DENIED_ERROR, $exception);
|
||||||
|
|
||||||
$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
|
$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
|
||||||
|
$event->allowCustomResponseCode();
|
||||||
}
|
}
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
if (null !== $this->logger) {
|
if (null !== $this->logger) {
|
||||||
|
@ -64,6 +64,6 @@ class FormAuthenticationEntryPointTest extends TestCase
|
|||||||
$entryPointResponse = $entryPoint->start($request);
|
$entryPointResponse = $entryPoint->start($request);
|
||||||
|
|
||||||
$this->assertEquals($response, $entryPointResponse);
|
$this->assertEquals($response, $entryPointResponse);
|
||||||
$this->assertEquals(401, $entryPointResponse->headers->get('X-Status-Code'));
|
$this->assertEquals(401, $entryPointResponse->getStatusCode());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -44,14 +44,19 @@ class ExceptionListenerTest extends TestCase
|
|||||||
/**
|
/**
|
||||||
* @dataProvider getAuthenticationExceptionProvider
|
* @dataProvider getAuthenticationExceptionProvider
|
||||||
*/
|
*/
|
||||||
public function testAuthenticationExceptionWithEntryPoint(\Exception $exception, \Exception $eventException = null)
|
public function testAuthenticationExceptionWithEntryPoint(\Exception $exception)
|
||||||
{
|
{
|
||||||
$event = $this->createEvent($exception = new AuthenticationException());
|
$event = $this->createEvent($exception);
|
||||||
|
|
||||||
$listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint());
|
$response = new Response('Forbidden', 403);
|
||||||
|
|
||||||
|
$listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint($response));
|
||||||
$listener->onKernelException($event);
|
$listener->onKernelException($event);
|
||||||
|
|
||||||
$this->assertEquals('OK', $event->getResponse()->getContent());
|
$this->assertTrue($event->isAllowingCustomResponseCode());
|
||||||
|
|
||||||
|
$this->assertEquals('Forbidden', $event->getResponse()->getContent());
|
||||||
|
$this->assertEquals(403, $event->getResponse()->getStatusCode());
|
||||||
$this->assertSame($exception, $event->getException());
|
$this->assertSame($exception, $event->getException());
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -100,7 +105,7 @@ class ExceptionListenerTest extends TestCase
|
|||||||
public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
|
public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
|
||||||
{
|
{
|
||||||
$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock();
|
$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock();
|
||||||
$kernel->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
|
$kernel->expects($this->once())->method('handle')->will($this->returnValue(new Response('Unauthorized', 401)));
|
||||||
|
|
||||||
$event = $this->createEvent($exception, $kernel);
|
$event = $this->createEvent($exception, $kernel);
|
||||||
|
|
||||||
@ -110,7 +115,10 @@ class ExceptionListenerTest extends TestCase
|
|||||||
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true), $httpUtils, null, '/error');
|
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true), $httpUtils, null, '/error');
|
||||||
$listener->onKernelException($event);
|
$listener->onKernelException($event);
|
||||||
|
|
||||||
$this->assertEquals('error', $event->getResponse()->getContent());
|
$this->assertTrue($event->isAllowingCustomResponseCode());
|
||||||
|
|
||||||
|
$this->assertEquals('Unauthorized', $event->getResponse()->getContent());
|
||||||
|
$this->assertEquals(401, $event->getResponse()->getStatusCode());
|
||||||
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
|
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -159,10 +167,10 @@ class ExceptionListenerTest extends TestCase
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
private function createEntryPoint()
|
private function createEntryPoint(Response $response = null)
|
||||||
{
|
{
|
||||||
$entryPoint = $this->getMockBuilder('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface')->getMock();
|
$entryPoint = $this->getMockBuilder('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface')->getMock();
|
||||||
$entryPoint->expects($this->once())->method('start')->will($this->returnValue(new Response('OK')));
|
$entryPoint->expects($this->once())->method('start')->will($this->returnValue($response ?: new Response('OK')));
|
||||||
|
|
||||||
return $entryPoint;
|
return $entryPoint;
|
||||||
}
|
}
|
||||||
|
@ -20,7 +20,7 @@
|
|||||||
"symfony/security-core": "~3.2",
|
"symfony/security-core": "~3.2",
|
||||||
"symfony/event-dispatcher": "~2.8|~3.0",
|
"symfony/event-dispatcher": "~2.8|~3.0",
|
||||||
"symfony/http-foundation": "~2.8|~3.0",
|
"symfony/http-foundation": "~2.8|~3.0",
|
||||||
"symfony/http-kernel": "~2.8|~3.0",
|
"symfony/http-kernel": "~3.3",
|
||||||
"symfony/polyfill-php56": "~1.0",
|
"symfony/polyfill-php56": "~1.0",
|
||||||
"symfony/polyfill-php70": "~1.0",
|
"symfony/polyfill-php70": "~1.0",
|
||||||
"symfony/property-access": "~2.8|~3.0"
|
"symfony/property-access": "~2.8|~3.0"
|
||||||
|
@ -19,7 +19,7 @@
|
|||||||
"php": ">=5.5.9",
|
"php": ">=5.5.9",
|
||||||
"symfony/event-dispatcher": "~2.8|~3.0",
|
"symfony/event-dispatcher": "~2.8|~3.0",
|
||||||
"symfony/http-foundation": "~2.8|~3.0",
|
"symfony/http-foundation": "~2.8|~3.0",
|
||||||
"symfony/http-kernel": "~2.8|~3.0",
|
"symfony/http-kernel": "~3.3",
|
||||||
"symfony/polyfill-php56": "~1.0",
|
"symfony/polyfill-php56": "~1.0",
|
||||||
"symfony/polyfill-php70": "~1.0",
|
"symfony/polyfill-php70": "~1.0",
|
||||||
"symfony/polyfill-util": "~1.0",
|
"symfony/polyfill-util": "~1.0",
|
||||||
|
Reference in New Issue
Block a user