Skip to content

Commit

Permalink
[Routing] added support for _scheme requirement
Browse files Browse the repository at this point in the history
The _scheme requirement can be used to force routes to always match one given scheme
and to always be generated with the given scheme.

So, if _scheme is set to https, URL generation will force an absolute URL if the
current scheme is http. And if you request the URL with http, you will be redirected
to the https URL.
  • Loading branch information
fabpot committed Apr 20, 2011
1 parent d993a91 commit 07aae98
Show file tree
Hide file tree
Showing 14 changed files with 177 additions and 59 deletions.
Expand Up @@ -51,23 +51,43 @@ public function redirectAction($route, $permanent = false)
/**
* Redirects to a URL.
*
* It expects a url path parameter.
* By default, the response status code is 301.
*
* If the url is empty, the status code will be 410.
* If the permanent path parameter is set, the status code will be 302.
* If the path is empty, the status code will be 410.
* If the permanent flag is set, the status code will be 302.
*
* @param string $url The url to redirect to
* @param string $path The path to redirect to
* @param Boolean $permanent Whether the redirect is permanent or not
* @param Boolean $scheme The URL scheme (null to keep the current one)
* @param integer $httpPort The HTTP port
* @param integer $httpsPort The HTTPS port
*
* @return Response A Response instance
*/
public function urlRedirectAction($url, $permanent = false)
public function urlRedirectAction($path, $permanent = false, $scheme = null, $httpPort = 80, $httpsPort = 443)
{
if (!$url) {
if (!$path) {
return new Response(null, 410);
}

$request = $this->container->get('request');
if (null === $scheme) {
$scheme = $request->getScheme();
}
$qs = $request->getQueryString();
if ($qs) {
$qs = '?'.$qs;
}

$port = '';
if ('http' === $scheme && 80 != $httpPort) {
$port = ':'.$httpPort;
} elseif ('https' === $scheme && 443 != $httpPort) {
$port = ':'.$httpsPort;
}

$url = $scheme.'://'.$request->getHttpHost().$port.$request->getBaseUrl().$path.$qs;

return new RedirectResponse($url, $permanent ? 301 : 302);
}
}
Expand Up @@ -131,6 +131,8 @@ private function addRouterSection(ArrayNodeDefinition $rootNode)
->scalarNode('cache_warmer')->defaultFalse()->end()
->scalarNode('resource')->isRequired()->end()
->scalarNode('type')->end()
->scalarNode('http_port')->defaultValue(80)->end()
->scalarNode('https_port')->defaultValue(443)->end()
->end()
->end()
->end()
Expand Down
Expand Up @@ -256,6 +256,10 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
$container->setAlias('router', 'router.cached');
}

$def = $container->getDefinition('request_listener');
$def->setArgument(2, $config['http_port']);
$def->setArgument(3, $config['https_port']);

