1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Conduit accept int/bool parameters as strings

Summary: Accept Conduit parameter values as strings (e.g. from `curl`) and convert to required type.

Test Plan:
Call conduit method with int/bool parameter iusing `curl` and make sure it does not result in validation error, e.g.
```
$ curl http://$PHABRICATOR_HOST/api/maniphest.search -d api.token=$CONDUIT_TOKEN -d constraints[modifiedEnd]=$(date +%s) -d constraints[hasParents]=true -d limit=1
```

Fixes T10456.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T10456

Differential Revision: https://secure.phabricator.com/D16694
This commit is contained in:
Giedrius Dubinskas 2016-10-14 14:45:57 +00:00 committed by gd
parent 636eaf231e
commit c71bb0550c
21 changed files with 127 additions and 100 deletions

View file

@ -15,7 +15,7 @@ final class ConduitCall extends Phobject {
private $request; private $request;
private $user; private $user;
public function __construct($method, array $params) { public function __construct($method, array $params, $strictly_typed = true) {
$this->method = $method; $this->method = $method;
$this->handler = $this->buildMethodHandler($method); $this->handler = $this->buildMethodHandler($method);
@ -41,7 +41,7 @@ final class ConduitCall extends Phobject {
"'".implode("', '", array_keys($invalid_params))."'")); "'".implode("', '", array_keys($invalid_params))."'"));
} }
$this->request = new ConduitAPIRequest($params); $this->request = new ConduitAPIRequest($params, $strictly_typed);
} }
public function getAPIRequest() { public function getAPIRequest() {

View file

@ -25,9 +25,11 @@ final class PhabricatorConduitAPIController
try { try {
list($metadata, $params) = $this->decodeConduitParams($request, $method); list($metadata, $params, $strictly_typed) = $this->decodeConduitParams(
$request,
$method);
$call = new ConduitCall($method, $params); $call = new ConduitCall($method, $params, $strictly_typed);
$method_implementation = $call->getMethodImplementation(); $method_implementation = $call->getMethodImplementation();
$result = null; $result = null;
@ -638,7 +640,7 @@ final class PhabricatorConduitAPIController
$metadata = idx($params, '__conduit__', array()); $metadata = idx($params, '__conduit__', array());
unset($params['__conduit__']); unset($params['__conduit__']);
return array($metadata, $params); return array($metadata, $params, true);
} }
// Otherwise, look for a single parameter called 'params' which has the // Otherwise, look for a single parameter called 'params' which has the
@ -659,7 +661,7 @@ final class PhabricatorConduitAPIController
$metadata = idx($params, '__conduit__', array()); $metadata = idx($params, '__conduit__', array());
unset($params['__conduit__']); unset($params['__conduit__']);
return array($metadata, $params); return array($metadata, $params, true);
} }
// If we do not have `params`, assume this is a simple HTTP request with // If we do not have `params`, assume this is a simple HTTP request with
@ -675,7 +677,7 @@ final class PhabricatorConduitAPIController
} }
} }
return array($metadata, $params); return array($metadata, $params, false);
} }
private function authorizeOAuthMethodAccess( private function authorizeOAuthMethodAccess(

View file

@ -3,17 +3,9 @@
final class ConduitBoolParameterType final class ConduitBoolParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
return $this->parseBoolValue($request, $key, $value, $strict);
if (!is_bool($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected boolean (true or false), got something else.'));
}
return $value;
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -3,10 +3,10 @@
final class ConduitColumnsParameterType final class ConduitColumnsParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
// We don't do any meaningful validation here because the transaction // We don't do any meaningful validation here because the transaction
// itself validates everything and the input format is flexible. // itself validates everything and the input format is flexible.
return parent::getParameterValue($request, $key); return parent::getParameterValue($request, $key, $strict);
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -3,15 +3,9 @@
final class ConduitEpochParameterType final class ConduitEpochParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
$value = $this->parseIntValue($request, $key, $value, $strict);
if (!is_int($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected epoch timestamp as integer, got something else.'));
}
if ($value <= 0) { if ($value <= 0) {
$this->raiseValidationException( $this->raiseValidationException(

View file

@ -3,19 +3,11 @@
final class ConduitIntListParameterType final class ConduitIntListParameterType
extends ConduitListParameterType { extends ConduitListParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$list = parent::getParameterValue($request, $key); $list = parent::getParameterValue($request, $key, $strict);
foreach ($list as $idx => $item) { foreach ($list as $idx => $item) {
if (!is_int($item)) { $list[$idx] = $this->parseIntValue($request, $key.'['.$idx.']', $item);
$this->raiseValidationException(
$request,
$key,
pht(
'Expected a list of integers, but item with index "%s" is '.
'not an integer.',
$idx));
}
} }
return $list; return $list;

View file

@ -3,17 +3,9 @@
final class ConduitIntParameterType final class ConduitIntParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
return $this->parseIntValue($request, $key, $value, $strict);
if (!is_int($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected integer, got something else.'));
}
return $value;
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -14,8 +14,8 @@ abstract class ConduitListParameterType
return $this->allowEmptyList; return $this->allowEmptyList;
} }
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
if (!is_array($value)) { if (!is_array($value)) {
$this->raiseValidationException( $this->raiseValidationException(
@ -48,17 +48,18 @@ abstract class ConduitListParameterType
return $value; return $value;
} }
protected function validateStringList(array $request, $key, array $list) { protected function parseStringList(
array $request,
$key,
array $list,
$strict) {
foreach ($list as $idx => $item) { foreach ($list as $idx => $item) {
if (!is_string($item)) { $list[$idx] = $this->parseStringValue(
$this->raiseValidationException( $request,
$request, $key.'['.$idx.']',
$key, $item,
pht( $strict);
'Expected a list of strings, but item with index "%s" is '.
'not a string.',
$idx));
}
} }
return $list; return $list;

View file

@ -3,9 +3,9 @@
final class ConduitPHIDListParameterType final class ConduitPHIDListParameterType
extends ConduitListParameterType { extends ConduitListParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$list = parent::getParameterValue($request, $key); $list = parent::getParameterValue($request, $key, $strict);
return $this->validateStringList($request, $key, $list); return $this->parseStringList($request, $key, $list, $strict);
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -3,8 +3,8 @@
final class ConduitPHIDParameterType final class ConduitPHIDParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
if (!is_string($value)) { if (!is_string($value)) {
$this->raiseValidationException( $this->raiseValidationException(

View file

@ -30,12 +30,12 @@ abstract class ConduitParameterType extends Phobject {
} }
final public function getValue(array $request, $key) { final public function getValue(array $request, $key, $strict = true) {
if (!$this->getExists($request, $key)) { if (!$this->getExists($request, $key)) {
return $this->getParameterDefault(); return $this->getParameterDefault();
} }
return $this->getParameterValue($request, $key); return $this->getParameterValue($request, $key, $strict);
} }
final public function getKeys($key) { final public function getKeys($key) {
@ -85,7 +85,7 @@ abstract class ConduitParameterType extends Phobject {
return array_key_exists($key, $request); return array_key_exists($key, $request);
} }
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
return $request[$key]; return $request[$key];
} }
@ -93,6 +93,53 @@ abstract class ConduitParameterType extends Phobject {
return array($key); return array($key);
} }
protected function parseStringValue(array $request, $key, $value, $strict) {
if (!is_string($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected string, got something else.'));
}
return $value;
}
protected function parseIntValue(array $request, $key, $value, $strict) {
if (!$strict && is_string($value) && ctype_digit($value)) {
$value = $value + 0;
if (!is_int($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Integer overflow.'));
}
} else if (!is_int($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected integer, got something else.'));
}
return $value;
}
protected function parseBoolValue(array $request, $key, $value, $strict) {
$bool_strings = array(
'0' => false,
'1' => true,
'false' => false,
'true' => true,
);
if (!$strict && is_string($value) && isset($bool_strings[$value])) {
$value = $bool_strings[$value];
} else if (!is_bool($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected boolean (true or false), got something else.'));
}
return $value;
}
abstract protected function getParameterTypeName(); abstract protected function getParameterTypeName();

View file

@ -3,8 +3,8 @@
final class ConduitPointsParameterType final class ConduitPointsParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
if (($value !== null) && !is_numeric($value)) { if (($value !== null) && !is_numeric($value)) {
$this->raiseValidationException( $this->raiseValidationException(

View file

@ -3,9 +3,9 @@
final class ConduitProjectListParameterType final class ConduitProjectListParameterType
extends ConduitListParameterType { extends ConduitListParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$list = parent::getParameterValue($request, $key); $list = parent::getParameterValue($request, $key, $strict);
$list = $this->validateStringList($request, $key, $list); $list = $this->parseStringList($request, $key, $list, $strict);
return id(new PhabricatorProjectPHIDResolver()) return id(new PhabricatorProjectPHIDResolver())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
->resolvePHIDs($list); ->resolvePHIDs($list);

View file

@ -3,9 +3,9 @@
final class ConduitStringListParameterType final class ConduitStringListParameterType
extends ConduitListParameterType { extends ConduitListParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$list = parent::getParameterValue($request, $key); $list = parent::getParameterValue($request, $key, $strict);
return $this->validateStringList($request, $key, $list); return $this->parseStringList($request, $key, $list, $strict);
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -3,17 +3,9 @@
final class ConduitStringParameterType final class ConduitStringParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
return $this->parseStringValue($request, $key, $value, $strict);
if (!is_string($value)) {
$this->raiseValidationException(
$request,
$key,
pht('Expected string, got something else.'));
}
return $value;
} }
protected function getParameterTypeName() { protected function getParameterTypeName() {

View file

@ -3,9 +3,9 @@
final class ConduitUserListParameterType final class ConduitUserListParameterType
extends ConduitListParameterType { extends ConduitListParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$list = parent::getParameterValue($request, $key); $list = parent::getParameterValue($request, $key, $strict);
$list = $this->validateStringList($request, $key, $list); $list = $this->parseStringList($request, $key, $list, $strict);
return id(new PhabricatorUserPHIDResolver()) return id(new PhabricatorUserPHIDResolver())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
->resolvePHIDs($list); ->resolvePHIDs($list);

View file

@ -3,8 +3,8 @@
final class ConduitUserParameterType final class ConduitUserParameterType
extends ConduitParameterType { extends ConduitParameterType {
protected function getParameterValue(array $request, $key) { protected function getParameterValue(array $request, $key, $strict) {
$value = parent::getParameterValue($request, $key); $value = parent::getParameterValue($request, $key, $strict);
if ($value === null) { if ($value === null) {
return null; return null;

View file

@ -6,9 +6,11 @@ final class ConduitAPIRequest extends Phobject {
private $user; private $user;
private $isClusterRequest = false; private $isClusterRequest = false;
private $oauthToken; private $oauthToken;
private $isStrictlyTyped = true;
public function __construct(array $params) { public function __construct(array $params, $strictly_typed) {
$this->params = $params; $this->params = $params;
$this->isStrictlyTyped = $strictly_typed;
} }
public function getValue($key, $default = null) { public function getValue($key, $default = null) {
@ -68,6 +70,10 @@ final class ConduitAPIRequest extends Phobject {
return $this->isClusterRequest; return $this->isClusterRequest;
} }
public function getIsStrictlyTyped() {
return $this->isStrictlyTyped;
}
public function newContentSource() { public function newContentSource() {
return PhabricatorContentSource::newForSource( return PhabricatorContentSource::newForSource(
PhabricatorConduitContentSource::SOURCECONST); PhabricatorConduitContentSource::SOURCECONST);

View file

@ -1115,7 +1115,9 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
continue; continue;
} }
$value = $field->readValueFromConduitRequest($constraints); $value = $field->readValueFromConduitRequest(
$constraints,
$request->getIsStrictlyTyped());
$saved_query->setParameter($field->getKey(), $value); $saved_query->setParameter($field->getKey(), $value);
} }

View file

@ -323,10 +323,14 @@ abstract class PhabricatorSearchField extends Phobject {
$this->getConduitKey()); $this->getConduitKey());
} }
public function readValueFromConduitRequest(array $constraints) { public function readValueFromConduitRequest(
array $constraints,
$strict = true) {
return $this->getConduitParameterType()->getValue( return $this->getConduitParameterType()->getValue(
$constraints, $constraints,
$this->getConduitKey()); $this->getConduitKey(),
$strict);
} }
public function getValidConstraintKeys() { public function getValidConstraintKeys() {

View file

@ -1903,7 +1903,10 @@ abstract class PhabricatorEditEngine
$parameter_type->setViewer($viewer); $parameter_type->setViewer($viewer);
try { try {
$xaction['value'] = $parameter_type->getValue($xaction, 'value'); $xaction['value'] = $parameter_type->getValue(
$xaction,
'value',
$request->getIsStrictlyTyped());
} catch (Exception $ex) { } catch (Exception $ex) {
throw new PhutilProxyException( throw new PhutilProxyException(
pht( pht(