From f84e31b644a09ab960770f6ba89da1bc1e340846 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 11:47:42 +0100 Subject: [PATCH 1/7] Enabling the exception / error handler for console mode only. Also fixing a catchable fatal error in the BuildFactory class. --- PHPCI/BuildFactory.php | 5 +++++ bootstrap.php | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/PHPCI/BuildFactory.php b/PHPCI/BuildFactory.php index 82ebc221..3f48eec0 100644 --- a/PHPCI/BuildFactory.php +++ b/PHPCI/BuildFactory.php @@ -21,11 +21,16 @@ class BuildFactory /** * @param $buildId * @return Build + * @throws \Exception */ public static function getBuildById($buildId) { $build = Factory::getStore('Build')->getById($buildId); + if (empty($build)) { + throw new \Exception('Build ID ' . $buildId . ' does not exist.'); + } + return self::getBuild($build); } diff --git a/bootstrap.php b/bootstrap.php index 2a271503..3b951a43 100755 --- a/bootstrap.php +++ b/bootstrap.php @@ -59,8 +59,10 @@ if (!file_exists(dirname(__FILE__) . '/vendor/autoload.php') && defined('PHPCI_I // Load Composer autoloader: require_once(dirname(__FILE__) . '/vendor/autoload.php'); -$loggerConfig = LoggerConfig::newFromFile(__DIR__ . "/loggerconfig.php"); -Handler::register($loggerConfig->getFor('_')); +if (defined('PHPCI_IS_CONSOLE') && PHPCI_IS_CONSOLE) { + $loggerConfig = LoggerConfig::newFromFile(__DIR__ . "/loggerconfig.php"); + Handler::register($loggerConfig->getFor('_')); +} // Load configuration if present: $conf = array(); From 22b1245e9b9ec8597825c03d82cfe17013dda951 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:04:42 +0100 Subject: [PATCH 2/7] Adding some form of exception handling to front-end requests --- PHPCI/Application.php | 11 ++++++++++- PHPCI/View/exception.phtml | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 PHPCI/View/exception.phtml diff --git a/PHPCI/Application.php b/PHPCI/Application.php index 5d1d9f9d..e4532d3b 100644 --- a/PHPCI/Application.php +++ b/PHPCI/Application.php @@ -10,6 +10,7 @@ namespace PHPCI; use b8; +use b8\Exception\HttpException; use b8\Http\Response; use b8\Http\Response\RedirectResponse; use b8\View; @@ -70,7 +71,15 @@ class Application extends b8\Application */ public function handleRequest() { - $this->response = parent::handleRequest(); + try { + $this->response = parent::handleRequest(); + } catch (\Exception $ex) { + $this->config->set('page_title', 'Error'); + + $view = new View('exception'); + $view->exception = $ex; + $this->response->setContent($view->render()); + } if (View::exists('layout') && $this->response->hasLayout()) { $view = new View('layout'); diff --git a/PHPCI/View/exception.phtml b/PHPCI/View/exception.phtml new file mode 100644 index 00000000..1ecbe6a4 --- /dev/null +++ b/PHPCI/View/exception.phtml @@ -0,0 +1,9 @@ +
+
+

Sorry, there was a problem

+
+ +
+ getMessage(); ?> +
+
\ No newline at end of file From 1120e310366ba60b476cf8fbd1539647a325421e Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:10:48 +0100 Subject: [PATCH 3/7] Updating exception handler to return the appropriate HTTP response code --- PHPCI/Application.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/PHPCI/Application.php b/PHPCI/Application.php index e4532d3b..011e27e5 100644 --- a/PHPCI/Application.php +++ b/PHPCI/Application.php @@ -73,11 +73,21 @@ class Application extends b8\Application { try { $this->response = parent::handleRequest(); + } catch (HttpException $ex) { + $this->config->set('page_title', 'Error'); + + $view = new View('exception'); + $view->exception = $ex; + + $this->response->setResponseCode($ex->getErrorCode()); + $this->response->setContent($view->render()); } catch (\Exception $ex) { $this->config->set('page_title', 'Error'); $view = new View('exception'); $view->exception = $ex; + + $this->response->setResponseCode(500); $this->response->setContent($view->render()); } From fe813b057cb29437ce244380d557b898d02f6fdd Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:11:02 +0100 Subject: [PATCH 4/7] Cleaning up exceptions in the Project controller --- PHPCI/Controller/ProjectController.php | 59 +++++++++++++++----------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/PHPCI/Controller/ProjectController.php b/PHPCI/Controller/ProjectController.php index eea5a7a8..c06676bd 100644 --- a/PHPCI/Controller/ProjectController.php +++ b/PHPCI/Controller/ProjectController.php @@ -9,17 +9,17 @@ namespace PHPCI\Controller; +use b8; +use b8\Controller; +use b8\Form; +use b8\Exception\HttpException\ForbiddenException; +use b8\Exception\HttpException\NotFoundException; +use b8\Store; use PHPCI\BuildFactory; use PHPCI\Helper\Github; use PHPCI\Helper\SshKey; use PHPCI\Model\Build; use PHPCI\Model\Project; -use b8; -use b8\Config; -use b8\Controller; -use b8\Store; -use b8\Form; -use b8\Exception\HttpException\NotFoundException; /** * Project Controller - Allows users to create, edit and view projects. @@ -41,8 +41,8 @@ class ProjectController extends \PHPCI\Controller public function init() { - $this->buildStore = Store\Factory::getStore('Build'); - $this->projectStore = Store\Factory::getStore('Project'); + $this->buildStore = Store\Factory::getStore('Build'); + $this->projectStore = Store\Factory::getStore('Project'); } /** @@ -51,17 +51,18 @@ class ProjectController extends \PHPCI\Controller public function view($projectId) { $project = $this->projectStore->getById($projectId); - if (!$project) { + + if (empty($project)) { throw new NotFoundException('Project with id: ' . $projectId . ' not found'); } - $page = $this->getParam('p', 1); - $builds = $this->getLatestBuildsHtml($projectId, (($page - 1) * 10)); + $page = $this->getParam('p', 1); + $builds = $this->getLatestBuildsHtml($projectId, (($page - 1) * 10)); - $this->view->builds = $builds[0]; - $this->view->total = $builds[1]; - $this->view->project = $project; - $this->view->page = $page; + $this->view->builds = $builds[0]; + $this->view->total = $builds[1]; + $this->view->project = $project; + $this->view->page = $page; $this->config->set('page_title', $project->getTitle()); @@ -76,6 +77,10 @@ class ProjectController extends \PHPCI\Controller /* @var \PHPCI\Model\Project $project */ $project = $this->projectStore->getById($projectId); + if (empty($project)) { + throw new NotFoundException('Project with id: ' . $projectId . ' not found'); + } + $build = new Build(); $build->setProjectId($projectId); $build->setCommitId('Manual'); @@ -96,7 +101,7 @@ class ProjectController extends \PHPCI\Controller public function delete($projectId) { if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } $project = $this->projectStore->getById($projectId); @@ -120,10 +125,10 @@ class ProjectController extends \PHPCI\Controller */ protected function getLatestBuildsHtml($projectId, $start = 0) { - $criteria = array('project_id' => $projectId); - $order = array('id' => 'DESC'); - $builds = $this->buildStore->getWhere($criteria, 10, $start, array(), $order); - $view = new b8\View('BuildsTable'); + $criteria = array('project_id' => $projectId); + $order = array('id' => 'DESC'); + $builds = $this->buildStore->getWhere($criteria, 10, $start, array(), $order); + $view = new b8\View('BuildsTable'); foreach ($builds['items'] as &$build) { $build = BuildFactory::getBuild($build); @@ -142,7 +147,7 @@ class ProjectController extends \PHPCI\Controller $this->config->set('page_title', 'Add Project'); if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } $method = $this->request->getMethod(); @@ -210,11 +215,15 @@ class ProjectController extends \PHPCI\Controller public function edit($projectId) { if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } - $method = $this->request->getMethod(); - $project = $this->projectStore->getById($projectId); + $method = $this->request->getMethod(); + $project = $this->projectStore->getById($projectId); + + if (empty($project)) { + throw new NotFoundException('Project with id: ' . $projectId . ' not found'); + } $this->config->set('page_title', 'Edit: ' . $project->getTitle()); @@ -234,7 +243,7 @@ class ProjectController extends \PHPCI\Controller } - $form = $this->projectForm($values, 'edit/' . $projectId); + $form = $this->projectForm($values, 'edit/' . $projectId); if ($method != 'POST' || ($method == 'POST' && !$form->validate())) { $view = new b8\View('ProjectForm'); From 7662202a39ec2ccf93e323cb81e7f9144de01652 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:15:11 +0100 Subject: [PATCH 5/7] Cleaning up exceptions in the Build controller --- PHPCI/Controller/BuildController.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/PHPCI/Controller/BuildController.php b/PHPCI/Controller/BuildController.php index 3ebb8432..1dff5a15 100644 --- a/PHPCI/Controller/BuildController.php +++ b/PHPCI/Controller/BuildController.php @@ -10,6 +10,7 @@ namespace PHPCI\Controller; use b8; +use b8\Exception\HttpException\NotFoundException; use PHPCI\BuildFactory; use PHPCI\Model\Build; @@ -36,7 +37,16 @@ class BuildController extends \PHPCI\Controller */ public function view($buildId) { - $build = BuildFactory::getBuildById($buildId); + try { + $build = BuildFactory::getBuildById($buildId); + } catch (\Exception $ex) { + $build = null; + } + + if (empty($build)) { + throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); + } + $this->view->plugins = $this->getUiPlugins(); $this->view->build = $build; $this->view->data = $this->getBuildData($build); @@ -110,6 +120,10 @@ class BuildController extends \PHPCI\Controller { $copy = BuildFactory::getBuildById($buildId); + if (empty($copy)) { + throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); + } + $build = new Build(); $build->setProjectId($copy->getProjectId()); $build->setCommitId($copy->getCommitId()); @@ -134,11 +148,10 @@ class BuildController extends \PHPCI\Controller throw new \Exception('You do not have permission to do that.'); } - $build = BuildFactory::getBuildById($buildId); + $build = BuildFactory::getBuildById($buildId); - if (!$build) { - $this->response->setResponseCode(404); - return '404 - Not Found'; + if (empty($build)) { + throw new NotFoundException('Build with ID: ' . $buildId . ' does not exist.'); } $this->buildStore->delete($build); From e589cfcefd3152ac5e5b328836b9078ce7bc5182 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:16:24 +0100 Subject: [PATCH 6/7] Cleaning up exceptions in the BuildStatus controller --- PHPCI/Controller/BuildStatusController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PHPCI/Controller/BuildStatusController.php b/PHPCI/Controller/BuildStatusController.php index 74a1fbad..8c34cd63 100644 --- a/PHPCI/Controller/BuildStatusController.php +++ b/PHPCI/Controller/BuildStatusController.php @@ -65,7 +65,8 @@ class BuildStatusController extends \PHPCI\Controller public function view($projectId) { $project = $this->projectStore->getById($projectId); - if (!$project) { + + if (empty($project)) { throw new NotFoundException('Project with id: ' . $projectId . ' not found'); } From 467232a9149d02b1b6955ba8cd5909ca75bb2881 Mon Sep 17 00:00:00 2001 From: Dan Cryer Date: Fri, 9 May 2014 12:19:48 +0100 Subject: [PATCH 7/7] Cleaning up exceptions in the User controller --- PHPCI/Controller/UserController.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/PHPCI/Controller/UserController.php b/PHPCI/Controller/UserController.php index 817cb5e8..93de822e 100644 --- a/PHPCI/Controller/UserController.php +++ b/PHPCI/Controller/UserController.php @@ -10,6 +10,8 @@ namespace PHPCI\Controller; use b8; +use b8\Exception\HttpException\ForbiddenException; +use b8\Exception\HttpException\NotFoundException; use b8\Form; use PHPCI\Controller; use PHPCI\Model\User; @@ -106,12 +108,11 @@ class UserController extends Controller public function add() { if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } $this->config->set('page_title', 'Add User'); - $method = $this->request->getMethod(); if ($method == 'POST') { @@ -150,14 +151,17 @@ class UserController extends Controller public function edit($userId) { if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } $method = $this->request->getMethod(); $user = $this->userStore->getById($userId); - $this->config->set('page_title', 'Edit: ' . $user->getName()); + if (empty($user)) { + throw new NotFoundException('User with ID: ' . $userId . ' does not exist.'); + } + $this->config->set('page_title', 'Edit: ' . $user->getName()); if ($method == 'POST') { $values = $this->getParams(); @@ -244,10 +248,15 @@ class UserController extends Controller public function delete($userId) { if (!$_SESSION['user']->getIsAdmin()) { - throw new \Exception('You do not have permission to do that.'); + throw new ForbiddenException('You do not have permission to do that.'); } $user = $this->userStore->getById($userId); + + if (empty($user)) { + throw new NotFoundException('User with ID: ' . $userId . ' does not exist.'); + } + $this->userStore->delete($user); header('Location: '.PHPCI_URL.'user');