$this->addClassesToCompile(array(
'Symfony\\Component\\Routing\\RouterInterface',
'Symfony\\Component\\Routing\\Matcher\\UrlMatcherInterface',
Expand Down
23 changes: 14 additions & 9 deletions src/Symfony/Bundle/FrameworkBundle/RequestListener.php
Expand Up @@ -29,14 +29,18 @@
*/
class RequestListener
{
protected $router;
protected $logger;
protected $container;
private $router;
private $logger;
private $container;
private $httpPort;
private $httpsPort;

public function __construct(ContainerInterface $container, RouterInterface $router, LoggerInterface $logger = null)
public function __construct(ContainerInterface $container, RouterInterface $router, $httpPort = 80, $httpsPort = 443, LoggerInterface $logger = null)
{
$this->container = $container;
$this->router = $router;
$this->httpPort = $httpPort;
$this->httpsPort = $httpsPort;
$this->logger = $logger;
}

Expand Down Expand Up @@ -73,11 +77,12 @@ protected function initializeRequestAttributes(Request $request, $master)
// set the context even if the parsing does not need to be done
// to have correct link generation
$this->router->setContext(array(
'base_url' => $request->getBaseUrl(),
'method' => $request->getMethod(),
'host' => $request->getHost(),
'port' => $request->getPort(),
'is_secure' => $request->isSecure(),
'base_url' => $request->getBaseUrl(),
'method' => $request->getMethod(),
'host' => $request->getHost(),
'scheme' => $request->getScheme(),
'http_port' => $this->httpPort,
'https_port' => $this->httpsPort,
));
}

Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml
Expand Up @@ -31,6 +31,8 @@
<tag name="monolog.logger" channel="request" />
<argument type="service" id="service_container" />
<argument type="service" id="router" />
<argument /> <!-- HTTP port -->
<argument /> <!-- HTTPS port -->
<argument type="service" id="logger" on-invalid="ignore" />
</service>

Expand Down
Expand Up @@ -19,12 +19,24 @@
*/
class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
{
public function redirect($pathinfo, $route)
/**
* Redirects the user to another URL.
*
* @param string $path The path info to redirect to.
* @param string $route The route that matched
* @param string $scheme The URL scheme (null to keep the current one)
*
* @return array An array of parameters
*/
public function redirect($path, $route, $scheme = null)
{
return array(
'_controller' => 'Symfony\\Bundle\\FrameworkBundle\\Controller\\RedirectController::urlRedirectAction',
'url' => $this->context['base_url'].$pathinfo,
'path' => $path,
'permanent' => true,
'scheme' => $scheme,
'httpPort' => isset($this->context['http_port']) ? $this->context['http_port'] : 80,
'httpsPort' => isset($this->context['https_port']) ? $this->context['https_port'] : 443,
'_route' => $route,
);
}
Expand Down
23 changes: 16 additions & 7 deletions src/Symfony/Component/Routing/Generator/UrlGenerator.php
Expand Up @@ -129,14 +129,23 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa

$url = (isset($this->context['base_url']) ? $this->context['base_url'] : '').$url;

if ($absolute && isset($this->context['host'])) {
$isSecure = (isset($this->context['is_secure']) && $this->context['is_secure']);
$port = isset($this->context['port']) ? $this->context['port'] : 80;
$urlBeginning = 'http'.($isSecure ? 's' : '').'://'.$this->context['host'];
if (($isSecure && $port != 443) || (!$isSecure && $port != 80)) {
$urlBeginning .= ':'.$port;
if (isset($this->context['host'])) {
$scheme = isset($this->context['scheme']) ? $this->context['scheme'] : 'http';
if (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme != $req) {
$absolute = true;
$scheme = $req;
}

if ($absolute) {
$port = '';
if ('http' === $scheme && 80 != ($httpPort = isset($this->context['http_port']) ? $this->context['http_port'] : 80)) {
$port = ':'.$httpPort;
} elseif ('https' === $scheme && 443 != ($httpsPort = isset($this->context['https_port']) ? $this->context['https_port'] : 443)) {
$port = ':'.$httpsPort;
}

$url = $scheme.'://'.$this->context['host'].$port.$url;
}
$url = $urlBeginning.$url;
}

return $url;
Expand Down
23 changes: 18 additions & 5 deletions src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
Expand Up @@ -41,17 +41,17 @@ public function dump(array $options = array())

// trailing slash support is only enabled if we know how to redirect the user
$interfaces = class_implements($options['base_class']);
$supportsTrailingSlash = isset($interfaces['Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface']);
$supportsRedirections = isset($interfaces['Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface']);

return
$this->startClass($options['class'], $options['base_class']).
$this->addConstructor().
$this->addMatcher($supportsTrailingSlash).
$this->addMatcher($supportsRedirections).
$this->endClass()
;
}

private function addMatcher($supportsTrailingSlash)
private function addMatcher($supportsRedirections)
{
$code = array();

Expand All @@ -61,7 +61,7 @@ private function addMatcher($supportsTrailingSlash)
$hasTrailingSlash = false;
$matches = false;
if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', $compiledRoute->getRegex(), $m)) {
if ($supportsTrailingSlash && substr($m['url'], -1) === '/') {
if ($supportsRedirections && substr($m['url'], -1) === '/') {
$conditions[] = sprintf("rtrim(\$pathinfo, '/') === '%s'", rtrim(str_replace('\\', '', $m['url']), '/'));
$hasTrailingSlash = true;
} else {
Expand All @@ -73,7 +73,7 @@ private function addMatcher($supportsTrailingSlash)
}

$regex = $compiledRoute->getRegex();
if ($supportsTrailingSlash && $pos = strpos($regex, '/$')) {
if ($supportsRedirections && $pos = strpos($regex, '/$')) {
$regex = substr($regex, 0, $pos).'/?$'.substr($regex, $pos + 2);
$hasTrailingSlash = true;
}
Expand Down Expand Up @@ -110,6 +110,19 @@ private function addMatcher($supportsTrailingSlash)
, $name);
}

if ($scheme = $route->getRequirement('_scheme')) {
if (!$supportsRedirections) {
throw new \LogicException('The "_scheme" requirement is only supported for route dumper that implements RedirectableUrlMatcherInterface.');
}

$code[] = sprintf(<<<EOF
if (isset(\$this->context['scheme']) && \$this->context['scheme'] !== '$scheme') {
return \$this->redirect(\$pathinfo, '%s', '$scheme');
}
EOF
, $name);
}

// optimize parameters array
if (true === $matches && $compiledRoute->getDefaults()) {
$code[] = sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));"
Expand Down
Expand Up @@ -21,17 +21,11 @@ interface RedirectableUrlMatcherInterface
/**
* Redirects the user to another URL.
*
* As the Routing component does not know know to redirect the user,
* the default implementation throw an exception.
*
* Override this method to implement your own logic.
*
* If you are using a Dumper, don't forget to change the default base.
*
* @param string $pathinfo The path info to redirect to.
* @param string $route The route that matched
* @param string $path The path info to redirect to.
* @param string $route The route that matched
* @param string $scheme The URL scheme (null to keep the current one)
*
* @return array An array of parameters
*/
function redirect($pathinfo, $route);
function redirect($path, $route, $scheme = null);
}
Expand Up @@ -19,11 +19,12 @@
*/
class RedirectableUrlMatcher extends UrlMatcher implements RedirectableUrlMatcherInterface
{
public function redirect($pathinfo, $route)
public function redirect($path, $route, $scheme = null)
{
return array(
'_controller' => 'Some controller reference...',
'url' => $this->context['base_url'].$pathinfo,
'path' => $path,
'scheme' => $scheme,
);
}
}
Expand Up @@ -100,6 +100,22 @@ public function match($pathinfo)
return array ( 'def' => 'test', '_route' => 'foofoo',);
}

// secure
if ($pathinfo === '/secure') {
if (isset($this->context['scheme']) && $this->context['scheme'] !== 'https') {
return $this->redirect($pathinfo, 'secure', 'https');
}
return array('_route' => 'secure');
}

// nonsecure
if ($pathinfo === '/nonsecure') {
if (isset($this->context['scheme']) && $this->context['scheme'] !== 'http') {
return $this->redirect($pathinfo, 'nonsecure', 'http');
}
return array('_route' => 'nonsecure');
}

throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new NotFoundException();
}
}
Expand Up @@ -61,8 +61,7 @@ public function testDumpWithRoutes()
'base_url' => '/app.php',
'method' => 'GET',
'host' => 'localhost',
'port' => 80,
'is_secure' => false
'scheme' => 'http',
));

