Merge branch '2.4' into 2.5

* 2.4: (21 commits) [HttpKernel] fixed some unit tests for 2.4 (signature now uses SHA256 instead of MD5) [HttpKernel] simplified code [HttpKernel] fixed internal fragment handling fixing yaml indentation [WebProfiler] replaced the import/export feature from the web interface to a CLI tool Forced all fragment uris to be signed, even for ESI Add tests and more assertions [FrameworkBundle][Translator] Validate locales. [HttpFoundation] added some missing tests [HttpFoundation] Improve string values in test codes [Security] Add more tests for StringUtils::equals fix comment: not fourth but sixth argument fixing typo in a comment [FrameworkBundle] fixed CS [FrameworkBundle] PhpExtractor bugfix and improvements [Finder] Fix findertest readability [Filesystem] Add FTP stream wrapper context option to enable overwrite (override) fix parsing of Authorization header Test examples from Drupal SA-CORE-2014-003 Fix potential DoS when parsing HOST ... Conflicts: src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php
symfony · Sep 3, 2014 · 6521614 · 6521614
2 parents 473c33d + 9a071a6
commit 6521614
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 31 deletions.
diff --git a/EventListener/FragmentListener.php b/EventListener/FragmentListener.php
@@ -12,11 +12,11 @@
 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;
 use Symfony\Component\HttpKernel\UriSigner;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
@@ -25,8 +25,8 @@
  * All URL paths starting with /_fragment are handled as
  * content fragments by this listener.
  *
- * If the request does not come from a trusted IP, it throws an
- * AccessDeniedHttpException exception.
+ * If throws an AccessDeniedHttpException exception if the request
+ * is not signed or if it is not an internal sub-request.
  *
  * @author Fabien Potencier <fabien@symfony.com>
  */
@@ -62,7 +62,9 @@ public function onKernelRequest(GetResponseEvent $event)
             return;
         }
 
-        $this->validateRequest($request);
+        if ($event->isMasterRequest()) {
+            $this->validateRequest($request);
+        }
 
         parse_str($request->query->get('_path', ''), $attributes);
         $request->attributes->add($attributes);
@@ -77,13 +79,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 : ''))) {
@@ -93,6 +88,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');

diff --git a/EventListener/SessionListener.php b/EventListener/SessionListener.php
@@ -25,7 +25,7 @@ abstract class SessionListener implements EventSubscriberInterface
 {
     public function onKernelRequest(GetResponseEvent $event)
     {
-        if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
+        if (!$event->isMasterRequest()) {
             return;
         }
 

diff --git a/EventListener/TestSessionListener.php b/EventListener/TestSessionListener.php
@@ -30,7 +30,7 @@ abstract class TestSessionListener implements EventSubscriberInterface
 {
     public function onKernelRequest(GetResponseEvent $event)
     {
-        if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
+        if (!$event->isMasterRequest()) {
             return;
         }
 
@@ -55,7 +55,7 @@ public function onKernelRequest(GetResponseEvent $event)
      */
     public function onKernelResponse(FilterResponseEvent $event)
     {
-        if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
+        if (!$event->isMasterRequest()) {
             return;
         }
 

diff --git a/Fragment/EsiFragmentRenderer.php b/Fragment/EsiFragmentRenderer.php
@@ -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.
@@ -25,6 +26,7 @@ class EsiFragmentRenderer extends RoutableFragmentRenderer
 {
     private $esi;
     private $inlineStrategy;
+    private $signer;
 
     /**
      * Constructor.
@@ -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 = null, InlineFragmentRenderer $inlineStrategy)
+    public function __construct(Esi $esi = null, InlineFragmentRenderer $inlineStrategy, UriSigner $signer = null)
     {
         $this->esi = $esi;
         $this->inlineStrategy = $inlineStrategy;
+        $this->signer = $signer;
     }
 
     /**
@@ -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'] : '');
@@ -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()));
+    }
 }
diff --git a/Tests/EventListener/FragmentListenerTest.php b/Tests/EventListener/FragmentListenerTest.php
@@ -47,19 +47,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
      */

diff --git a/Tests/Fragment/EsiFragmentRendererTest.php b/Tests/Fragment/EsiFragmentRendererTest.php
@@ -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
 {
@@ -41,7 +42,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=Jz1P8NErmhKTeI6onI1EdAXTB85359MY3RIk5mSJ60w%3D" alt="/_fragment?_path=_format%3Dhtml%26_locale%3Dfr%26_controller%3Dalt_controller&_hash=iPJEdRoUpGrM1ztqByiorpfMPtiW%2FOWwdH1DBUXHhEc%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)