Skip to content

Commit

Permalink
Forced all fragment uris to be signed, even for ESI
Browse files Browse the repository at this point in the history
  • Loading branch information
stof committed Sep 2, 2014
1 parent b554961 commit 654b1f2
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<tag name="kernel.fragment_renderer" />
<argument type="service" id="esi" />
<argument type="service" id="fragment.renderer.inline" />
<argument type="service" id="uri_signer" />
<call method="setFragmentPath"><argument>%fragment.path%</argument></call>
</service>
</services>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Component\HttpKernel\EventListener;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\IpUtils;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
Expand Down Expand Up @@ -77,13 +76,6 @@ protected function validateRequest(Request $request)
throw new AccessDeniedHttpException();
}

// does the Request come from a trusted IP?
$trustedIps = array_merge($this->getLocalIpAddresses(), $request->getTrustedProxies());
$remoteAddress = $request->server->get('REMOTE_ADDR');
if (IpUtils::checkIp($remoteAddress, $trustedIps)) {
return;
}

// is the Request signed?
// 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 : ''))) {
Expand All @@ -93,6 +85,11 @@ protected function validateRequest(Request $request)
throw new AccessDeniedHttpException();
}

/**
* @deprecated Deprecated since 2.3.19, to be removed in 3.0.
*
* @return string[]
*/
protected function getLocalIpAddresses()
{
return array('127.0.0.1', 'fe80::1', '::1');
Expand Down
22 changes: 19 additions & 3 deletions src/Symfony/Component/HttpKernel/Fragment/EsiFragmentRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Controller\ControllerReference;
use Symfony\Component\HttpKernel\HttpCache\Esi;
use Symfony\Component\HttpKernel\UriSigner;

/**
* Implements the ESI rendering strategy.
Expand All @@ -25,6 +26,7 @@ class EsiFragmentRenderer extends RoutableFragmentRenderer
{
private $esi;
private $inlineStrategy;
private $signer;

/**
* Constructor.
Expand All @@ -34,11 +36,13 @@ class EsiFragmentRenderer extends RoutableFragmentRenderer
*
* @param Esi $esi An Esi instance
* @param FragmentRendererInterface $inlineStrategy The inline strategy to use when ESI is not supported
* @param UriSigner $signer
*/
public function __construct(Esi $esi, FragmentRendererInterface $inlineStrategy)
public function __construct(Esi $esi, FragmentRendererInterface $inlineStrategy, UriSigner $signer = null)
{
$this->esi = $esi;
$this->inlineStrategy = $inlineStrategy;
$this->signer = $signer;
}

/**
Expand All @@ -61,12 +65,12 @@ public function render($uri, Request $request, array $options = array())
}

if ($uri instanceof ControllerReference) {
$uri = $this->generateFragmentUri($uri, $request);
$uri = $this->generateSignedFragmentUri($uri, $request);
}

$alt = isset($options['alt']) ? $options['alt'] : null;
if ($alt instanceof ControllerReference) {
$alt = $this->generateFragmentUri($alt, $request);
$alt = $this->generateSignedFragmentUri($alt, $request);
}

$tag = $this->esi->renderIncludeTag($uri, $alt, isset($options['ignore_errors']) ? $options['ignore_errors'] : false, isset($options['comment']) ? $options['comment'] : '');
Expand All @@ -81,4 +85,16 @@ public function getName()
{
return 'esi';
}

private function generateSignedFragmentUri($uri, Request $request)
{
if (null === $this->signer) {
throw new \LogicException('You must use a URI when using the ESI rendering strategy or set a URL signer.');
}

// we need to sign the absolute URI, but want to return the path only.
$fragmentUri = $this->signer->sign($this->generateFragmentUri($uri, $request, true));

return substr($fragmentUri, strlen($request->getSchemeAndHttpHost()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,6 @@ public function testAccessDeniedWithNonSafeMethods()
$listener->onKernelRequest($event);
}

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

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

$listener->onKernelRequest($event);
}

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpKernel\Fragment\EsiFragmentRenderer;
use Symfony\Component\HttpKernel\HttpCache\Esi;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\UriSigner;

class EsiFragmentRendererTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -48,7 +49,52 @@ public function testRender()
$this->assertEquals('<esi:include src="/" />', $strategy->render('/', $request)->getContent());
$this->assertEquals("<esi:comment text=\"This is a comment\" />\n<esi:include src=\"/\" />", $strategy->render('/', $request, array('comment' => 'This is a comment'))->getContent());
$this->assertEquals('<esi:include src="/" alt="foo" />', $strategy->render('/', $request, array('alt' => 'foo'))->getContent());
$this->assertEquals('<esi:include src="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller" alt="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller" />', $strategy->render(new ControllerReference('main_controller', array(), array()), $request, array('alt' => new ControllerReference('alt_controller', array(), array())))->getContent());
}

public function testRenderControllerReference()
{
$signer = new UriSigner('foo');
$strategy = new EsiFragmentRenderer(new Esi(), $this->getInlineStrategy(), $signer);

$request = Request::create('/');
$request->setLocale('fr');
$request->headers->set('Surrogate-Capability', 'ESI/1.0');

$reference = new ControllerReference('main_controller', array(), array());
$altReference = new ControllerReference('alt_controller', array(), array());

$this->assertEquals(
'<esi:include src="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dmain_controller&_hash=wDaFy1WsZUOWrrMdRMgJ1cOskFo%3D" alt="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller&_hash=56ycnRUlgaremRQVStZsGbVhIv8%3D" />',
$strategy->render($reference, $request, array('alt' => $altReference))->getContent()
);
}

/**
* @expectedException \LogicException
*/
public function testRenderControllerReferenceWithoutSignerThrowsException()
{
$strategy = new EsiFragmentRenderer(new Esi(), $this->getInlineStrategy());

$request = Request::create('/');
$request->setLocale('fr');
$request->headers->set('Surrogate-Capability', 'ESI/1.0');

$strategy->render(new ControllerReference('main_controller'), $request);
}

/**
* @expectedException \LogicException
*/
public function testRenderAltControllerReferenceWithoutSignerThrowsException()
{
$strategy = new EsiFragmentRenderer(new Esi(), $this->getInlineStrategy());

$request = Request::create('/');
$request->setLocale('fr');
$request->headers->set('Surrogate-Capability', 'ESI/1.0');

$strategy->render('/', $request, array('alt' => new ControllerReference('alt_controller')));
}

private function getInlineStrategy($called = false)
Expand Down

0 comments on commit 654b1f2

Please sign in to comment.