$absoluteUrlWithParameter = $projectUrlGenerator->generate('Test', array('foo' => 'bar'), true);
Expand All @@ -88,8 +87,7 @@ public function testDumpWithoutRoutes()
'base_url' => '/app.php',
'method' => 'GET',
'host' => 'localhost',
'port' => 80,
'is_secure' => false
'scheme' => 'http',
));

$projectUrlGenerator->generate('Test', array());
Expand Down
Expand Up @@ -28,23 +28,23 @@ public function testAbsoluteUrlWithPort80()
public function testAbsoluteSecureUrlWithPort443()
{
$routes = $this->getRoutes('test', new Route('/testing'));
$url = $this->getGenerator($routes, array('port' => 443, 'is_secure' => true))->generate('test', array(), true);
$url = $this->getGenerator($routes, array('scheme' => 'https'))->generate('test', array(), true);

$this->assertEquals('https://localhost/app.php/testing', $url);
}

public function testAbsoluteUrlWithNonStandardPort()
{
$routes = $this->getRoutes('test', new Route('/testing'));
$url = $this->getGenerator($routes, array('port' => 8080))->generate('test', array(), true);
$url = $this->getGenerator($routes, array('http_port' => 8080))->generate('test', array(), true);

$this->assertEquals('http://localhost:8080/app.php/testing', $url);
}

