1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 21:02:41 +01:00

Throw when callers pass an invalid constraint to a "*.search" method

Summary:
Ref T11593. When you call a `*.search` method like `maniphest.search`, we don't currently validate that all the constraints you pass are recognized.

I think there were two very weak arguments for not doing this:

  - It makes compatibility in `arc` across versions slightly easier: if we add a new constraint, we could add it to `arc` but also do client-side filtering for a while.
  - Conduit parameter types //could//, in theory, accept multiple inputs or optional/alias inputs.

These reasons are pretty fluff and T11593 is a concrete issue caused by not validating. Just validate instead.

Test Plan:
  - Made a `maniphest.search` call with a bogus constraint, got an explicit error about the bad constraint.
  - Made a `maniphest.search` call with a valid constraint (`"ids"`).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11593

Differential Revision: https://secure.phabricator.com/D16507
This commit is contained in:
epriestley 2016-09-07 07:21:41 -07:00
parent 9ced2d7eb5
commit 38ae81fb39
3 changed files with 28 additions and 0 deletions

View file

@ -38,6 +38,9 @@ abstract class ConduitParameterType extends Phobject {
return $this->getParameterValue($request, $key); return $this->getParameterValue($request, $key);
} }
final public function getKeys($key) {
return $this->getParameterKeys($key);
}
final public function getDefaultValue() { final public function getDefaultValue() {
return $this->getParameterDefault(); return $this->getParameterDefault();
@ -86,6 +89,10 @@ abstract class ConduitParameterType extends Phobject {
return $request[$key]; return $request[$key];
} }
protected function getParameterKeys($key) {
return array($key);
}
abstract protected function getParameterTypeName(); abstract protected function getParameterTypeName();

View file

@ -1094,6 +1094,22 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
} }
} }
$valid_constraints = array();
foreach ($fields as $field) {
foreach ($field->getValidConstraintKeys() as $key) {
$valid_constraints[$key] = true;
}
}
foreach ($constraints as $key => $constraint) {
if (empty($valid_constraints[$key])) {
throw new Exception(
pht(
'Constraint "%s" is not a valid constraint for this query.',
$key));
}
}
foreach ($fields as $field) { foreach ($fields as $field) {
if (!$field->getValueExistsInConduitRequest($constraints)) { if (!$field->getValueExistsInConduitRequest($constraints)) {
continue; continue;

View file

@ -329,6 +329,11 @@ abstract class PhabricatorSearchField extends Phobject {
$this->getConduitKey()); $this->getConduitKey());
} }
public function getValidConstraintKeys() {
return $this->getConduitParameterType()->getKeys(
$this->getConduitKey());
}
/* -( Utility Methods )----------------------------------------------------- */ /* -( Utility Methods )----------------------------------------------------- */