diff --git a/core/API/Request.php b/core/API/Request.php index 5386ca27db68e1287012ab56166028b80cbdbd43..834d83290eb2aa577fbfc9b8e822e8b514f80e5d 100644 --- a/core/API/Request.php +++ b/core/API/Request.php @@ -278,6 +278,22 @@ class Request return sprintf('\Piwik\Plugins\%s\API', $plugin); } + /** + * Detect if request is an API request. Meaning the module is 'API' and an API method having a valid format was + * specified. + * + * @param array $request eg array('module' => 'API', 'method' => 'Test.getMethod') + * @return bool + * @throws Exception + */ + public static function isApiRequest($request) + { + $module = Common::getRequestVar('module', '', 'string', $request); + $method = Common::getRequestVar('method', '', 'string', $request); + + return $module === 'API' && !empty($method) && (count(explode('.', $method)) === 2); + } + /** * If the token_auth is found in the $request parameter, * the current session will be authenticated using this token_auth. diff --git a/core/ExceptionHandler.php b/core/ExceptionHandler.php index ac61707a29bca45d82d1851038111216485852a6..12552b85e449504b998b286dd7063475c420347f 100644 --- a/core/ExceptionHandler.php +++ b/core/ExceptionHandler.php @@ -9,6 +9,8 @@ namespace Piwik; use Exception; +use Piwik\API\Request; +use Piwik\API\ResponseBuilder; use Piwik\Container\ContainerDoesNotExistException; use Piwik\Plugins\CoreAdminHome\CustomLogo; @@ -67,7 +69,15 @@ class ExceptionHandler $message = $ex->getMessage(); - if (!method_exists($ex, 'isHtmlMessage') || !$ex->isHtmlMessage()) { + $isHtmlMessage = method_exists($ex, 'isHtmlMessage') && $ex->isHtmlMessage(); + + if (!$isHtmlMessage && Request::isApiRequest($_GET)) { + + $outputFormat = strtolower(Common::getRequestVar('format', 'xml', 'string', $_GET + $_POST)); + $response = new ResponseBuilder($outputFormat); + return $response->getResponseException($ex); + + } elseif (!$isHtmlMessage) { $message = Common::sanitizeInputValue($message); } diff --git a/plugins/Installation/Installation.php b/plugins/Installation/Installation.php index 3d501c26270f09a9bd8c43092ee472433a0de6d3..6a53bec2775cdb90a4031b14f8903bfefd5be2c3 100644 --- a/plugins/Installation/Installation.php +++ b/plugins/Installation/Installation.php @@ -8,6 +8,8 @@ */ namespace Piwik\Plugins\Installation; +use Piwik\API\Request; +use Piwik\API\ResponseBuilder; use Piwik\Common; use Piwik\Config; use Piwik\FrontController; @@ -40,8 +42,17 @@ class Installation extends \Piwik\Plugin public function displayDbConnectionMessage($exception = null) { + Common::sendResponseCode(500); + + $errorMessage = $exception->getMessage(); + + if (Request::isApiRequest($_GET)) { + $ex = new DatabaseConnectionFailedException($errorMessage); + throw $ex; + } + $view = new PiwikView("@Installation/cannotConnectToDb"); - $view->exceptionMessage = $exception->getMessage(); + $view->exceptionMessage = $errorMessage; $ex = new DatabaseConnectionFailedException($view->render()); $ex->setIsHtmlMessage(); diff --git a/plugins/Installation/tests/System/APITest.php b/plugins/Installation/tests/System/APITest.php new file mode 100644 index 0000000000000000000000000000000000000000..291c19daf8ca8a9a5fd06b0dc9ff6ecd5fcfc878 --- /dev/null +++ b/plugins/Installation/tests/System/APITest.php @@ -0,0 +1,85 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\Installation\tests\System; + +use Piwik\Application\Kernel\GlobalSettingsProvider; +use Piwik\Config; +use Piwik\Plugins\Installation\tests\Fixtures\SimpleFixtureTrackFewVisits; +use Piwik\Tests\Framework\Constraint\HttpResponseText; +use Piwik\Tests\Framework\Fixture; +use Piwik\Tests\Framework\TestCase\SystemTestCase; +use Piwik\Tests\Framework\TestingEnvironmentVariables; + +/** + * @group Installation + * @group APITest + * @group Plugins + */ +class APITest extends SystemTestCase +{ + /** + * @var SimpleFixtureTrackFewVisits + */ + public static $fixture = null; // initialized below class definition + + public static function setUpBeforeClass() + { + parent::setUpBeforeClass(); + + $testingEnvironment = new \Piwik\Tests\Framework\TestingEnvironmentVariables(); + $testingEnvironment->configFileLocal = PIWIK_INCLUDE_PATH . '/plugins/Installation/tests/resources/config.ini.php'; + $testingEnvironment->save(); + } + + public function test_shouldReturnHttp500_IfWrongDbInfo() + { + $this->assertResponseCode(500, $this->getUrl()); + } + + public function test_shouldReturnValidApiResponse_IfWrongDbInfo_formatXML() + { + $http = new HttpResponseText(''); + $response = $http->getResponse($this->getUrl()); + + $response = str_replace("\n", "", $response); + + $this->assertStringStartsWith('<?xml version="1.0" encoding="utf-8" ?><result> <error message=', $response); + $this->assertContains('Access denied', $response); + $this->assertStringEndsWith('</result>', $response); + } + + public function test_shouldReturnValidApiResponse_IfWrongDbInfo_formatJSON() + { + $http = new HttpResponseText(''); + $response = $http->getResponse($this->getUrl() . '&format=json'); + + $response = str_replace("\n", "", $response); + + $this->assertStringStartsWith('{"result":"error","message":"', $response); + $this->assertContains('Access denied', $response); + } + + private function getUrl() + { + return Fixture::getRootUrl() . 'tests/PHPUnit/proxy/index.php?module=API&method=API.getPiwikVersion'; + } + + public static function getOutputPrefix() + { + return ''; + } + + public static function getPathToTestDirectory() + { + return dirname(__FILE__); + } + +} + +APITest::$fixture = new SimpleFixtureTrackFewVisits(); \ No newline at end of file diff --git a/plugins/Installation/tests/resources/config.ini.php b/plugins/Installation/tests/resources/config.ini.php new file mode 100644 index 0000000000000000000000000000000000000000..43a364646ad877ba967736f519e6917e1abc9870 --- /dev/null +++ b/plugins/Installation/tests/resources/config.ini.php @@ -0,0 +1,15 @@ +; <?php exit; ?> DO NOT REMOVE THIS LINE +; file automatically generated or modified by Piwik; you can manually override the default values in global.ini.php by redefining them in this file. +[database] +host = "127.0.0.1" +username = "abc" +password = "xyz" +dbname = "piwik" +tables_prefix = "piwik_" +charset = "utf8" + +[database_tests] +username = "abc" +password = "xyz" +tables_prefix = "" + diff --git a/tests/PHPUnit/Framework/Constraint/HttpResponseText.php b/tests/PHPUnit/Framework/Constraint/HttpResponseText.php index 54d70982624494916d549b3243a7ee025d37e077..223d067a3aa33e3b5d1915647976dc4a07ac317b 100644 --- a/tests/PHPUnit/Framework/Constraint/HttpResponseText.php +++ b/tests/PHPUnit/Framework/Constraint/HttpResponseText.php @@ -20,17 +20,10 @@ class HttpResponseText extends \PHPUnit_Framework_Constraint $this->value = $value; } - /** - * Evaluates the constraint for parameter $other. Returns TRUE if the - * constraint is met, FALSE otherwise. - * - * @param mixed $other Value or object to evaluate. - * @return bool - */ - public function matches($other) + public function getResponse($url) { $options = array( - CURLOPT_URL => $other, + CURLOPT_URL => $url, CURLOPT_HEADER => false, CURLOPT_TIMEOUT => 1, CURLOPT_RETURNTRANSFER => true @@ -41,7 +34,19 @@ class HttpResponseText extends \PHPUnit_Framework_Constraint $response = @curl_exec($ch); curl_close($ch); - $this->actualCode = $response; + return $response; + } + + /** + * Evaluates the constraint for parameter $other. Returns TRUE if the + * constraint is met, FALSE otherwise. + * + * @param mixed $other Value or object to evaluate. + * @return bool + */ + public function matches($other) + { + $this->actualCode = $this->getResponse($other); return $this->value === $this->actualCode; } diff --git a/tests/PHPUnit/Integration/API/RequestTest.php b/tests/PHPUnit/Integration/API/RequestTest.php index e66d75efb36754fcd86a5b966fea85a81c550b83..aab254cd6f52626f0e8c7650114ad37e6e00f2c5 100644 --- a/tests/PHPUnit/Integration/API/RequestTest.php +++ b/tests/PHPUnit/Integration/API/RequestTest.php @@ -84,6 +84,16 @@ class RequestTest extends IntegrationTestCase $this->assertTrue($this->access->hasSuperUserAccess()); } + public function test_isApiRequest_shouldDetectIfItIsApiRequestOrNot() + { + $this->assertFalse(Request::isApiRequest(array())); + $this->assertFalse(Request::isApiRequest(array('module' => '', 'method' => ''))); + $this->assertFalse(Request::isApiRequest(array('module' => 'API'))); // no method + $this->assertFalse(Request::isApiRequest(array('module' => 'CoreHome', 'method' => 'index.test'))); // not api + $this->assertFalse(Request::isApiRequest(array('module' => 'API', 'method' => 'testmethod'))); // no valid action + $this->assertTrue(Request::isApiRequest(array('module' => 'API', 'method' => 'test.method'))); + } + private function assertSameUserAsBeforeIsAuthenticated() { $this->assertEquals($this->userAuthToken, $this->access->getTokenAuth());