Skip to content

Commit 0ebb469

Browse files
ycerutoyceruto
committed
Improving redirect config when using RedirectController
1 parent 940eabb commit 0ebb469

File tree

2 files changed

+103
-3
lines changed

2 files changed

+103
-3
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,23 @@ public function urlRedirectAction(Request $request, string $path, bool $permanen
157157

158158
return new RedirectResponse($url, $statusCode);
159159
}
160+
161+
public function __invoke(Request $request): Response
162+
{
163+
$p = $request->attributes->get('_route_params', []);
164+
165+
if (\array_key_exists('route', $p)) {
166+
if (\array_key_exists('path', $p)) {
167+
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')));
168+
}
169+
170+
return $this->redirectAction($request, $p['route'], $p['permanent'] ?? false, $p['ignoreAttributes'] ?? false, $p['keepRequestMethod'] ?? false, $p['keepQueryParams'] ?? false);
171+
}
172+
173+
if (\array_key_exists('path', $p)) {
174+
return $this->urlRedirectAction($request, $p['path'], $p['permanent'] ?? false, $p['scheme'] ?? null, $p['httpPort'] ?? null, $p['httpsPort'] ?? null, $p['keepRequestMethod'] ?? false);
175+
}
176+
177+
throw new \RuntimeException(sprintf('The parameter "path" or "route" is required to configure the redirect action in "%s" routing configuration.', $request->attributes->get('_route')));
178+
}
160179
}

‎src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php‎

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@ public function testEmptyRoute()
4242
} catch (HttpException $e) {
4343
$this->assertSame(404, $e->getStatusCode());
4444
}
45+
46+
$request = new Request([], [], ['_route_params' => ['route' => '', 'permanent' => true]]);
47+
try {
48+
$controller($request);
49+
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
50+
} catch (HttpException $e) {
51+
$this->assertSame(410, $e->getStatusCode());
52+
}
53+
54+
$request = new Request([], [], ['_route_params' => ['route' => '', 'permanent' => false]]);
55+
try {
56+
$controller($request);
57+
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
58+
} catch (HttpException $e) {
59+
$this->assertSame(404, $e->getStatusCode());
60+
}
4561
}
4662

