From 0ebb469269663749ade2f4977d19356f9fa3ceab Mon Sep 17 00:00:00 2001 From: Yonel Ceruto Date: Fri, 16 Aug 2019 16:47:08 -0400 Subject: [PATCH] Improving redirect config when using RedirectController --- .../Controller/RedirectController.php | 19 ++++ .../Controller/RedirectControllerTest.php | 87 ++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php index 91afeffa7066..00d1a4a6a1aa 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php @@ -157,4 +157,23 @@ public function urlRedirectAction(Request $request, string $path, bool $permanen return new RedirectResponse($url, $statusCode); } + + public function __invoke(Request $request): Response + { + $p = $request->attributes->get('_route_params', []); + + if (\array_key_exists('route', $p)) { + if (\array_key_exists('path', $p)) { + throw new \RuntimeException(sprintf('Ambiguous redirection settings, use the "path" or "route" parameter, not both: "%s" and "%s" found respectively in "%s" routing configuration.', $p['path'], $p['route'], $request->attributes->get('_route'))); + } + + return $this->redirectAction($request, $p['route'], $p['permanent'] ?? false, $p['ignoreAttributes'] ?? false, $p['keepRequestMethod'] ?? false, $p['keepQueryParams'] ?? false); + } + + if (\array_key_exists('path', $p)) { + return $this->urlRedirectAction($request, $p['path'], $p['permanent'] ?? false, $p['scheme'] ?? null, $p['httpPort'] ?? null, $p['httpsPort'] ?? null, $p['keepRequestMethod'] ?? false); + } + + throw new \RuntimeException(sprintf('The parameter "path" or "route" is required to configure the redirect action in "%s" routing configuration.', $request->attributes->get('_route'))); + } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php index 1c76d0366c62..c8942614d1fb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php @@ -42,6 +42,22 @@ public function testEmptyRoute() } catch (HttpException $e) { $this->assertSame(404, $e->getStatusCode()); } + + $request = new Request([], [], ['_route_params' => ['route' => '', 'permanent' => true]]); + try { + $controller($request); + $this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown'); + } catch (HttpException $e) { + $this->assertSame(410, $e->getStatusCode()); + } + + $request = new Request([], [], ['_route_params' => ['route' => '', 'permanent' => false]]); + try { + $controller($request); + $this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown'); + } catch (HttpException $e) { + $this->assertSame(404, $e->getStatusCode()); + } } /** @@ -71,7 +87,7 @@ public function testRoute($permanent, $keepRequestMethod, $keepQueryParams, $ign $router = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock(); $router - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('generate') ->with($this->equalTo($route), $this->equalTo($expectedAttributes)) ->willReturn($url); @@ -79,7 +95,10 @@ public function testRoute($permanent, $keepRequestMethod, $keepQueryParams, $ign $controller = new RedirectController($router); $returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes, $keepRequestMethod, $keepQueryParams); + $this->assertRedirectUrl($returnResponse, $url); + $this->assertEquals($expectedCode, $returnResponse->getStatusCode()); + $returnResponse = $controller($request); $this->assertRedirectUrl($returnResponse, $url); $this->assertEquals($expectedCode, $returnResponse->getStatusCode()); } @@ -116,14 +135,35 @@ public function testEmptyPath() } catch (HttpException $e) { $this->assertSame(404, $e->getStatusCode()); } + + $request = new Request([], [], ['_route_params' => ['path' => '', 'permanent' => true]]); + try { + $controller($request); + $this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown'); + } catch (HttpException $e) { + $this->assertSame(410, $e->getStatusCode()); + } + + $request = new Request([], [], ['_route_params' => ['path' => '', 'permanent' => false]]); + try { + $controller($request); + $this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown'); + } catch (HttpException $e) { + $this->assertSame(404, $e->getStatusCode()); + } } public function testFullURL() { $request = new Request(); $controller = new RedirectController(); + $returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/'); + $this->assertRedirectUrl($returnResponse, 'http://foo.bar/'); + $this->assertEquals(302, $returnResponse->getStatusCode()); + $request = new Request([], [], ['_route_params' => ['path' => 'http://foo.bar/']]); + $returnResponse = $controller($request); $this->assertRedirectUrl($returnResponse, 'http://foo.bar/'); $this->assertEquals(302, $returnResponse->getStatusCode()); } @@ -132,8 +172,13 @@ 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()); + $request = new Request([], [], ['_route_params' => ['path' => 'http://foo.bar/', 'keepRequestMethod' => true]]); + $returnResponse = $controller($request); $this->assertRedirectUrl($returnResponse, 'http://foo.bar/'); $this->assertEquals(307, $returnResponse->getStatusCode()); } @@ -151,12 +196,18 @@ public function testUrlRedirectDefaultPorts() $controller = $this->createRedirectController(null, $httpsPort); $returnValue = $controller->urlRedirectAction($request, $path, false, 'https'); $this->assertRedirectUrl($returnValue, $expectedUrl); + $request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => 'https']]); + $returnValue = $controller($request); + $this->assertRedirectUrl($returnValue, $expectedUrl); $expectedUrl = "http://$host:$httpPort$baseUrl$path"; $request = $this->createRequestObject('https', $host, $httpPort, $baseUrl); $controller = $this->createRedirectController($httpPort); $returnValue = $controller->urlRedirectAction($request, $path, false, 'http'); $this->assertRedirectUrl($returnValue, $expectedUrl); + $request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => 'http']]); + $returnValue = $controller($request); + $this->assertRedirectUrl($returnValue, $expectedUrl); } public function urlRedirectProvider() @@ -205,6 +256,10 @@ public function testUrlRedirect($scheme, $httpPort, $httpsPort, $requestScheme, $returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $httpPort, $httpsPort); $this->assertRedirectUrl($returnValue, $expectedUrl); + + $request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => $scheme, 'httpPort' => $httpPort, 'httpsPort' => $httpsPort]]); + $returnValue = $controller($request); + $this->assertRedirectUrl($returnValue, $expectedUrl); } public function pathQueryParamsProvider() @@ -234,6 +289,10 @@ public function testPathQueryParams($expectedUrl, $path, $queryString) $returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $port, null); $this->assertRedirectUrl($returnValue, $expectedUrl); + + $request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => $scheme, 'httpPort' => $port]]); + $returnValue = $controller($request); + $this->assertRedirectUrl($returnValue, $expectedUrl); } public function testRedirectWithQuery() @@ -247,10 +306,13 @@ public function testRedirectWithQuery() $request->query = new ParameterBag(['base' => 'zaza']); $request->attributes = new ParameterBag(['_route_params' => ['base2' => 'zaza']]); $urlGenerator = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock(); - $urlGenerator->expects($this->once())->method('generate')->willReturn('/test?base=zaza&base2=zaza')->with('/test', ['base' => 'zaza', 'base2' => 'zaza'], UrlGeneratorInterface::ABSOLUTE_URL); + $urlGenerator->expects($this->exactly(2))->method('generate')->willReturn('/test?base=zaza&base2=zaza')->with('/test', ['base' => 'zaza', 'base2' => 'zaza'], UrlGeneratorInterface::ABSOLUTE_URL); $controller = new RedirectController($urlGenerator); $this->assertRedirectUrl($controller->redirectAction($request, '/test', false, false, false, true), '/test?base=zaza&base2=zaza'); + + $request->attributes->set('_route_params', ['base2' => 'zaza', 'route' => '/test', 'ignoreAttributes' => false, 'keepRequestMethod' => false, 'keepQueryParams' => true]); + $this->assertRedirectUrl($controller($request), '/test?base=zaza&base2=zaza'); } public function testRedirectWithQueryWithRouteParamsOveriding() @@ -264,10 +326,29 @@ public function testRedirectWithQueryWithRouteParamsOveriding() $request->query = new ParameterBag(['base' => 'zaza']); $request->attributes = new ParameterBag(['_route_params' => ['base' => 'zouzou']]); $urlGenerator = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock(); - $urlGenerator->expects($this->once())->method('generate')->willReturn('/test?base=zouzou')->with('/test', ['base' => 'zouzou'], UrlGeneratorInterface::ABSOLUTE_URL); + $urlGenerator->expects($this->exactly(2))->method('generate')->willReturn('/test?base=zouzou')->with('/test', ['base' => 'zouzou'], UrlGeneratorInterface::ABSOLUTE_URL); $controller = new RedirectController($urlGenerator); $this->assertRedirectUrl($controller->redirectAction($request, '/test', false, false, false, true), '/test?base=zouzou'); + + $request->attributes->set('_route_params', ['base' => 'zouzou', 'route' => '/test', 'ignoreAttributes' => false, 'keepRequestMethod' => false, 'keepQueryParams' => true]); + $this->assertRedirectUrl($controller($request), '/test?base=zouzou'); + } + + public function testMissingPathOrRouteParameter() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('The parameter "path" or "route" is required to configure the redirect action in "_redirect" routing configuration.'); + + (new RedirectController())(new Request([], [], ['_route' => '_redirect'])); + } + + public function testAmbiguousPathAndRouteParameter() + { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Ambiguous redirection settings, use the "path" or "route" parameter, not both: "/foo" and "bar" found respectively in "_redirect" routing configuration.'); + + (new RedirectController())(new Request([], [], ['_route' => '_redirect', '_route_params' => ['path' => '/foo', 'route' => 'bar']])); } private function createRequestObject($scheme, $host, $port, $baseUrl, $queryString = '')