Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[WIP] Kernel refactor #6459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jan 11, 2013
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
added some unit tests (and fixed some bugs)
  • Loading branch information
fabpot committed Jan 10, 2013
commit f7da1f0eb83f549c2245a3d2d1036be0688c78c6
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Bridge\Twig\Extension\HttpKernelExtension;
use Symfony\Bridge\Twig\Tests\TestCase;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\HttpContentRenderer;

class HttpKernelExtensionTest extends TestCase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Exception\LogicException;

/**
* Adds services tagged kernel.content_renderer_strategy as HTTP content rendering strategies.
Expand All @@ -31,6 +30,15 @@ public function process(ContainerBuilder $container)

$definition = $container->getDefinition('http_content_renderer');
foreach (array_keys($container->findTaggedServiceIds('kernel.content_renderer_strategy')) as $id) {
// We must assume that the class value has been correctly filled, even if the service is created by a factory
$class = $container->getDefinition($id)->getClass();

$refClass = new \ReflectionClass($class);
$interface = 'Symfony\Component\HttpKernel\RenderingStrategy\RenderingStrategyInterface';
if (!$refClass->implementsInterface($interface)) {
throw new \InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $id, $interface));
}

$definition->addMethodCall('addStrategy', array(new Reference($id)));
}
}
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function build(ContainerBuilder $container)
$container->addCompilerPass(new AddCacheClearerPass());
$container->addCompilerPass(new TranslationExtractorPass());
$container->addCompilerPass(new TranslationDumperPass());
$container->addCompilerPass(new HttpRenderingStrategyPass());
$container->addCompilerPass(new HttpRenderingStrategyPass(), PassConfig::TYPE_AFTER_REMOVING);

if ($container->getParameter('kernel.debug')) {
$container->addCompilerPass(new ContainerBuilderDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@
<argument>%kernel.debug%</argument>
</service>

<service id="http_content_renderer.strategy.default" class="%http_content_renderer.strategy.default.class%" public="false">
<service id="http_content_renderer.strategy.default" class="%http_content_renderer.strategy.default.class%">
<tag name="kernel.content_renderer_strategy" />
<argument type="service" id="http_kernel" />
<call method="setUrlGenerator"><argument type="service" id="router" /></call>
</service>

<!-- FIXME: this service should be private but it cannot due to a bug in PhpDumper -->
<service id="http_content_renderer.strategy.hinclude" class="%http_content_renderer.strategy.hinclude.class%">
Copy link
Member

Choose a reason for hiding this comment

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

strategies could be marked as private, allowing to inline them in the container

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, but because of a bug in the PHPDumper, that's not possible for hinclude. I'm working on fixing the bug so that we can use private services later on.

Copy link
Member Author

Choose a reason for hiding this comment

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

see #6565 for the bug fix

<tag name="kernel.content_renderer_strategy" />
<argument type="service" id="templating" on-invalid="null" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<argument type="service" id="esi" on-invalid="ignore" />
</service>

<service id="http_content_renderer.strategy.esi" class="%http_content_renderer.strategy.esi.class%" public="false">
<service id="http_content_renderer.strategy.esi" class="%http_content_renderer.strategy.esi.class%">
<tag name="kernel.content_renderer_strategy" />
<argument type="service" id="esi" />
<argument type="service" id="http_content_renderer.strategy.default" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\HttpRenderingStrategyPass;

class HttpRenderingStrategyPassTest extends \PHPUnit_Framework_TestCase
{
/**
* Tests that content rendering not implementing RenderingStrategyInterface
* trigger an exception.
*
* @expectedException \InvalidArgumentException
*/
public function testContentRendererWithoutInterface()
{
// one service, not implementing any interface
$services = array(
'my_content_renderer' => array(),
);

$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
$definition->expects($this->atLeastOnce())
->method('getClass')
->will($this->returnValue('stdClass'));

$builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
$builder->expects($this->any())
->method('hasDefinition')
->will($this->returnValue(true));

// We don't test kernel.content_renderer_strategy here
$builder->expects($this->atLeastOnce())
->method('findTaggedServiceIds')
->will($this->returnValue($services));

$builder->expects($this->atLeastOnce())
->method('getDefinition')
->will($this->returnValue($definition));

$pass = new HttpRenderingStrategyPass();
$pass->process($builder);
}

