diff --git a/src/applications/conduit/method/maniphest/base/ConduitAPI_maniphest_Method.php b/src/applications/conduit/method/maniphest/base/ConduitAPI_maniphest_Method.php index a3dee43da0..2855d221a6 100644 --- a/src/applications/conduit/method/maniphest/base/ConduitAPI_maniphest_Method.php +++ b/src/applications/conduit/method/maniphest/base/ConduitAPI_maniphest_Method.php @@ -21,6 +21,12 @@ */ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { + public function defineErrorTypes() { + return array( + 'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.' + ); + } + protected function buildTaskInfoDictionary(ManiphestTask $task) { $results = $this->buildTaskInfoDictionaries(array($task)); return idx($results, $task->getPHID()); @@ -88,32 +94,54 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $status = $request->getValue('status'); if ($status !== null) { + $valid_statuses = ManiphestTaskStatus::getTaskStatusMap(); + if (!isset($valid_statuses[$status])) { + throw id(new ConduitException('ERR-INVALID-PARAMETER')) + ->setErrorDescription('Status set to invalid value.'); + } $changes[ManiphestTransactionType::TYPE_STATUS] = $status; } } $priority = $request->getValue('priority'); if ($priority !== null) { + $valid_priorities = ManiphestTaskPriority::getTaskPriorityMap(); + if (!isset($valid_priorities[$priority])) { + throw id(new ConduitException('ERR-INVALID-PARAMETER')) + ->setErrorDescription('Priority set to invalid value.'); + } $changes[ManiphestTransactionType::TYPE_PRIORITY] = $priority; } $owner_phid = $request->getValue('ownerPHID'); if ($owner_phid !== null) { + $this->validatePHIDList(array($owner_phid), + PhabricatorPHIDConstants::PHID_TYPE_USER, + 'ownerPHID'); $changes[ManiphestTransactionType::TYPE_OWNER] = $owner_phid; } $ccs = $request->getValue('ccPHIDs'); if ($ccs !== null) { + $this->validatePHIDList($ccs, + PhabricatorPHIDConstants::PHID_TYPE_USER, + 'ccPHIDS'); $changes[ManiphestTransactionType::TYPE_CCS] = $ccs; } $project_phids = $request->getValue('projectPHIDs'); if ($project_phids !== null) { + $this->validatePHIDList($project_phids, + PhabricatorPHIDConstants::PHID_TYPE_PROJ, + 'projectPHIDS'); $changes[ManiphestTransactionType::TYPE_PROJECTS] = $project_phids; } $file_phids = $request->getValue('filePHIDs'); if ($file_phids !== null) { + $this->validatePHIDList($file_phids, + PhabricatorPHIDConstants::PHID_TYPE_FILE, + 'filePHIDS'); $file_map = array_fill_keys($file_phids, true); $attached = $task->getAttached(); $attached[PhabricatorPHIDConstants::PHID_TYPE_FILE] = $file_map; @@ -223,4 +251,24 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { return $result; } + /** + * Note this is a temporary stop gap since its easy to make malformed Tasks. + * Long-term, the values set in @{method:defineParamTypes} will be used to + * validate data implicitly within the larger Conduit application. + * + * TODO -- remove this in favor of generalized Conduit hotness + */ + private function validatePHIDList(array $phid_list, $phid_type, $field) { + $phid_groups = phid_group_by_type($phid_list); + unset($phid_groups[$phid_type]); + if (!empty($phid_groups)) { + throw id(new ConduitException('ERR-INVALID-PARAMETER')) + ->setErrorDescription( + 'One or more PHIDs were invalid for '.$field.'.' + ); + } + + return true; + } + } diff --git a/src/applications/conduit/method/maniphest/base/__init__.php b/src/applications/conduit/method/maniphest/base/__init__.php index 31771cefab..1067b1f930 100644 --- a/src/applications/conduit/method/maniphest/base/__init__.php +++ b/src/applications/conduit/method/maniphest/base/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/conduit/method/base'); +phutil_require_module('phabricator', 'applications/conduit/protocol/exception'); phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype'); @@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/auxiliary') phutil_require_module('phabricator', 'applications/maniphest/storage/transaction'); phutil_require_module('phabricator', 'applications/metamta/contentsource/source'); phutil_require_module('phabricator', 'applications/phid/constants'); +phutil_require_module('phabricator', 'applications/phid/utils'); phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'infrastructure/events/constant/type'); phutil_require_module('phabricator', 'infrastructure/events/event'); diff --git a/src/applications/conduit/method/maniphest/createtask/ConduitAPI_maniphest_createtask_Method.php b/src/applications/conduit/method/maniphest/createtask/ConduitAPI_maniphest_createtask_Method.php index b8f988fdd2..bc6c49adaf 100644 --- a/src/applications/conduit/method/maniphest/createtask/ConduitAPI_maniphest_createtask_Method.php +++ b/src/applications/conduit/method/maniphest/createtask/ConduitAPI_maniphest_createtask_Method.php @@ -36,6 +36,7 @@ final class ConduitAPI_maniphest_createtask_Method public function defineErrorTypes() { return array( + 'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.' ); } diff --git a/src/applications/conduit/method/maniphest/update/ConduitAPI_maniphest_update_Method.php b/src/applications/conduit/method/maniphest/update/ConduitAPI_maniphest_update_Method.php index 4e8bae0d0e..4d4adf1039 100644 --- a/src/applications/conduit/method/maniphest/update/ConduitAPI_maniphest_update_Method.php +++ b/src/applications/conduit/method/maniphest/update/ConduitAPI_maniphest_update_Method.php @@ -26,6 +26,13 @@ final class ConduitAPI_maniphest_update_Method return "Update an existing Maniphest task."; } + public function defineErrorTypes() { + return array( + 'ERR-BAD-TASK' => 'No such maniphest task exists.', + 'ERR-INVALID-PARAMETER' => 'Missing or malformed parameter.' + ); + } + public function defineParamTypes() { return $this->getTaskFields($is_new = false); } @@ -34,12 +41,6 @@ final class ConduitAPI_maniphest_update_Method return 'nonempty dict'; } - public function defineErrorTypes() { - return array( - 'ERR-BAD-TASK' => 'No such task exists.', - ); - } - protected function execute(ConduitAPIRequest $request) { $id = $request->getValue('id'); $phid = $request->getValue('phid');