Skip to content
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

Fixing InvalidArgumentException with 'Cannot redirect to an empty URL' #198

Merged
merged 4 commits into from
May 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ vendor/
composer.phar
composer.lock
phpunit.xml
/.idea
19 changes: 15 additions & 4 deletions Controller/ThemeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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'],
Expand All @@ -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;
}
}
59 changes: 59 additions & 0 deletions Tests/Common/Comparator/SymfonyResponse.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
233 changes: 233 additions & 0 deletions Tests/Controller/ThemeControllerTest.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
15 changes: 11 additions & 4 deletions Tests/UseCaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down