public function testValidContentRenderer()
{
$services = array(
'my_content_renderer' => array(),
);

$renderer = $this->getMock('Symfony\Component\DependencyInjection\Definition');
$renderer
->expects($this->once())
->method('addMethodCall')
->with('addStrategy', array(new Reference('my_content_renderer')))
;

$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
$definition->expects($this->atLeastOnce())
->method('getClass')
->will($this->returnValue('Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler\RenderingStrategyService'));

$builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder');
$builder->expects($this->any())
->method('hasDefinition')
->will($this->returnValue(true));

// We don't test kernel.content_renderer_strategy here
$builder->expects($this->atLeastOnce())
->method('findTaggedServiceIds')
->will($this->returnValue($services));

$builder->expects($this->atLeastOnce())
->method('getDefinition')
->will($this->onConsecutiveCalls($renderer, $definition));

$pass = new HttpRenderingStrategyPass();
$pass->process($builder);
}
}

class RenderingStrategyService implements \Symfony\Component\HttpKernel\RenderingStrategy\RenderingStrategyInterface
{
public function render($uri, Request $request = null, array $options = array())
{
}

public function getName()
{
return 'test';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormBuilder;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\Util\PropertyPath;
use Symfony\Component\Form\Extension\Validator\Constraints\Form;
use Symfony\Component\Form\Extension\Validator\EventListener\ValidationListener;
Expand Down
3 changes: 1 addition & 2 deletions 3 src/Symfony/Component/HttpFoundation/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -926,8 +926,7 @@ public function getSchemeAndHttpHost()
*/
public function getUri()
{
$qs = $this->getQueryString();
if (null !== $qs) {
if (null !== $qs = $this->getQueryString()) {
$qs = '?'.$qs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function onKernelRequest(GetResponseEvent $event)

parse_str($request->query->get('path', ''), $attributes);
$request->attributes->add($attributes);
$request->attributes->set('_route_params', array_replace($request->attributes->get('_route_params'), $attributes));
$request->attributes->set('_route_params', array_replace($request->attributes->get('_route_params', array()), $attributes));
$request->query->remove('path');
}

Expand All @@ -76,7 +76,8 @@ protected function validateRequest(Request $request)
}

// is the Request signed?
if ($this->signer->check($request->getUri())) {
// we cannot use $request->getUri() here as we want to work with the original URI (no query string reordering)
if ($this->signer->check($request->getSchemeAndHttpHost().$request->getBaseUrl().$request->getPathInfo().(null !== ($qs = $request->server->get('QUERY_STRING')) ? '?'.$qs : ''))) {
return;
}

Expand Down
2 changes: 2 additions & 0 deletions 2 src/Symfony/Component/HttpKernel/HttpContentRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ public function onKernelResponse(FilterResponseEvent $event)
* @param array $options An array of options
*
* @return string|null The Response content or null when the Response is streamed
*
* @throws \InvalidArgumentException when the strategy does not exist
*/
public function render($uri, $strategy = 'default', array $options = array())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ public function __construct(HttpKernelInterface $kernel)

/**
* {@inheritdoc}
*
* Additional available options:
*
* * alt: an alternative URI to render in case of an error
*/
public function render($uri, Request $request = null, array $options = array())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function __construct(Esi $esi, RenderingStrategyInterface $defaultStrateg
*/
public function render($uri, Request $request = null, array $options = array())
{
if (!$this->esi->hasSurrogateEsiCapability($request)) {
if (null === $request || !$this->esi->hasSurrogateEsiCapability($request)) {
return $this->defaultStrategy->render($uri, $request, $options);
}

Expand All @@ -69,7 +69,7 @@ public function render($uri, Request $request = null, array $options = array())
$alt = $this->generateProxyUri($alt, $request);
}

return $this->esi->renderIncludeTag($uri, $alt, $options['ignore_errors'], isset($options['comment']) ? $options['comment'] : '');
return $this->esi->renderIncludeTag($uri, $alt, isset($options['ignore_errors']) ? $options['ignore_errors'] : false, isset($options['comment']) ? $options['comment'] : '');
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public function setUrlGenerator(UrlGeneratorInterface $generator)
* @param Request $request A Request instance
*
* @return string A proxy URI
*
* @throws \LogicException when the _proxy route is not available
* @throws \LogicException when there is no registered route generator
*/
protected function generateProxyUri(ControllerReference $reference, Request $request = null)
{
Expand All @@ -63,7 +66,7 @@ protected function generateProxyUri(ControllerReference $reference, Request $req
}

try {
$uri = $this->generator->generate('_proxy', array('_controller' => $reference->controller, '_format' => $format), true);
$uri = $this->generator->generate('_proxy', array('_controller' => $reference->controller, '_format' => $format), UrlGeneratorInterface::ABSOLUTE_URL);
} catch (RouteNotFoundException $e) {
throw new \LogicException('Unable to generate a proxy URL as the "_proxy" route is not registered.', 0, $e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpKernel\Tests\EventListener;

use Symfony\Component\HttpKernel\EventListener\RouterProxyListener;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\UriSigner;

class RouterProxyListenerTest extends \PHPUnit_Framework_TestCase
{
protected function setUp()
{
if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) {
$this->markTestSkipped('The "EventDispatcher" component is not available');
}
}

public function testOnlyTrigerredOnProxyRoute()
{
$request = Request::create('http://example.com/foo?path=foo%3D=bar');

$listener = new RouterProxyListener(new UriSigner('foo'));
$event = $this->createGetResponseEvent($request, 'foobar');

$expected = $request->attributes->all();

$listener->onKernelRequest($event);

$this->assertEquals($expected, $request->attributes->all());
$this->assertTrue($request->query->has('path'));
}

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
*/
public function testAccessDeniedWithNonSafeMethods()
{
$request = Request::create('http://example.com/foo', 'POST');

$listener = new RouterProxyListener(new UriSigner('foo'));
$event = $this->createGetResponseEvent($request);

$listener->onKernelRequest($event);
}

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
*/
public function testAccessDeniedWithNonLocalIps()
{
$request = Request::create('http://example.com/foo', 'GET', array(), array(), array(), array('REMOTE_ADDR' => '10.0.0.1'));

$listener = new RouterProxyListener(new UriSigner('foo'));
$event = $this->createGetResponseEvent($request);

$listener->onKernelRequest($event);
}

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
*/
public function testAccessDeniedWithWrongSignature()
{
$request = Request::create('http://example.com/foo', 'GET', array(), array(), array(), array('REMOTE_ADDR' => '10.0.0.1'));

$listener = new RouterProxyListener(new UriSigner('foo'));
$event = $this->createGetResponseEvent($request);

$listener->onKernelRequest($event);
}

public function testWithSignatureAndNoPath()
{
$signer = new UriSigner('foo');
$request = Request::create($signer->sign('http://example.com/foo'), 'GET', array(), array(), array(), array('REMOTE_ADDR' => '10.0.0.1'));

$listener = new RouterProxyListener($signer);
$event = $this->createGetResponseEvent($request);

$listener->onKernelRequest($event);

$this->assertEquals(array('foo' => 'foo'), $request->attributes->get('_route_params'));
$this->assertFalse($request->query->has('path'));
}

public function testWithSignatureAndPath()
{
$signer = new UriSigner('foo');
$request = Request::create($signer->sign('http://example.com/foo?path=bar%3Dbar'), 'GET', array(), array(), array(), array('REMOTE_ADDR' => '10.0.0.1'));

$listener = new RouterProxyListener($signer);
$event = $this->createGetResponseEvent($request);

$listener->onKernelRequest($event);

$this->assertEquals(array('foo' => 'foo', 'bar' => 'bar'), $request->attributes->get('_route_params'));
$this->assertFalse($request->query->has('path'));
}

private function createGetResponseEvent(Request $request, $route = '_proxy')
{
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
$request->attributes->set('_route', $route);
$request->attributes->set('_route_params', array('foo' => 'foo'));

return new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
}
}
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.