10000 RedirectController - use 307/308 HTTP status codes by ZipoKing · Pull Request #22 · symfony/framework-bundle · GitHub
[go: up one dir, main page]
Skip to content

RedirectController - use 307/308 HTTP status codes #22

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
wants to merge 2 commits into from
Closed

RedirectController - use 307/308 HTTP status codes #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a9ba77
Add support for HTTP 307/308 status codes in RedirectController
ZipoKing Feb 18, 2018
39d42da
Use new array syntax
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
48 changes: 31 additions & 17 deletions Controller/RedirectController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,29 +46,37 @@ 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
{
if ('' == $route) {
throw new HttpException($permanent ? 410 : 404);
throw new HttpException($permanent ? Response::HTTP_GONE : Response::HTTP_NOT_FOUND);
}

$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 ? Response::HTTP_PERMANENTLY_REDIRECT : Response::HTTP_TEMPORARY_REDIRECT;
} else {
@trigger_error('Since next major release redirect action will be made with 307/308 HTTP status codes', \E_USER_DEPRECATED);
$statusCode = $permanent ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND;
}

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

/**
Expand All @@ -80,22 +88,28 @@ 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);
throw new HttpException($permanent ? Response::HTTP_GONE : Response::HTTP_NOT_FOUND);
}

$statusCode = $permanent ? 301 : 302;
if ($keepRequestMethod) {
$statusCode = $permanent ? Response::HTTP_PERMANENTLY_REDIRECT : Response::HTTP_TEMPORARY_REDIRECT;
} else {
@trigger_error('Since next major release redirect action will be made with 307/308 HTTP status codes', \E_USER_DEPRECATED);
$statusCode = $permanent ? Response::HTTP_MOVED_PERMANENTLY : Response::HTTP_FOUND;
}

// redirect if the path is a full URL
if (parse_url($path, PHP_URL_SCHEME)) {
Expand Down
41 changes: 28 additions & 13 deletions Tests/Controller/RedirectControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ public function testEmptyRoute()
$controller->redirectAction($request, '', true);
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
} catch (HttpException $e) {
$this->assertSame(410, $e->getStatusCode());
$this->assertSame(Response::HTTP_GONE, $e->getStatusCode());
}

try {
$controller->redirectAction($request, '', false);
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
} catch (HttpException $e) {
$this->assertSame(404, $e->getStatusCode());
$this->assertSame(Response::HTTP_NOT_FOUND, $e->getStatusCode());
}
}

/**
* @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,20 +77,24 @@ 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());
}

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()),
);
return [
[true, false, false, Response::HTTP_MOVED_PERMANENTLY, ['additional-parameter' => 'value']],
[false, false, false, Response::HTTP_FOUND, ['additional-parameter' => 'value']],
[false, false, true, Response::HTTP_FOUND, []],
[false, false, ['additional-parameter'], Response::HTTP_FOUND, []],
[true, true, false, Response::HTTP_PERMANENTLY_REDIRECT, ['additional-parameter' => 'value']],
[false, true, false, Response::HTTP_TEMPORARY_REDIRECT, ['additional-parameter' => 'value']],
[false, true, true, Response::HTTP_TEMPORARY_REDIRECT, []],
[false, true, ['additional-parameter'], Response::HTTP_TEMPORARY_REDIRECT, []],
];
}

public function testEmptyPath()
Expand All @@ -101,14 +106,14 @@ public function testEmptyPath()
$controller->urlRedirectAction($request, '', true);
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
} catch (HttpException $e) {
$this->assertSame(410, $e->getStatusCode());
$this->assertSame(Response::HTTP_GONE, $e->getStatusCode());
}

try {
$controller->urlRedirectAction($request, '', false);
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
} catch (HttpException $e) {
$this->assertSame(404, $e->getStatusCode());
$this->assertSame(Response::HTTP_NOT_FOUND, $e->getStatusCode());
}
}

Expand All @@ -119,7 +124,17 @@ public function testFullURL()
$returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/');

$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
$this->assertEquals(302, $returnResponse->getStatusCode());
$this->assertEquals(Response::HTTP_FOUND, $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(Response::HTTP_TEMPORARY_REDIRECT, $returnResponse->getStatusCode());
}

public function testUrlRedirectDefaultPorts()
Expand Down
0