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>

diff --git a/lib/private/AppFramework/Http/Dispatcher.php b/lib/private/AppFramework/Http/Dispatcher.php
index 7ff7f7a9cc044ec4a560bd4135868a03d1a1f00f..5649060ba7621c6d4d96ed5167027a351ed05d7b 100644 (file)
--- a/lib/private/AppFramework/Http/Dispatcher.php
+++ b/lib/private/AppFramework/Http/Dispatcher.php
@@ -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'],
+                               ));
+                       }
+               }
+       }
 }

diff --git a/lib/private/AppFramework/Utility/ControllerMethodReflector.php b/lib/private/AppFramework/Utility/ControllerMethodReflector.php
index b76b3c33c42a9d031a18d1b3c95bcb0c7c3689f1..5a1ed0fd6ee540628743a60ecd382f8f1bc9b284 100644 (file)
--- a/lib/private/AppFramework/Utility/ControllerMethodReflector.php
+++ b/lib/private/AppFramework/Utility/ControllerMethodReflector.php
@@ -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
         */

diff --git a/tests/lib/AppFramework/Http/DispatcherTest.php b/tests/lib/AppFramework/Http/DispatcherTest.php
index bb7541c4d22a9ddada389a7f1c04c461b2bc2e32..d9d9eb82a6846b0052070e599d3456d83c4bd066 100644 (file)
--- a/tests/lib/AppFramework/Http/DispatcherTest.php
+++ b/tests/lib/AppFramework/Http/DispatcherTest.php
@@ -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);
+               }
+       }
 }

diff --git a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php
index 5452fb853b90959f3d76d0d78b49c75327c93fe3..2ba2f34425b8d7d132f0295c1da133c3e1d68817 100644 (file)
--- a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php
+++ b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php
@@ -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']);
+       }
 }