4763
/**
@@ -71,15 +87,18 @@ public function testRoute($permanent, $keepRequestMethod, $keepQueryParams, $ign
7187

7288
$router = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock();
7389
$router
74-
->expects($this->once())
90+
->expects($this->exactly(2))
7591
->method('generate')
7692
->with($this->equalTo($route), $this->equalTo($expectedAttributes))
7793
->willReturn($url);
7894

7995
$controller = new RedirectController($router);
8096

8197
$returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes, $keepRequestMethod, $keepQueryParams);
98+
$this->assertRedirectUrl($returnResponse, $url);
99+
$this->assertEquals($expectedCode, $returnResponse->getStatusCode());
82100

101+
$returnResponse = $controller($request);
83102
$this->assertRedirectUrl($returnResponse, $url);
84103
$this->assertEquals($expectedCode, $returnResponse->getStatusCode());
85104
}
@@ -116,14 +135,35 @@ public function testEmptyPath()
116135
} catch (HttpException $e) {
117136
$this->assertSame(404, $e->getStatusCode());
118137
}
138+
139+
$request = new Request([], [], ['_route_params' => ['path' => '', 'permanent' => true]]);
140+
try {
141+
$controller($request);
142+
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
143+
} catch (HttpException $e) {
144+
$this->assertSame(410, $e->getStatusCode());
145+
}
146+
147+
$request = new Request([], [], ['_route_params' => ['path' => '', 'permanent' => false]]);
148+
try {
149+
$controller($request);
150+
$this->fail('Expected Symfony\Component\HttpKernel\Exception\HttpException to be thrown');
151+
} catch (HttpException $e) {
152+
$this->assertSame(404, $e->getStatusCode());
153+
}
119154
}
120155

121156
public function testFullURL()
122157
{
123158
$request = new Request();
124159
$controller = new RedirectController();
160+
125161
$returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/');
162+
$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
163+
$this->assertEquals(302, $returnResponse->getStatusCode());
126164

165+
$request = new Request([], [], ['_route_params' => ['path' => 'http://foo.bar/']]);
166+
$returnResponse = $controller($request);
127167
$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
128168
$this->assertEquals(302, $returnResponse->getStatusCode());
129169
}
@@ -132,8 +172,13 @@ public function testFullURLWithMethodKeep()
132172
{
133173
$request = new Request();
134174
$controller = new RedirectController();
175+
135176
$returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/', false, null, null, null, true);
177+
$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
178+
$this->assertEquals(307, $returnResponse->getStatusCode());
136179

180+
$request = new Request([], [], ['_route_params' => ['path' => 'http://foo.bar/', 'keepRequestMethod' => true]]);
181+
$returnResponse = $controller($request);
137182
$this->assertRedirectUrl($returnResponse, 'http://foo.bar/');
138183
$this->assertEquals(307, $returnResponse->getStatusCode());
139184
}
@@ -151,12 +196,18 @@ public function testUrlRedirectDefaultPorts()
151196
$controller = $this->createRedirectController(null, $httpsPort);
152197
$returnValue = $controller->urlRedirectAction($request, $path, false, 'https');
153198
$this->assertRedirectUrl($returnValue, $expectedUrl);
199+
$request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => 'https']]);
200+
$returnValue = $controller($request);
201+
$this->assertRedirectUrl($returnValue, $expectedUrl);
154202

155203
$expectedUrl = "http://$host:$httpPort$baseUrl$path";
156204
$request = $this->createRequestObject('https', $host, $httpPort, $baseUrl);
157205
$controller = $this->createRedirectController($httpPort);
158206
$returnValue = $controller->urlRedirectAction($request, $path, false, 'http');
159207
$this->assertRedirectUrl($returnValue, $expectedUrl);
208+
$request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => 'http']]);
209+
$returnValue = $controller($request);
210+
$this->assertRedirectUrl($returnValue, $expectedUrl);
160211
}
161212

162213
public function urlRedirectProvider()
@@ -205,6 +256,10 @@ public function testUrlRedirect($scheme, $httpPort, $httpsPort, $requestScheme,
205256

206257
$returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $httpPort, $httpsPort);
207258
$this->assertRedirectUrl($returnValue, $expectedUrl);
259+
260+
$request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => $scheme, 'httpPort' => $httpPort, 'httpsPort' => $httpsPort]]);
261+
$returnValue = $controller($request);
262+
$this->assertRedirectUrl($returnValue, $expectedUrl);
208263
}
209264

210265
public function pathQueryParamsProvider()
@@ -234,6 +289,10 @@ public function testPathQueryParams($expectedUrl, $path, $queryString)
234289

235290
$returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $port, null);
236291
$this->assertRedirectUrl($returnValue, $expectedUrl);
292+
293+
$request->attributes = new ParameterBag(['_route_params' => ['path' => $path, 'scheme' => $scheme, 'httpPort' => $port]]);
294+
$returnValue = $controller($request);
295+
$this->assertRedirectUrl($returnValue, $expectedUrl);
237296
}
238297

239298
public function testRedirectWithQuery()
@@ -247,10 +306,13 @@ public function testRedirectWithQuery()
247306
$request->query = new ParameterBag(['base' => 'zaza']);
248307
$request->attributes = new ParameterBag(['_route_params' => ['base2' => 'zaza']]);
249308
$urlGenerator = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock();
250-
$urlGenerator->expects($this->once())->method('generate')->willReturn('/test?base=zaza&base2=zaza')->with('/test', ['base' => 'zaza', 'base2' => 'zaza'], UrlGeneratorInterface::ABSOLUTE_URL);
309+
$urlGenerator->expects($this->exactly(2))->method('generate')->willReturn('/test?base=zaza&base2=zaza')->with('/test', ['base' => 'zaza', 'base2' => 'zaza'], UrlGeneratorInterface::ABSOLUTE_URL);
251310

252311
$controller = new RedirectController($urlGenerator);
253312
$this->assertRedirectUrl($controller->redirectAction($request, '/test', false, false, false, true), '/test?base=zaza&base2=zaza');
313+
314+
$request->attributes->set('_route_params', ['base2' => 'zaza', 'route' => '/test', 'ignoreAttributes' => false, 'keepRequestMethod' => false, 'keepQueryParams' => true]);
315+
$this->assertRedirectUrl($controller($request), '/test?base=zaza&base2=zaza');
254316
}
255317

256318
public function testRedirectWithQueryWithRouteParamsOveriding()
@@ -264,10 +326,29 @@ public function testRedirectWithQueryWithRouteParamsOveriding()
264326
$request->query = new ParameterBag(['base' => 'zaza']);
265327
$request->attributes = new ParameterBag(['_route_params' => ['base' => 'zouzou']]);
266328
$urlGenerator = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock();
267-
$urlGenerator->expects($this->once())->method('generate')->willReturn('/test?base=zouzou')->with('/test', ['base' => 'zouzou'], UrlGeneratorInterface::ABSOLUTE_URL);
329+
$urlGenerator->expects($this->exactly(2))->method('generate')->willReturn('/test?base=zouzou')->with('/test', ['base' => 'zouzou'], UrlGeneratorInterface::ABSOLUTE_URL);
268330

269331
$controller = new RedirectController($urlGenerator);
270332
$this->assertRedirectUrl($controller->redirectAction($request, '/test', false, false, false, true), '/test?base=zouzou');
333+
334+
$request->attributes->set('_route_params', ['base' => 'zouzou', 'route' => '/test', 'ignoreAttributes' => false, 'keepRequestMethod' => false, 'keepQueryParams' => true]);
335+
$this->assertRedirectUrl($controller($request), '/test?base=zouzou');
336+
}
337+
338+
public function testMissingPathOrRouteParameter()
339+
{
340+
$this->expectException(\RuntimeException::class);
341+
$this->expectExceptionMessage('The parameter "path" or "route" is required to configure the redirect action in "_redirect" routing configuration.');
342+
343+
(new RedirectController())(new Request([], [], ['_route' => '_redirect']));
344+
}
345+
346+
public function testAmbiguousPathAndRouteParameter()
347+
{
348+
$this->expectException(\RuntimeException::class);
349+
$this->expectExceptionMessage('Ambiguous redirection settings, use the "path" or "route" parameter, not both: "/foo" and "bar" found respectively in "_redirect" routing configuration.');
350+
351+
(new RedirectController())(new Request([], [], ['_route' => '_redirect', '_route_params' => ['path' => '/foo', 'route' => 'bar']]));
271352
}
272353

273354
private function createRequestObject($scheme, $host, $port, $baseUrl, $queryString = '')

0 commit comments

Comments
 (0)