Commit a5cb55ce authored by Fabien Potencier's avatar Fabien Potencier

merged branch lmcnearney/master (PR #623)

This PR was squashed before being merged into the master branch (closes #623).

Discussion
----------

Include querystring parameters when issuing a redirect from RedirectableUrlMatcher

When a route has a scheme requirement (HTTP or HTTPS) and the current request's scheme does not match it, it issues a redirect to the required scheme. Prior to this change, the new URL being redirected to left off the querystring parameters.

It would be cleaner to set the QUERY_STRING parameter as part of RequestContext->fromRequest() but that's part of the Symfony core. Including it inside the $this['url_matcher'] closure keeps the change localized to Silex.

Commits
-------

09034777 Include querystring parameters when issuing a redirect from RedirectableUrlMatcher
parents 59e7dbd3 09034777
...@@ -125,6 +125,11 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte ...@@ -125,6 +125,11 @@ class Application extends \Pimple implements HttpKernelInterface, TerminableInte
}); });
$this['url_matcher'] = $this->share(function () use ($app) { $this['url_matcher'] = $this->share(function () use ($app) {
// Inject the query string into the RequestContext for Symfony versions <= 2.2
if ($app['request']->server->get('QUERY_STRING') !== '' && !method_exists($app['request_context'], 'getQueryString')) {
$app['request_context']->setParameter('QUERY_STRING', $app['request']->server->get('QUERY_STRING'));
}
return new RedirectableUrlMatcher($app['routes'], $app['request_context']); return new RedirectableUrlMatcher($app['routes'], $app['request_context']);
}); });
......
...@@ -29,6 +29,16 @@ class RedirectableUrlMatcher extends BaseRedirectableUrlMatcher ...@@ -29,6 +29,16 @@ class RedirectableUrlMatcher extends BaseRedirectableUrlMatcher
{ {
$url = $this->context->getBaseUrl().$path; $url = $this->context->getBaseUrl().$path;
// Query string support was added to RequestContext in 2.3
// Fall back to parameter injected by url_matcher closure for earlier versions
$query = method_exists($this->context, 'getQueryString')
? $this->context->getQueryString()
: $this->context->getParameter('QUERY_STRING') ?: '';
if ($query !== '') {
$url .= '?'.$query;
}
if ($this->context->getHost()) { if ($this->context->getHost()) {
if ($scheme) { if ($scheme) {
$port = ''; $port = '';
......
...@@ -207,6 +207,20 @@ class RouterTest extends \PHPUnit_Framework_TestCase ...@@ -207,6 +207,20 @@ class RouterTest extends \PHPUnit_Framework_TestCase
$this->assertTrue($response->isRedirect('https://example.com/secured')); $this->assertTrue($response->isRedirect('https://example.com/secured'));
} }
public function testRequireHttpsRedirectIncludesQueryString()
{
$app = new Application();
$app->match('/secured', function () {
return 'secured content';
})
->requireHttps();
$request = Request::create('http://example.com/secured?query=string');
$response = $app->handle($request);
$this->assertTrue($response->isRedirect('https://example.com/secured?query=string'));
}
public function testClassNameControllerSyntax() public function testClassNameControllerSyntax()
{ {
$app = new Application(); $app = new Application();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment