Merge pull request #198 from iakunin/empty-referer-fix

Fixing InvalidArgumentException with 'Cannot redirect to an empty URL'

.gitignore

@@ -2,3 +2,4 @@ vendor/
 composer.phar
 composer.lock
 phpunit.xml
+/.idea

Controller/ThemeController.php

@@ -13,8 +13,8 @@
 
 use Liip\ThemeBundle\ActiveTheme;
 use Symfony\Component\HttpFoundation\Cookie;
-use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 /**
@@ -73,14 +73,13 @@ public function switchAction(Request $request)
 
         $this->activeTheme->setName($theme);
 
-        $url = $request->headers->get('Referer', '/');
-        $response = new RedirectResponse($url);
+        $response = new RedirectResponse($this->extractUrl($request));
 
         if (!empty($this->cookieOptions)) {
             $cookie = new Cookie(
                 $this->cookieOptions['name'],
                 $theme,
-                time() + $this->cookieOptions['lifetime'],
+                $request->server->get('REQUEST_TIME') + $this->cookieOptions['lifetime'],
                 $this->cookieOptions['path'],
                 $this->cookieOptions['domain'],
                 (bool) $this->cookieOptions['secure'],
@@ -92,4 +91,16 @@ public function switchAction(Request $request)
 
         return $response;
     }
+
+    /**
+     * @param Request $request
+     *
+     * @return string
+     */
+    private function extractUrl(Request $request)
+    {
+        $url = $request->headers->get('Referer');
+
+        return empty($url) ? '/' : $url;
+    }
 }

Tests/Common/Comparator/SymfonyResponse.php

@@ -0,0 +1,59 @@
+<?php
+
+namespace Liip\ThemeBundle\Tests\Common\Comparator;
+
+use SebastianBergmann\Comparator\Comparator;
+use SebastianBergmann\Comparator\Factory;
+use SebastianBergmann\Comparator\ObjectComparator;
+use Symfony\Component\HttpFoundation\Response;
+
+class SymfonyResponse extends Comparator
+{
+    const PREDEFINED_DATE = 'Tue, 15 Nov 1994 08:12:31 GMT';
+
+    /**
+     * @var ObjectComparator
+     */
+    private $inner;
+
+    public function __construct()
+    {
+        parent::__construct();
+
+        $this->inner = new ObjectComparator();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function accepts($expected, $actual)
+    {
+        return
+            $expected instanceof Response
+                &&
+            $actual instanceof Response
+                &&
+            $this->inner->accepts($expected, $actual);
+    }
+
+    /**
+     * @param Response $expected
+     * @param Response $actual
+     * {@inheritdoc}
+     */
+    public function assertEquals($expected, $actual, $delta = 0, $canonicalize = false, $ignoreCase = false)
+    {
+        $expected->headers->set('Date', self::PREDEFINED_DATE);
+        $actual->headers->set('Date', self::PREDEFINED_DATE);
+
+        $this->inner->assertEquals($expected, $actual, $delta, $canonicalize, $ignoreCase);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function setFactory(Factory $factory)
+    {
+        $this->inner->setFactory($factory);
+    }
+}

Tests/Controller/ThemeControllerTest.php

@@ -0,0 +1,233 @@
+<?php
+
+/*
+ * This file is part of the Liip/ThemeBundle
+ *
+ * (c) Liip AG
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Liip\ThemeBundle\Tests\Controller;
+
+use Liip\ThemeBundle\ActiveTheme;
+use Liip\ThemeBundle\Controller\ThemeController;
+use Liip\ThemeBundle\Tests\Common\Comparator\SymfonyResponse as SymfonyResponseComparator;
+use PHPUnit\Framework\MockObject\Matcher\Invocation;
+use SebastianBergmann\Comparator\Factory as ComparatorFactory;
+use Symfony\Component\HttpFoundation\Cookie;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+class ThemeControllerTest extends \PHPUnit\Framework\TestCase
+{
+    const WRONG_THEME = 'wrong_theme';
+    const RIGHT_THEME = 'right_theme';
+    const REFERER = 'some_referer';
+    const DEFAULT_REDIRECT_URL = '/';
+    const COOKIE_NAME = 'some_cookie_name';
+    const COOKIE_LIFETIME = 112233;
+    const COOKIE_PATH = '/';
+    const COOKIE_DOMAIN = 'some_domain';
+    const IS_COOKIE_SECURE = false;
+    const IS_COOKIE_HTTP_ONLY = false;
+    const REQUEST_TIME = 123;
+
+    /**
+     * @var SymfonyResponseComparator
+     */
+    private $symfonyResponseComparator;
+
+    /**
+     * @dataProvider switchActionDataProvider
+     * @param string|null $referer
+     * @param RedirectResponse $expectedResponse
+     */
+    public function testSwitchAction($referer, RedirectResponse $expectedResponse)
+    {
+        $controller = $this->createThemeController(self::once());
+
+        $actualResponse = $controller->switchAction($this->createRequest($referer));
+
+        self::assertEquals($expectedResponse, $actualResponse);
+    }
+
+    /**
+     * @return mixed[]
+     */
+    public function switchActionDataProvider()
+    {
+        return array(
+            'not empty referer' => array(
+                'referer' => self::REFERER,
+                'expectedResponse' => $this->createExpectedResponse(self::REFERER),
+            ),
+            'empty string as referer' => array(
+                'referer' => '',
+                'expectedResponse' => $this->createExpectedResponse(self::DEFAULT_REDIRECT_URL),
+            ),
+            'zero string as referer' => array(
+                'referer' => '0',
+                'expectedResponse' => $this->createExpectedResponse(self::DEFAULT_REDIRECT_URL),
+            ),
+            'zero int as referer' => array(
+                'referer' => 0,
+                'expectedResponse' => $this->createExpectedResponse(self::DEFAULT_REDIRECT_URL),
+            ),
+            'empty array as referer' => array(
+                'referer' => array(),
+                'expectedResponse' => $this->createExpectedResponse(self::DEFAULT_REDIRECT_URL),
+            ),
+            'null as referer' => array(
+                'referer' => null,
+                'expectedResponse' => $this->createExpectedResponse(self::DEFAULT_REDIRECT_URL),
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider switchActionDataProvider
+     * @param string|null $referer
+     * @param RedirectResponse $expectedResponse
+     */
+    public function testSwitchActionWithCookieOptions($referer, RedirectResponse $expectedResponse)
+    {
+        $controller = $this->createThemeController(self::once(), $this->createCookieOptions());
+
+        $actualResponse = $controller->switchAction($this->createRequest($referer));
+
+        self::assertEquals($this->addCookie($expectedResponse), $actualResponse);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+     * @expectedExceptionMessage The theme "wrong_theme" does not exist
+     */
+    public function testSwitchActionWithNotFoundTheme()
+    {
+        $controller = $this->createThemeController(self::never());
+
+        $controller->switchAction($this->createRequestWithWrongTheme());
+    }
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->symfonyResponseComparator = new SymfonyResponseComparator();
+        ComparatorFactory::getInstance()->register($this->symfonyResponseComparator);
+    }
+
+    protected function tearDown()
+    {
+        ComparatorFactory::getInstance()->unregister($this->symfonyResponseComparator);
+        $this->symfonyResponseComparator = null;
+
+        parent::tearDown();
+    }
+
+    /**
+     * @param Invocation $activeThemeInvocation
+     * @param mixed[]|null $cookieOptions
+     * @return ThemeController
+     */
+    private function createThemeController(Invocation $activeThemeInvocation, array $cookieOptions = null)
+    {
+        return new ThemeController(
+            $this->createActiveThemeMock($activeThemeInvocation),
+            array(self::RIGHT_THEME),
+            $cookieOptions
+        );
+    }
+
+    /**
+     * @param Invocation $invocation
+     * @return ActiveTheme
+     */
+    private function createActiveThemeMock(Invocation $invocation)
+    {
+        $mock = $this->createMock('\Liip\ThemeBundle\ActiveTheme');
+
+        $mock
+            ->expects($invocation)
+            ->method('setName')
+            ->with(self::RIGHT_THEME)
+        ;
+
+        return $mock;
+    }
+
+    /**
+     * @param string|null $referer
+     * @return Request
+     */
+    private function createRequest($referer)
+    {
+        $request = new Request(array('theme' => self::RIGHT_THEME));
+
+        $request->headers->add(
+            array('Referer' => array($referer))
+        );
+
+        $request->server->add(
+            array('REQUEST_TIME' => self::REQUEST_TIME)
+        );
+
+        return $request;
+    }
+
+    /**
+     * @param string $url
+     * @return RedirectResponse
+     */
+    private function createExpectedResponse($url)
+    {
+        return new RedirectResponse($url);
+    }
+
+    /**
+     * @return mixed[]
+     */
+    private function createCookieOptions()
+    {
+        return array(
+            'name' => self::COOKIE_NAME,
+            'lifetime' => self::COOKIE_LIFETIME,
+            'path' => self::COOKIE_PATH,
+            'domain' => self::COOKIE_DOMAIN,
+            'secure' => self::IS_COOKIE_SECURE,
+            'http_only' => self::IS_COOKIE_HTTP_ONLY,
+        );
+    }
+
+    /**
+     * @return Request
+     */
+    private function createRequestWithWrongTheme()
+    {
+        return new Request(array('theme' => self::WRONG_THEME));
+    }
+
+    /**
+     * @param Response $response
+     * @return Response
+     */
+    private function addCookie(Response $response)
+    {
+        $response->headers->setCookie(
+            new Cookie(
+                self::COOKIE_NAME,
+                self::RIGHT_THEME,
+                self::REQUEST_TIME + self::COOKIE_LIFETIME,
+                self::COOKIE_PATH,
+                self::COOKIE_DOMAIN,
+                self::IS_COOKIE_SECURE,
+                self::IS_COOKIE_HTTP_ONLY
+            )
+        );
+
+        return $response;
+    }
+}

Tests/UseCaseTest.php

@@ -12,10 +12,10 @@
 
 namespace Liip\ThemeBundle\Tests\EventListener;
 
-use Symfony\Component\HttpKernel\HttpKernelInterface;
-use Liip\ThemeBundle\EventListener\ThemeRequestListener;
-use Liip\ThemeBundle\Controller\ThemeController;
 use Liip\ThemeBundle\ActiveTheme;
+use Liip\ThemeBundle\Controller\ThemeController;
+use Liip\ThemeBundle\EventListener\ThemeRequestListener;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
 
 /**
  * Bundle Functional tests.
@@ -98,7 +98,14 @@ protected function getMockRequest($theme, $cookieReturnValue = 'cookie', $userAg
             ->getMock();
         $request->server->expects($this->any())
             ->method('get')
-            ->will($this->returnValue($userAgent));
+            ->will(
+                $this->returnValueMap(
+                    array(
+                        array('HTTP_USER_AGENT', null, $userAgent),
+                        array('REQUEST_TIME', null, 123),
+                    )
+                )
+            );
 
         return $request;
     }