Skip to content

[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController #26213

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

Closed
ZipoKing wants to merge 4 commits into from
Closed

[FrameworkBundle] Add support to 307/308 HTTP status codes in RedirectController #26213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4265fb
Issue #26171 - allow to use 307/308 HTTP response codes
ZipoKing Feb 18, 2018
b955a6f
Changelog update, remove deprecation notes
ZipoKing Feb 18, 2018
6aad61c
fabbot-guided changes
ZipoKing Feb 18, 2018
103c122
Use direct HTTP status codes instead of constants
ZipoKing Feb 18, 2018
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
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ CHANGELOG
* Added option in workflow dump command to label graph with a custom label
* Using a `RouterInterface` that does not implement the `WarmableInterface` is deprecated and will not be supported in Symfony 5.0.
* The `RequestDataCollector` class has been deprecated and will be removed in Symfony 5.0. Use the `Symfony\Component\HttpKernel\DataCollector\RequestDataCollector` class instead.
* The `RedirectController` class allows for 307/308 HTTP status codes

4.0.0
-----
Expand Down
42 changes: 27 additions & 15 deletions src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ public function __construct(UrlGeneratorInterface $router = null, int $httpPort
* In case the route name is empty, the status code will be 404 when permanent is false
* and 410 otherwise.
*
* @param Request $request The request instance
* @param string $route The route name to redirect to
* @param bool $permanent Whether the redirection is permanent
* @param bool|array $ignoreAttributes Whether to ignore attributes or an array of attributes to ignore
* @param Request $request The request instance
* @param string $route The route name to redirect to
* @param bool $permanent Whether the redirection is permanent
* @param bool|array $ignoreAttributes Whether to ignore attributes or an array of attributes to ignore
* @param bool $keepRequestMethod Wheter redirect action should keep HTTP request method
*
* @throws HttpException In case the route name is empty
*/
public function redirectAction(Request $request, string $route, bool $permanent = false, $ignoreAttributes = false): Response
public function redirectAction(Request $request, string $route, bool $permanent = false, $ignoreAttributes = false, bool $keepRequestMethod = false): Response
Copy link
Member

@nicolas-grekas nicolas-grekas Feb 18, 2018

Choose a reason for hiding this comment

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

for other reviewers: the class is final, so this is not a BC break

{
if ('' == $route) {
throw new HttpException($permanent ? 410 : 404);
Expand All @@ -62,13 +63,19 @@ public function redirectAction(Request $request, string $route, bool $permanent
$attributes = array();
if (false === $ignoreAttributes || is_array($ignoreAttributes)) {
$attributes = $request->attributes->get('_route_params');
unset($attributes['route'], $attributes['permanent'], $attributes['ignoreAttributes']);
unset($attributes['route'], $attributes['permanent'], $attributes['ignoreAttributes'], $attributes['keepRequestMethod']);
if ($ignoreAttributes) {
$attributes = array_diff_key($attributes, array_flip($ignoreAttributes));
}
}

return new RedirectResponse($this->router->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);
if ($keepRequestMethod) {
$statusCode = $permanent ? 308 : 307;
} else {
$statusCode = $permanent ? 301 : 302;
}

return new RedirectResponse($this->router->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $statusCode);
}

/**
Expand All @@ -80,22 +87,27 @@ public function redirectAction(Request $request, string $route, bool $permanent
* In case the path is empty, the status code will be 404 when permanent is false
* and 410 otherwise.
*
* @param Request $request The request instance
* @param string $path The absolute path or URL to redirect to
* @param bool $permanent Whether the redirect is permanent or not
* @param string|null $scheme The URL scheme (null to keep the current one)
* @param int|null $httpPort The HTTP port (null to keep the current one for the same scheme or the default configured port)
* @param int|null $httpsPort The HTTPS port (null to keep the current one for the same scheme or the default configured port)
* @param Request $request The request instance
* @param string $path The absolute path or URL to redirect to
* @param bool $permanent Whether the redirect is permanent or not
* @param string|null $scheme The URL scheme (null to keep the current one)
* @param int|null $httpPort The HTTP port (null to keep the current one for the same scheme or the default configured port)
* @param int|null $httpsPort The HTTPS port (null to keep the current one for the same scheme or the default configured port)
* @param bool $keepRequestMethod Wheter redirect action should keep HTTP request method
*
* @throws HttpException In case the path is empty
*/
public function urlRedirectAction(Request $request, string $path, bool $permanent = false, string $scheme = null, int $httpPort = null, int $httpsPort = null): Response
public function urlRedirectAction(Request $request, string $path, bool $permanent = false, string $scheme = null, int $httpPort = null, int $httpsPort = null, bool $keepRequestMethod = false): Response
{
if ('' == $path) {
throw new HttpException($permanent ? 410 : 404);
}

$statusCode = $permanent ? 301 : 302;
if ($keepRequestMethod) {
$statusCode = $permanent ? 308 : 307;
} else {
$statusCode = $permanent ? 301 : 302;
}

// redirect if the path is a full URL
if (parse_url($path, PHP_URL_SCHEME)) {
Expand Down
27 changes: 21 additions & 6 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function testEmptyRoute()
/**
* @dataProvider provider
*/
public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expectedAttributes)
public function testRoute($permanent, $keepRequestMethod, $ignoreAttributes, $expectedCode, $expectedAttributes)
{
$request = new Request();

Expand All @@ -62,6 +62,7 @@ public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expecte
'permanent' => $permanent,
'additional-parameter' => 'value',
'ignoreAttributes' => $ignoreAttributes,
'keepRequestMethod' => $keepRequestMethod,
),
);

Expand All @@ -76,7 +77,7 @@ public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expecte

$controller = new RedirectController($router);

$returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes);
$returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes, $keepRequestMethod);

$this->assertRedirectUrl($returnResponse, $url);
$this->assertEquals($expectedCode, $returnResponse->getStatusCode());
Expand All @@ -85,10 +86,14 @@ public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expecte
public function provider()
{
return array(
array(true, false, 301, array('additional-parameter' => 'value')),
array(false, false, 302, array('additional-parameter' => 'value')),
array(false, true, 302, array()),
array(false, array('additional-parameter'), 302, array()),
array(true, false, false, 301, array('additional-parameter' => 'value')),
array(false, false, false, 302, array('additional-parameter' => 'value')),
array(false, false, true, 302, array()),
array(false, false, array('additional-parameter'), 302, array()),
array(true, true, false, 308, array('additional-parameter' => 'value')),
array(false, true, false, 307, array('additional-parameter' => 'value')),
array(false, true, true, 307, array()),
array(false, true, array('additional-parameter'), 307, array()),
);
}

Expand Down Expand Up @@ -122,6 +127,16 @@ public function testFullURL()
$this->assertEquals(302, $returnResponse->getStatusCode());
}

public function testFullURLWithMethodKeep()
{
$request = new Request();
$controller = new RedirectController();
$returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/', false, null, null, null, true);

$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
$this->assertEquals(307, $returnResponse->getStatusCode());
}

public function testUrlRedirectDefaultPorts()
{
$host = 'www.example.com';
Expand Down