public function testAbsoluteSecureUrlWithNonStandardPort()
{
$routes = $this->getRoutes('test', new Route('/testing'));
$url = $this->getGenerator($routes, array('port' => 8080, 'is_secure' => true))->generate('test', array(), true);
$url = $this->getGenerator($routes, array('https_port' => 8080, 'scheme' => 'https'))->generate('test', array(), true);

$this->assertEquals('https://localhost:8080/app.php/testing', $url);
}
Expand Down Expand Up @@ -125,15 +125,34 @@ public function testGenerateForRouteWithInvalidManditoryParameter()
$this->getGenerator($routes)->generate('test', array('foo' => 'bar'), true);
}

public function testSchemeRequirementDoesNothingIfSameCurrentScheme()
{
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
$this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));

$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
$this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
}

public function testSchemeRequirementForcesAbsoluteUrl()
{
$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
$this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));

$routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
$this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
}

protected function getGenerator(RouteCollection $routes, array $context = array())
{
$generator = new UrlGenerator($routes);
$generator->setContext(array_replace(array(
'base_url' => '/app.php',
'method' => 'GET',
'host' => 'localhost',
'port' => 80,
'is_secure' => false,
'http_port' => 80,
'https_port' => 443,
'scheme' => 'http',
), $context));

return $generator;
Expand Down

9 comments on commit 07aae98

@kriswallsmith
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the Apache URL matcher support this?

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 07aae98 Apr 20, 2011

Choose a reason for hiding this comment

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

Good catch. I totally forgot to have a look at the Apache URL matcher.

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

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

I'd guess they can conflict if you send mixed messages to both you probably end up in a redirect loop. I don't know why anyone would force http on the auth page though if they specify https for the route.

Also @schmittjoh can you look at my last comment on http://trac.symfony-project.org/ticket/9686 (the second part)? I talked with @fabpot quickly and it seems this could be handled in the security component entirely. If you have questions I'm on irc..

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 07aae98 Apr 20, 2011

Choose a reason for hiding this comment

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

I have a commit where I remove the ChannelListener but I decided to keep it for now. They look similar but they solve different problems:

The pros for the _scheme routing requirement:

  • It feels more natural to have this feature in the Routing component (we already have HTTP method requirement); people will probably find it easier to configure it this way.
  • The Routing takes care of generating absolute HTTPS or HTTP URLs when it makes sense (which avoids the redirection round trip most of the time) -- this is probably the biggest advantage of this solution over the previous one.
  • It is faster as the requirement is "compiled" into the cache.
  • You can use the feature even if you don't use the Security component (and almost everybody use the Routing).

The pros of the ChannelListener:

  • The requirement being defined in the route definition, it's not possible to change it for a third-party bundle (think of the admin gen bundle which you want to be served as HTTPS only).
  • Configuration is on a route basis, and not a regexp like in the Security component (securing everything under /admin for instance).

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

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

Configuration is on a route basis, and not a regexp like in the Security component (securing everything under /admin for instance).

Regarding that, it would be nice to have a setting in the framework > routing options to change the default scheme to https.

@lsmith77
Copy link
Contributor

Choose a reason for hiding this comment

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

Is someone taking this on? Is there a ticket ..? Just want to make sure this isnt forgotten.

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 07aae98 Apr 23, 2011

Choose a reason for hiding this comment

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

Taking on what?

@lsmith77
Copy link
Contributor

Choose a reason for hiding this comment

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

@fabpot: what @Seldaek and @schmittjoh mentioned

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

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

I put it on my todos, I don't think it's important enough to have a ticket hanging around.

Please sign in to comment.