Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[HttpKernel] Deprecate X-Status-Code for better alternative #19822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 1, 2017
Merged

[HttpKernel] Deprecate X-Status-Code for better alternative #19822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions 5 UPGRADE-3.3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ HttpKernel

* The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array of pools indexed
by name to the constructor instead.

* 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
-------
Expand Down
5 changes: 5 additions & 0 deletions 5 UPGRADE-4.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ HttpKernel

* The `Psr6CacheClearer::addPool()` method has been removed. Pass an array of pools indexed
by name to the constructor instead.

* 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
-------
Expand Down
23 changes: 23 additions & 0 deletions 23 src/Symfony/Component/HttpKernel/Event/GetResponseForExceptionEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ class GetResponseForExceptionEvent extends GetResponseEvent
*/
private $exception;

/**
* @var bool
*/
private $allowCustomResponseCode = false;

public function __construct(HttpKernelInterface $kernel, Request $request, $requestType, \Exception $e)
{
parent::__construct($kernel, $request, $requestType);
Expand Down Expand Up @@ -64,4 +69,22 @@ public function setException(\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;
}
}
4 changes: 3 additions & 1 deletion 4 src/Symfony/Component/HttpKernel/HttpKernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,12 @@ private function handleException(\Exception $e, $request, $type)

// the developer asked for a specific 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->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
if ($e instanceof HttpExceptionInterface) {
// keep the HTTP status code and headers
Expand Down
27 changes: 27 additions & 0 deletions 27 src/Symfony/Component/HttpKernel/Tests/Event/GetResponseForExceptionEventTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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());
}
}
30 changes: 29 additions & 1 deletion 30 src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface;
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
use Symfony\Component\HttpKernel\Event\FilterControllerArgumentsEvent;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
Expand Down Expand Up @@ -111,9 +112,10 @@ public function testHandleHttpException()
}

/**
* @group legacy
* @dataProvider getStatusCodes
*/
public function testHandleWhenAnExceptionIsHandledWithASpecificStatusCode($responseStatusCode, $expectedStatusCode)
public function testLegacyHandleWhenAnExceptionIsHandledWithASpecificStatusCode($responseStatusCode, $expectedStatusCode)
{
$dispatcher = new EventDispatcher();
$dispatcher->addListener(KernelEvents::EXCEPTION, function ($event) use ($responseStatusCode, $expectedStatusCode) {
Expand All @@ -137,6 +139,32 @@ public function getStatusCodes()
);
}

/**
* @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()
{
$dispatcher = new EventDispatcher();
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Security/Http/EntryPoint/FormAuthenticationEntryPoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function start(Request $request, AuthenticationException $authException =

$response = $this->httpKernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST);
if (200 === $response->getStatusCode()) {
$response->headers->set('X-Status-Code', 401);
$response->setStatusCode(401);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot I'm not sure about this change, in the authentication listeners found in the Symfony\Component\Security\Http\Firewall namespace the response returned from the AuthenticationEntryPointInterface::start() method is set on the response. From what I can see the X-Status-Code is only ever used when handling the exception in the kernel, so I don't think this will have any adverse impact

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot interested on hearing your thoughts on the above? would be good to get this into 3.2 before the development window closes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks suspicious to me. You are saying that this can basically be removed, right? If that's the case, we probably need to understand what changed between the time the PR adding this was merged and now.

}

return $response;
Expand Down
2 changes: 2 additions & 0 deletions 2 src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ private function handleAuthenticationException(GetResponseForExceptionEvent $eve

try {
$event->setResponse($this->startAuthentication($event->getRequest(), $exception));
$event->allowCustomResponseCode();
} catch (\Exception $e) {
$event->setException($e);
}
Expand Down Expand Up @@ -155,6 +156,7 @@ private function handleAccessDeniedException(GetResponseForExceptionEvent $event
$subRequest->attributes->set(Security::ACCESS_DENIED_ERROR, $exception);

$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
$event->allowCustomResponseCode();
}
} catch (\Exception $e) {
if (null !== $this->logger) {
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Security/Http/Tests/EntryPoint/FormAuthenticationEntryPointTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ public function testStartWithUseForward()
$entryPointResponse = $entryPoint->start($request);

$this->assertEquals($response, $entryPointResponse);
$this->assertEquals(401, $entryPointResponse->headers->get('X-Status-Code'));
$this->assertEquals(401, $entryPointResponse->getStatusCode());
}
}
24 changes: 16 additions & 8 deletions 24 src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,19 @@ public function testAuthenticationExceptionWithoutEntryPoint(\Exception $excepti
/**
* @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);

$response = new Response('Forbidden', 403);

$listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint());
$listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint($response));
$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());
}

Expand Down Expand Up @@ -100,7 +105,7 @@ public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandle
public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
{
$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);

Expand All @@ -110,7 +115,10 @@ public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandle
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true), $httpUtils, null, '/error');
$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());
}

Expand Down Expand Up @@ -159,10 +167,10 @@ public function getAccessDeniedExceptionProvider()
);
}

private function createEntryPoint()
private function createEntryPoint(Response $response = null)
{
$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;
}
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Security/Http/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"symfony/security-core": "~3.2",
"symfony/event-dispatcher": "~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-php70": "~1.0",
"symfony/property-access": "~2.8|~3.0"
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/Security/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"php": ">=5.5.9",
"symfony/event-dispatcher": "~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-php70": "~1.0",
"symfony/polyfill-util": "~1.0",
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.