mirror of
https://github.com/nextcloud/server.git
synced 2026-04-29 18:11:41 -04:00
enh(dispatcher): enforce psalm ranges in the http dispatcher
- allows devs to provide int ranges for API arguments Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
parent
7646f68cd7
commit
3fa43a529b
4 changed files with 116 additions and 7 deletions
|
|
@ -42,6 +42,7 @@ use OCP\AppFramework\Http\Response;
|
|||
use OCP\Diagnostics\IEventLogger;
|
||||
use OCP\IConfig;
|
||||
use OCP\IRequest;
|
||||
use OutOfRangeException;
|
||||
use Psr\Container\ContainerInterface;
|
||||
use Psr\Log\LoggerInterface;
|
||||
|
||||
|
|
@ -197,7 +198,7 @@ class Dispatcher {
|
|||
private function executeController(Controller $controller, string $methodName): Response {
|
||||
$arguments = [];
|
||||
|
||||
// valid types that will be casted
|
||||
// valid types that will be cast
|
||||
$types = ['int', 'integer', 'bool', 'boolean', 'float', 'double'];
|
||||
|
||||
foreach ($this->reflector->getParameters() as $param => $default) {
|
||||
|
|
@ -219,6 +220,7 @@ class Dispatcher {
|
|||
$value = false;
|
||||
} elseif ($value !== null && \in_array($type, $types, true)) {
|
||||
settype($value, $type);
|
||||
$this->ensureParameterValueSatisfiesRange($param, $value);
|
||||
} elseif ($value === null && $type !== null && $this->appContainer->has($type)) {
|
||||
$value = $this->appContainer->get($type);
|
||||
}
|
||||
|
|
@ -250,4 +252,22 @@ class Dispatcher {
|
|||
|
||||
return $response;
|
||||
}
|
||||
|
||||
/**
|
||||
* @psalm-param mixed $value
|
||||
* @throws OutOfRangeException
|
||||
*/
|
||||
private function ensureParameterValueSatisfiesRange(string $param, $value): void {
|
||||
$rangeInfo = $this->reflector->getRange($param);
|
||||
if ($rangeInfo) {
|
||||
if ($value < $rangeInfo['min'] || $value > $rangeInfo['max']) {
|
||||
throw new OutOfRangeException(sprintf(
|
||||
'Parameter %s must be between %d and %d',
|
||||
$param,
|
||||
$rangeInfo['min'],
|
||||
$rangeInfo['max'],
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ class ControllerMethodReflector implements IControllerMethodReflector {
|
|||
public $annotations = [];
|
||||
private $types = [];
|
||||
private $parameters = [];
|
||||
private array $ranges = [];
|
||||
|
||||
/**
|
||||
* @param object $object an object or classname
|
||||
|
|
@ -54,26 +55,38 @@ class ControllerMethodReflector implements IControllerMethodReflector {
|
|||
if ($docs !== false) {
|
||||
// extract everything prefixed by @ and first letter uppercase
|
||||
preg_match_all('/^\h+\*\h+@(?P<annotation>[A-Z]\w+)((?P<parameter>.*))?$/m', $docs, $matches);
|
||||
foreach ($matches['annotation'] as $key => $annontation) {
|
||||
$annontation = strtolower($annontation);
|
||||
foreach ($matches['annotation'] as $key => $annotation) {
|
||||
$annotation = strtolower($annotation);
|
||||
$annotationValue = $matches['parameter'][$key];
|
||||
if (isset($annotationValue[0]) && $annotationValue[0] === '(' && $annotationValue[\strlen($annotationValue) - 1] === ')') {
|
||||
$cutString = substr($annotationValue, 1, -1);
|
||||
$cutString = str_replace(' ', '', $cutString);
|
||||
$splittedArray = explode(',', $cutString);
|
||||
foreach ($splittedArray as $annotationValues) {
|
||||
$splitArray = explode(',', $cutString);
|
||||
foreach ($splitArray as $annotationValues) {
|
||||
[$key, $value] = explode('=', $annotationValues);
|
||||
$this->annotations[$annontation][$key] = $value;
|
||||
$this->annotations[$annotation][$key] = $value;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->annotations[$annontation] = [$annotationValue];
|
||||
$this->annotations[$annotation] = [$annotationValue];
|
||||
}
|
||||
|
||||
// extract type parameter information
|
||||
preg_match_all('/@param\h+(?P<type>\w+)\h+\$(?P<var>\w+)/', $docs, $matches);
|
||||
$this->types = array_combine($matches['var'], $matches['type']);
|
||||
preg_match_all('/@psalm-param\h+(?P<type>\w+)<(?P<rangeMin>(-?\d+|min)),\h*(?P<rangeMax>(-?\d+|max))>\h+\$(?P<var>\w+)/', $docs, $matches);
|
||||
foreach ($matches['var'] as $index => $varName) {
|
||||
if ($matches['type'][$index] !== 'int') {
|
||||
// only int ranges are possible at the moment
|
||||
// @see https://psalm.dev/docs/annotating_code/type_syntax/scalar_types
|
||||
continue;
|
||||
}
|
||||
$this->ranges[$varName] = [
|
||||
'min' => $matches['rangeMin'][$index] === 'min' ? PHP_INT_MIN : (int)$matches['rangeMin'][$index],
|
||||
'max' => $matches['rangeMax'][$index] === 'max' ? PHP_INT_MAX : (int)$matches['rangeMax'][$index],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($reflection->getParameters() as $param) {
|
||||
|
|
@ -106,6 +119,14 @@ class ControllerMethodReflector implements IControllerMethodReflector {
|
|||
return null;
|
||||
}
|
||||
|
||||
public function getRange(string $parameter): ?array {
|
||||
if (array_key_exists($parameter, $this->ranges)) {
|
||||
return $this->ranges[$parameter];
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return array the arguments of the method with key => default value
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -522,4 +522,51 @@ class DispatcherTest extends \Test\TestCase {
|
|||
|
||||
$this->assertEquals('{"text":[3,true,4,1]}', $response[3]);
|
||||
}
|
||||
|
||||
|
||||
public function rangeDataProvider(): array {
|
||||
return [
|
||||
[PHP_INT_MIN, PHP_INT_MAX, 42, false],
|
||||
[0, 12, -5, true],
|
||||
[-12, 0, 5, true],
|
||||
[7, 14, 5, true],
|
||||
[7, 14, 10, false],
|
||||
[-14, -7, -10, false],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider rangeDataProvider
|
||||
*/
|
||||
public function testEnsureParameterValueSatisfiesRange(int $min, int $max, int $input, bool $throw): void {
|
||||
$this->reflector = $this->createMock(ControllerMethodReflector::class);
|
||||
$this->reflector->expects($this->any())
|
||||
->method('getRange')
|
||||
->willReturn([
|
||||
'min' => $min,
|
||||
'max' => $max,
|
||||
]);
|
||||
|
||||
$this->dispatcher = new Dispatcher(
|
||||
$this->http,
|
||||
$this->middlewareDispatcher,
|
||||
$this->reflector,
|
||||
$this->request,
|
||||
$this->config,
|
||||
\OC::$server->getDatabaseConnection(),
|
||||
$this->logger,
|
||||
$this->eventLogger,
|
||||
$this->container,
|
||||
);
|
||||
|
||||
if ($throw) {
|
||||
$this->expectException(\OutOfRangeException::class);
|
||||
}
|
||||
|
||||
$this->invokePrivate($this->dispatcher, 'ensureParameterValueSatisfiesRange', ['myArgument', $input]);
|
||||
if (!$throw) {
|
||||
// do not mark this test risky
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,14 @@ class MiddleController extends BaseController {
|
|||
|
||||
public function test3() {
|
||||
}
|
||||
|
||||
/**
|
||||
* @psalm-param int<-4, 42> $rangedOne
|
||||
* @psalm-param int<min, max> $rangedTwo
|
||||
* @return void
|
||||
*/
|
||||
public function test4(int $rangedOne, int $rangedTwo) {
|
||||
}
|
||||
}
|
||||
|
||||
class EndController extends MiddleController {
|
||||
|
|
@ -234,4 +242,17 @@ class ControllerMethodReflectorTest extends \Test\TestCase {
|
|||
|
||||
$this->assertFalse($reader->hasAnnotation('Annotation'));
|
||||
}
|
||||
|
||||
public function testRangeDetection() {
|
||||
$reader = new ControllerMethodReflector();
|
||||
$reader->reflect('Test\AppFramework\Utility\EndController', 'test4');
|
||||
|
||||
$rangeInfo1 = $reader->getRange('rangedOne');
|
||||
$this->assertSame(-4, $rangeInfo1['min']);
|
||||
$this->assertSame(42, $rangeInfo1['max']);
|
||||
|
||||
$rangeInfo2 = $reader->getRange('rangedTwo');
|
||||
$this->assertSame(PHP_INT_MIN, $rangeInfo2['min']);
|
||||
$this->assertSame(PHP_INT_MAX, $rangeInfo2['max']);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue