Skip to content

Commit

Permalink
some performance tweaks
Browse files Browse the repository at this point in the history
This adds lazy loading for firewall configurations. This is useful when you have multiple firewalls, only the firewalls which are actually needed to process the Request are initialized. So, your event dispatcher is not as costly to initialize anymore.

It also implements re-using of RequestMatchers if all matching rules are the same, and exposes the remaining rules which are already implemented by the request matcher (host, ip, methods) in the access-control section
  • Loading branch information
schmittjoh authored and fabpot committed Jan 21, 2011
1 parent 82d29d2 commit 507da2a
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 34 deletions.
Expand Up @@ -29,6 +29,8 @@
*/
class SecurityExtension extends Extension
{
protected $requestMatchers = array();

/**
* Loads the web configuration.
*
Expand Down Expand Up @@ -108,22 +110,31 @@ protected function createAuthorization($config, ContainerBuilder $container)
}

// matcher
$id = 'security.matcher.url.'.$i;
$definition = $container->register($id, '%security.matcher.class%');
$definition->setPublic(false);
$path = $host = $methods = $ip = null;
if (isset($access['path'])) {
$definition->addMethodCall('matchPath', array(is_array($access['path']) ? $access['path']['pattern'] : $access['path']));
$path = $access['path'];
}
if (isset($access['host'])) {
$host = $access['host'];
}
if (count($tMethods = $this->fixConfig($access, 'method')) > 0) {
$methods = $tMethods;
}
if (isset($access['ip'])) {
$ip = $access['ip'];
}

$matchAttributes = array();
$attributes = $this->fixConfig($access, 'attribute');
foreach ($attributes as $key => $attribute) {
if (isset($attribute['key'])) {
$key = $attribute['key'];
}
$definition->addMethodCall('matchAttribute', array($key, $attribute['pattern']));
$matchAttributes[$key] = $attribute['pattern'];
}
$matcher = $this->createRequestMatcher($container, $path, $host, $methods, $ip, $matchAttributes);

$container->getDefinition('security.access_map')->addMethodCall('add', array(new Reference($id), $roles, $channel));
$container->getDefinition('security.access_map')->addMethodCall('add', array($matcher, $roles, $channel));
}
}

Expand Down Expand Up @@ -158,12 +169,21 @@ protected function createFirewalls($config, ContainerBuilder $container)
$container->merge($c);

// load firewall map
$map = $container->getDefinition('security.firewall.map');
$mapDef = $container->getDefinition('security.firewall.map');
$map = array();
foreach ($firewalls as $firewall) {
list($matcher, $listeners, $exceptionListener) = $this->createFirewall($container, $firewall, $providerIds);

$map->addMethodCall('add', array($matcher, $listeners, $exceptionListener));
$contextId = 'security.firewall.map.context.'.count($map);
$context = $container->setDefinition($contextId, clone $container->getDefinition('security.firewall.context'));
$context
->setPublic(true)
->setArgument(0, $listeners)
->setArgument(1, $exceptionListener)
;
$map[$contextId] = $matcher;
}
$mapDef->setArgument(1, $map);
}

protected function createFirewall(ContainerBuilder $container, $firewall, $providerIds)
Expand All @@ -175,13 +195,7 @@ protected function createFirewall(ContainerBuilder $container, $firewall, $provi
$i = 0;
$matcher = null;
if (isset($firewall['pattern'])) {
$id = 'security.matcher.map'.$id.'.'.++$i;
$container
->register($id, '%security.matcher.class%')
->setPublic(false)
->addMethodCall('matchPath', array($firewall['pattern']))
;
$matcher = new Reference($id);
$matcher = $this->createRequestMatcher($container, $firewall['pattern']);
}

// Security disabled?
Expand Down Expand Up @@ -577,6 +591,30 @@ protected function createSwitchUserListener($container, $id, $config, $defaultPr
return $switchUserListenerId;
}

protected function createRequestMatcher($container, $path = null, $host = null, $methods = null, $ip = null, array $attributes = array())
{
$serialized = serialize(array($path, $host, $methods, $ip, $attributes));
$id = 'security.request_matcher.'.md5($serialized).sha1($serialized);

if (isset($this->requestMatchers[$id])) {
return $this->requestMatchers[$id];
}

// only add arguments that are necessary
$arguments = array($path, $host, $methods, $ip, $attributes);
while (count($arguments) > 0 && !end($arguments)) {
array_pop($arguments);
}

$container
->register($id, '%security.matcher.class%')
->setPublic(false)
->setArguments($arguments)
;

return $this->requestMatchers[$id] = new Reference($id);
}

public function aclLoad(array $config, ContainerBuilder $container)
{
if (!$container->hasDefinition('security.acl')) {
Expand Down
10 changes: 7 additions & 3 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/security.xml
Expand Up @@ -76,7 +76,8 @@
<parameter key="security.exception_listener.class">Symfony\Component\HttpKernel\Security\Firewall\ExceptionListener</parameter>
<parameter key="security.context_listener.class">Symfony\Component\HttpKernel\Security\Firewall\ContextListener</parameter>
<parameter key="security.firewall.class">Symfony\Component\HttpKernel\Security\Firewall</parameter>
<parameter key="security.firewall.map.class">Symfony\Component\HttpKernel\Security\FirewallMap</parameter>
<parameter key="security.firewall.map.class">Symfony\Bundle\FrameworkBundle\Security\FirewallMap</parameter>
<parameter key="security.firewall.context.class">Symfony\Bundle\FrameworkBundle\Security\FirewallContext</parameter>
<parameter key="security.matcher.class">Symfony\Component\HttpFoundation\RequestMatcher</parameter>

<parameter key="security.role_hierarchy.class">Symfony\Component\Security\Role\RoleHierarchy</parameter>
Expand Down Expand Up @@ -157,8 +158,11 @@
<tag name="kernel.listener" priority="-128" />
<argument type="service" id="security.firewall.map" />
</service>
<service id="security.firewall.map" class="%security.firewall.map.class%" public="false" />

<service id="security.firewall.map" class="%security.firewall.map.class%" public="false">
<argument type="service" id="service_container" />
<argument type="collection" />
</service>

<service id="security.context_listener" class="%security.context_listener.class%" public="false">
<argument type="service" id="security.context" />
<argument type="collection"></argument>
Expand Down
Expand Up @@ -85,5 +85,10 @@
<argument type="service" id="security.authentication.manager" />
<argument type="service" id="logger" on-invalid="null" />
</service>

<service id="security.firewall.context" class="%security.firewall.context.class%" public="false">
<argument type="collection" />
<argument type="service" id="security.exception_listener" />
</service>
</services>
</container>
28 changes: 28 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Security/FirewallContext.php
@@ -0,0 +1,28 @@
<?php

namespace Symfony\Bundle\FrameworkBundle\Security;

use Symfony\Component\HttpKernel\Security\Firewall\ExceptionListener;

/**
* This is a wrapper around the actual firewall configuration which allows us
* to lazy load the context for one specific firewall only when we need it.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class FirewallContext
{
protected $listeners;
protected $exceptionListener;

public function __construct(array $listeners, ExceptionListener $exceptionListener)
{
$this->listeners = $listeners;
$this->exceptionListener = $exceptionListener;
}

public function getContext()
{
return array($this->listeners, $this->exceptionListener);
}
}
37 changes: 37 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Security/FirewallMap.php
@@ -0,0 +1,37 @@
<?php

namespace Symfony\Bundle\FrameworkBundle\Security;

use Symfony\Component\HttpKernel\Security\FirewallMapInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* This is a lazy-loading firewall map implementation
*
* Listeners will only be initialized if we really need them.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class FirewallMap implements FirewallMapInterface
{
protected $container;
protected $map;

public function __construct(ContainerInterface $container, array $map)
{
$this->container = $container;
$this->map = $map;
}

public function getListeners(Request $request)
{
foreach ($this->map as $contextId => $requestMatcher) {
if (null === $requestMatcher || $requestMatcher->matches($request)) {
return $this->container->get($contextId)->getContext();
}
}

return array(array(), null);
}
}
Expand Up @@ -50,11 +50,12 @@ public function testFirewalls()
{
$container = $this->getContainer('firewall');

$arguments = $container->getDefinition('security.firewall.map')->getArguments();
$listeners = array();
foreach ($container->getDefinition('security.firewall.map')->getMethodCalls() as $call) {
if ($call[0] == 'add') {
$listeners[] = array_map(function ($ref) { return preg_replace('/\.[a-f0-9]+$/', '', (string) $ref); }, $call[1][1]);
}
foreach (array_keys($arguments[1]) as $contextId) {
$contextDef = $container->getDefinition($contextId);
$arguments = $contextDef->getArguments();
$listeners[] = array_map(function ($ref) { return preg_replace('/\.[a-f0-9]+$/', '', (string) $ref); }, $arguments[0]);
}

$this->assertEquals(array(
Expand Down Expand Up @@ -84,10 +85,22 @@ public function testAccess()
}
}

$this->assertEquals(array(
array('security.matcher.url.0', array('ROLE_USER'), 'https'),
array('security.matcher.url.1', array('IS_AUTHENTICATED_ANONYMOUSLY'), null),
), $rules);
$matcherIds = array();
foreach ($rules as $rule) {
list($matcherId, $roles, $channel) = $rule;

$this->assertFalse(isset($matcherIds[$matcherId]));
$matcherIds[$matcherId] = true;

$i = count($matcherIds);
if (1 === $i) {
$this->assertEquals(array('ROLE_USER'), $roles);
$this->assertEquals('https', $channel);
} else if (2 === $i) {
$this->assertEquals(array('IS_AUTHENTICATED_ANONYMOUSLY'), $roles);
$this->assertNull($channel);
}
}
}

protected function getContainer($file)
Expand Down
11 changes: 10 additions & 1 deletion src/Symfony/Component/HttpFoundation/RequestMatcher.php
Expand Up @@ -22,7 +22,16 @@ class RequestMatcher implements RequestMatcherInterface
protected $host;
protected $methods;
protected $ip;
protected $attributes = array();
protected $attributes;

public function __construct($path = null, $host = null, $methods = null, $ip = null, array $attributes = array())
{
$this->path = $path;
$this->host = $host;
$this->methods = $methods;
$this->ip = $ip;
$this->attributes = $attributes;
}

/**
* Adds a check for the URL host name.
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/HttpKernel/Security/Firewall.php
Expand Up @@ -37,7 +37,7 @@ class Firewall
*
* @param FirewallMap $map A FirewallMap instance
*/
public function __construct(FirewallMap $map)
public function __construct(FirewallMapInterface $map)
{
$this->map = $map;
$this->currentListeners = array();
Expand Down Expand Up @@ -71,12 +71,12 @@ public function handle(Event $event)
// disconnect all listeners from core.security to avoid the overhead
// of most listeners having to do this manually
$this->dispatcher->disconnect('core.security');

// ensure that listeners disconnect from wherever they have connected to
foreach ($this->currentListeners as $listener) {
$listener->unregister($this->dispatcher);
}

// register listeners for this firewall
list($listeners, $exception) = $this->map->getListeners($request);
if (null !== $exception) {
Expand All @@ -85,7 +85,7 @@ public function handle(Event $event)
foreach ($listeners as $listener) {
$listener->register($this->dispatcher);
}

// save current listener instances
$this->currentListeners = $listeners;
if (null !== $exception) {
Expand Down
Expand Up @@ -60,7 +60,7 @@ public function __construct(SecurityContext $securityContext, AuthenticationMana
}

/**
*
*
*
* @param EventDispatcher $dispatcher An EventDispatcher instance
* @param integer $priority The priority
Expand All @@ -69,7 +69,7 @@ public function register(EventDispatcher $dispatcher)
{
$dispatcher->connect('core.security', array($this, 'handle'), 0);
}

/**
* {@inheritDoc}
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/Security/FirewallMap.php
Expand Up @@ -21,7 +21,7 @@
*
* @author Fabien Potencier <fabien.potencier@symfony-project.com>
*/
class FirewallMap
class FirewallMap implements FirewallMapInterface
{
protected $map = array();

Expand Down
28 changes: 28 additions & 0 deletions src/Symfony/Component/HttpKernel/Security/FirewallMapInterface.php
@@ -0,0 +1,28 @@
<?php

namespace Symfony\Component\HttpKernel\Security;

use Symfony\Component\HttpFoundation\Request;

/**
* This interface must be implemented by firewall maps.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
interface FirewallMapInterface
{
/**
* Returns the authentication listeners, and the exception listener to use
* for the given request.
*
* If there are no authentication listeners, the first inner are must be
* empty.
*
* If there is no exception listener, the second element of the outer array
* must be null.
*
* @param Request $request
* @return array of the format array(array(AuthenticationListener), ExceptionListener)
*/
function getListeners(Request $request);
}

0 comments on commit 507da2a

Please sign in to comment.