Merge branch 'bugfix/monitoring-controllers-response-codes-6281'

fixes #6281

application/controllers/ErrorController.php

@@ -3,6 +3,8 @@
 
 use Icinga\Application\Icinga;
 use Icinga\Application\Logger;
+use Icinga\Exception\Http\HttpMethodNotAllowedException;
+use Icinga\Exception\Http\HttpNotFoundException;
 use Icinga\Exception\MissingParameterException;
 use Icinga\Security\SecurityException;
 use Icinga\Web\Controller\ActionController;
@@ -34,11 +36,7 @@ class ErrorController extends ActionController
                 $path = preg_split('~/~', $path);
                 $path = array_shift($path);
                 $this->getResponse()->setHttpResponseCode(404);
-                $title = preg_replace('/\r?\n.*$/s', '', $exception->getMessage());
+                $this->view->message = $this->translate('Page not found.');
-                $this->view->title = 'Server error: ' . $title;
-                if ($this->getInvokeArg('displayExceptions')) {
-                    $this->view->stackTrace = $exception->getTraceAsString();
-                }
                 if ($modules->hasInstalled($path) && ! $modules->hasEnabled($path)) {
                     $this->view->message .= ' ' . sprintf(
                         $this->translate('Enabling the "%s" module might help!'),
@@ -49,8 +47,12 @@ class ErrorController extends ActionController
                 break;
             default:
                 switch (true) {
-                    case $exception instanceof SecurityException:
+                    case $exception instanceof HttpMethodNotAllowedException:
-                        $this->getResponse()->setHttpResponseCode(403);
+                        $this->getResponse()->setHttpResponseCode(405);
+                        $this->getResponse()->setHeader('Allow', $exception->getAllowedMethods());
+                        break;
+                    case $exception instanceof HttpNotFoundException:
+                        $this->getResponse()->setHttpResponseCode(404);
                         break;
                     case $exception instanceof MissingParameterException:
                         $this->getResponse()->setHttpResponseCode(400);
@@ -59,12 +61,13 @@ class ErrorController extends ActionController
                             'Missing parameter ' . $exception->getParameter()
                         );
                         break;
+                    case $exception instanceof SecurityException:
+                        $this->getResponse()->setHttpResponseCode(403);
+                        break;
                     default:
                         $this->getResponse()->setHttpResponseCode(500);
                         break;
                 }
-                $title = preg_replace('/\r?\n.*$/s', '', $exception->getMessage());
-                $this->view->title = 'Server error: ' . $title;
                 $this->view->message = $exception->getMessage();
                 if ($this->getInvokeArg('displayExceptions')) {
                     $this->view->stackTrace = $exception->getTraceAsString();

application/views/scripts/error/error.phtml

@@ -1,13 +1,8 @@
 <div class="controls">
 <?= $this->tabs->showOnlyCloseButton() ?>
-<?php if ($this->title): ?>
-<h1><?= $this->escape($title) ?></h1>
-<?php endif ?>
 </div>
 <div class="content">
-<?php if ($this->message): ?>
   <p><strong><?= nl2br($this->escape($message)) ?></strong></p>
-<?php endif ?>
 <?php if (isset($stackTrace)) : ?>
   <hr />
   <pre><?= $this->escape($stackTrace) ?></pre>

library/Icinga/Exception/Http/HttpException.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Icinga\Exception\Http;
+
+use Icinga\Exception\IcingaException;
+
+/**
+ * Base class for HTTP exceptions
+ */
+abstract class HttpException extends IcingaException
+{
+
+}

library/Icinga/Exception/Http/HttpMethodNotAllowedException.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace Icinga\Exception\Http;
+
+/**
+ * Exception thrown if the HTTP method is not allowed
+ */
+class HttpMethodNotAllowedException extends HttpException
+{
+    /**
+     * Allowed HTTP methods
+     *
+     * @var string
+     */
+    protected $allowedMethods;
+
+    /**
+     * Get the allowed HTTP methods
+     *
+     * @return string
+     */
+    public function getAllowedMethods()
+    {
+        return $this->allowedMethods;
+    }
+
+    /**
+     * Set the allowed HTTP methods
+     *
+     * @param   string $allowedMethods
+     *
+     * @return  $this
+     */
+    public function setAllowedMethods($allowedMethods)
+    {
+        $this->allowedMethods = (string) $allowedMethods;
+        return $this;
+    }
+}

library/Icinga/Exception/Http/HttpNotFoundException.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Icinga\Exception\Http;
+
+/**
+ * Exception thrown for sending a HTTP 404 response w/ a custom message
+ */
+class HttpNotFoundException extends HttpException
+{
+
+}

library/Icinga/Web/Controller.php

@@ -3,9 +3,9 @@
 
 namespace Icinga\Web;
 
-use Zend_Controller_Action_Exception;
 use Icinga\Data\Sortable;
 use Icinga\Data\QueryInterface;
+use Icinga\Exception\Http\HttpNotFoundException;
 use Icinga\Web\Controller\ModuleActionController;
 use Icinga\Web\Widget\Limiter;
 use Icinga\Web\Widget\Paginator;
@@ -55,11 +55,11 @@ class Controller extends ModuleActionController
      *
      * @param   $message
      *
-     * @throws  Zend_Controller_Action_Exception
+     * @throws  HttpNotFoundException
      */
     public function httpNotFound($message)
     {
-        throw new Zend_Controller_Action_Exception($message, 404);
+        throw new HttpNotFoundException($message);
     }
 
     /**

library/Icinga/Web/Controller/ActionController.php

@@ -7,6 +7,7 @@ use Exception;
 use Icinga\Application\Benchmark;
 use Icinga\Application\Config;
 use Icinga\Authentication\Manager;
+use Icinga\Exception\Http\HttpMethodNotAllowedException;
 use Icinga\Exception\IcingaException;
 use Icinga\Exception\ProgrammingError;
 use Icinga\File\Pdf;
@@ -192,16 +193,17 @@ class ActionController extends Zend_Controller_Action
     /**
      * Respond with HTTP 405 if the current request's method is not one of the given methods
      *
-     * @param   string $httpMethod                  Unlimited number of allowed HTTP methods
+     * @param   string $httpMethod              Unlimited number of allowed HTTP methods
      *
-     * @throws  \Zend_Controller_Action_Exception   If the request method is not one of the given methods
+     * @throws  HttpMethodNotAllowedException   If the request method is not one of the given methods
      */
     public function assertHttpMethod($httpMethod)
     {
         $httpMethods = array_flip(array_map('strtoupper', func_get_args()));
         if (! isset($httpMethods[$this->getRequest()->getMethod()])) {
-            $this->getResponse()->setHeader('Allow', implode(', ', array_keys($httpMethods)));
+            $e = new HttpMethodNotAllowedException($this->translate('Method Not Allowed'));
-            throw new \Zend_Controller_Action_Exception($this->translate('Method Not Allowed'), 405);
+            $e->setAllowedMethods(implode(', ', array_keys($httpMethods)));
+            throw $e;
         }
     }
 

modules/monitoring/application/controllers/CommentController.php

@@ -20,13 +20,11 @@ class Monitoring_CommentController extends Controller
 
     /**
      * Fetch the first comment with the given id and add tabs
-     *
-     * @throws Zend_Controller_Action_Exception
      */
     public function init()
     {
-        $commentId = $this->params->get('comment_id');
+        $commentId = $this->params->getRequired('comment_id');
-         
+
         $this->comment = $this->backend->select()->from('comment', array(
             'id'         => 'comment_internal_id',
             'objecttype' => 'comment_objecttype',
@@ -41,9 +39,9 @@ class Monitoring_CommentController extends Controller
             'host_display_name',
             'service_display_name'
         ))->where('comment_internal_id', $commentId)->getQuery()->fetchRow();
-        
+
-        if (false === $this->comment) {
+        if ($this->comment === false) {
-            throw new Zend_Controller_Action_Exception($this->translate('Comment not found'));
+            $this->httpNotFound($this->translate('Comment not found'));
         }
 
         $this->getTabs()->add(
@@ -88,7 +86,7 @@ class Monitoring_CommentController extends Controller
     private function createDelCommentForm()
     {
         $this->assertPermission('monitoring/command/comment/delete');
-        
+
         $delCommentForm = new DeleteCommentCommandForm();
         $delCommentForm->setAction(
             Url::fromPath('monitoring/comment/show')

modules/monitoring/application/controllers/DowntimeController.php

@@ -30,13 +30,11 @@ class Monitoring_DowntimeController extends Controller
 
     /**
      * Fetch the downtime matching the given id and add tabs
-     *
-     * @throws Zend_Controller_Action_Exception
      */
     public function init()
     {
-        $downtimeId = $this->params->get('downtime_id');
+        $downtimeId = $this->params->getRequired('downtime_id');
-         
+
         $this->downtime = $this->backend->select()->from('downtime', array(
             'id'              => 'downtime_internal_id',
             'objecttype'      => 'downtime_objecttype',
@@ -60,17 +58,17 @@ class Monitoring_DowntimeController extends Controller
             'host_display_name',
             'service_display_name'
         ))->where('downtime_internal_id', $downtimeId)->getQuery()->fetchRow();
-        
+
-        if (false === $this->downtime) {
+        if ($this->downtime === false) {
-            throw new Zend_Controller_Action_Exception($this->translate('Downtime not found'));
+            $this->httpNotFound($this->translate('Downtime not found'));
         }
-        
+
         if (isset($this->downtime->service_description)) {
             $this->isService = true;
         } else {
             $this->isService = false;
         }
-        
+
         $this->getTabs()
             ->add(
                 'downtime',

modules/monitoring/application/controllers/HostController.php

@@ -1,7 +1,6 @@
 <?php
 /* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
 
-use Icinga\Exception\MissingParameterException;
 use Icinga\Module\Monitoring\Forms\Command\Object\AcknowledgeProblemCommandForm;
 use Icinga\Module\Monitoring\Forms\Command\Object\AddCommentCommandForm;
 use Icinga\Module\Monitoring\Forms\Command\Object\ProcessCheckResultCommandForm;
@@ -22,27 +21,15 @@ class Monitoring_HostController extends MonitoredObjectController
 
     /**
      * Fetch the requested host from the monitoring backend
-     *
-     * @throws Zend_Controller_Action_Exception If the host was not found
      */
     public function init()
     {
-        if ($this->params->get('host') === null) {
+        $host = new Host($this->backend, $this->params->getRequired('host'));
-            throw new MissingParameterException(
-                $this->translate('Required parameter \'%s\' is missing'),
-                'host'
-            );
-        }
-
-        $host = new Host($this->backend, $this->params->get('host'));
 
         $this->applyRestriction('monitoring/hosts/filter', $host);
 
         if ($host->fetch() === false) {
-            throw new Zend_Controller_Action_Exception(
+            $this->httpNotFound($this->translate('Host not found'));
-                sprintf($this->translate('Host \'%s\' not found'), $this->params->get('host')),
-                404
-            );
         }
         $this->object = $host;
         $this->createTabs();

modules/monitoring/application/controllers/ServiceController.php

@@ -1,7 +1,6 @@
 <?php
 /* Icinga Web 2 | (c) 2013-2015 Icinga Development Team | GPLv2+ */
 
-use Icinga\Exception\MissingParameterException;
 use Icinga\Module\Monitoring\Forms\Command\Object\AcknowledgeProblemCommandForm;
 use Icinga\Module\Monitoring\Forms\Command\Object\AddCommentCommandForm;
 use Icinga\Module\Monitoring\Forms\Command\Object\ProcessCheckResultCommandForm;
@@ -22,27 +21,17 @@ class Monitoring_ServiceController extends MonitoredObjectController
 
     /**
      * Fetch the requested service from the monitoring backend
-     *
-     * @throws Zend_Controller_Action_Exception If the service was not found
      */
     public function init()
     {
-        if ($this->params->get('host') === null || $this->params->get('service') === null) {
+        $service = new Service(
-            throw new MissingParameterException(
+            $this->backend, $this->params->getRequired('host'), $this->params->getRequired('service')
-                $this->translate('One of the required parameters \'%s\' is missing'),
+        );
-                'host or service'
-            );
-        }
-
-        $service = new Service($this->backend, $this->params->get('host'), $this->params->get('service'));
 
         $this->applyRestriction('monitoring/services/filter', $service);
 
         if ($service->fetch() === false) {
-            throw new Zend_Controller_Action_Exception(
+            $this->httpNotFound($this->translate('Service not found'));
-                sprintf($this->translate('Service \'%s\' not found'), $this->params->get('service')),
-                404
-            );
         }
         $this->object = $service;
         $this->createTabs();