diff --git a/web/api/app/Controller/Component/FilterComponent.php b/web/api/app/Controller/Component/FilterComponent.php index c649a1f8b..ba8a9bc56 100644 --- a/web/api/app/Controller/Component/FilterComponent.php +++ b/web/api/app/Controller/Component/FilterComponent.php @@ -73,8 +73,16 @@ class FilterComponent extends Component { ); // Build a CakePHP find() condition based on the named parameters - // that are passed in - public function buildFilter($namedParams) { + // that are passed in. + // + // $validFields, when given, is the list of field names a caller may filter + // on. Without it an unknown name is passed straight through into the SQL and + // surfaces as a PDOException, so a caller asking for a field that does not + // exist - or one that lives on Monitors rather than the model being queried, + // such as MonitorName - got an opaque 500 with a stack trace rather than a + // message naming the field. Callers that have not been given a list keep the + // old behaviour. + public function buildFilter($namedParams, $validFields = null) { $conditions = array(); if ($namedParams) { foreach ($namedParams as $attribute => $value) { @@ -85,14 +93,25 @@ class FilterComponent extends Component { } $matches = NULL; if (preg_match('/^(?P[a-z0-9\.]+)(?P.+)?$/i', $lhs, $matches) !== 1) { - throw new Exception('Invalid argument before `:`: ' . $lhs); + throw new BadRequestException('Invalid argument before `:`: ' . $lhs); } $operator = isset($matches['operator']) ? trim($matches['operator']) : ''; // Only allow operators on our allow list. No operator // specified defaults to `=` by cakePHP. if ($operator != '' && !in_array($operator, $this->twoOperandSQLOperands)) { - throw new Exception('Invalid operator: ' . $operator); + throw new BadRequestException('Invalid operator: ' . $operator); + } + + if ($validFields !== null) { + // A field may be written Model.Field; only the column is checked. + $field = $matches['field']; + $bare = (false !== strpos($field, '.')) + ? substr($field, strrpos($field, '.') + 1) + : $field; + if (!in_array($bare, $validFields)) { + throw new BadRequestException('Unknown filter field: ' . $bare); + } } if (($operator == 'LIKE') and (false === strpos($value, '%'))) { $value = '%'.$value.'%'; diff --git a/web/api/app/Controller/EventsController.php b/web/api/app/Controller/EventsController.php index 937863c5f..c4a8641b1 100644 --- a/web/api/app/Controller/EventsController.php +++ b/web/api/app/Controller/EventsController.php @@ -74,7 +74,14 @@ class EventsController extends AppController { unset($named_params[$k]); } } - $conditions = $this->FilterComponent->buildFilter($named_params); + // Events columns, plus the two names this action resolves itself: DateTime + // is a pseudo-attribute turned into an overlap test below, and GroupId is + // served by the Groups_Monitors join added further down. Anything else + // would reach MySQL as an unknown column and come back as a 500. + $valid_fields = array_keys($this->Event->schema()); + $valid_fields[] = 'DateTime'; + $valid_fields[] = 'GroupId'; + $conditions = $this->FilterComponent->buildFilter($named_params, $valid_fields); #ZM\Debug(print_r($conditions, true)); foreach ($conditions as $k=>$v) { if ( 0 === strpos($k, 'DateTime') ) {