feat(dispatcher): Extend the amount of checks executed

Support for various psalm scalar types: non-empty-string, positive-int,
...

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
pull/63326/head
Carl Schwan 3 days ago
parent b3f362fff2
commit feb731a69a
No known key found for this signature in database
GPG Key ID: 02325448204E452A
  1. 12
      lib/private/AppFramework/Http/Dispatcher.php
  2. 38
      lib/private/AppFramework/Utility/ControllerMethodReflector.php
  3. 41
      lib/public/AppFramework/Http/InvalidStringParameterException.php
  4. 42
      tests/lib/AppFramework/Http/DispatcherTest.php
  5. 117
      tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php

@ -16,6 +16,7 @@ use OC\DB\ConnectionAdapter;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\InvalidStringParameterException;
use OCP\AppFramework\Http\ParameterOutOfRangeException;
use OCP\AppFramework\Http\Response;
use OCP\Diagnostics\IEventLogger;
@ -160,6 +161,8 @@ class Dispatcher {
} elseif ($value !== null && \in_array($type, $types, true)) {
settype($value, $type);
$this->ensureParameterValueSatisfiesRange($param, $value, $default);
} elseif ($value !== null && $type === 'string' && \is_string($value)) {
$this->ensureParameterValueSatisfiesStringConstraint($param, $value);
} elseif ($value === null && $type !== null && $this->appContainer->has($type)) {
$value = $this->appContainer->get($type);
}
@ -225,4 +228,13 @@ class Dispatcher {
}
}
}
/**
* @throws InvalidStringParameterException
*/
private function ensureParameterValueSatisfiesStringConstraint(string $param, string $value): void {
if (!$this->reflector->satisfiesStringConstraint($param, $value)) {
throw new InvalidStringParameterException($param, $this->reflector->getStringConstraint($param));
}
}
}

@ -20,6 +20,7 @@ class ControllerMethodReflector implements IControllerMethodReflector {
private array $types = [];
private array $parameters = [];
private array $ranges = [];
private array $stringConstraints = [];
private int $startLine = 0;
private string $file = '';
private ?\ReflectionMethod $reflectionMethod = null;
@ -38,6 +39,7 @@ class ControllerMethodReflector implements IControllerMethodReflector {
$this->types = [];
$this->parameters = [];
$this->ranges = [];
$this->stringConstraints = [];
$this->reflectionMethod = new \ReflectionMethod($object, $method);
$this->startLine = $this->reflectionMethod->getStartLine();
$this->file = $this->reflectionMethod->getFileName();
@ -79,6 +81,23 @@ class ControllerMethodReflector implements IControllerMethodReflector {
'max' => $matches['rangeMax'][$index] === 'max' ? PHP_INT_MAX : (int)$matches['rangeMax'][$index],
];
}
// extract psalm int aliases that imply a fixed range
preg_match_all('/@(?:psalm-)?param\h+(\?)?(?P<type>positive-int|non-negative-int|negative-int|non-positive-int)(\|null)?\h+\$(?P<var>\w+)/', $docs, $matches);
foreach ($matches['var'] as $index => $varName) {
$this->ranges[$varName] = match ($matches['type'][$index]) {
'positive-int' => ['min' => 1, 'max' => PHP_INT_MAX],
'non-negative-int' => ['min' => 0, 'max' => PHP_INT_MAX],
'negative-int' => ['min' => PHP_INT_MIN, 'max' => -1],
'non-positive-int' => ['min' => PHP_INT_MIN, 'max' => 0],
};
}
// extract psalm scalar string types
preg_match_all('/@(?:psalm-)?param\h+(\?)?(?P<type>non-empty-lowercase-string|non-falsy-string|non-empty-string|lowercase-string|numeric-string)(\|null)?\h+\$(?P<var>\w+)/', $docs, $matches);
foreach ($matches['var'] as $index => $varName) {
$this->stringConstraints[$varName] = $matches['type'][$index];
}
}
foreach ($this->reflectionMethod->getParameters() as $param) {
@ -120,6 +139,25 @@ class ControllerMethodReflector implements IControllerMethodReflector {
return null;
}
public function getStringConstraint(string $parameter): ?string {
return $this->stringConstraints[$parameter] ?? null;
}
/**
* Whether $value satisfies the psalm string type annotated for $parameter,
* or true if none was annotated
*/
public function satisfiesStringConstraint(string $parameter, string $value): bool {
return match ($this->getStringConstraint($parameter)) {
'non-empty-string' => $value !== '',
'non-empty-lowercase-string' => $value !== '' && $value === strtolower($value),
'lowercase-string' => $value === strtolower($value),
'non-falsy-string' => $value !== '' && $value !== '0',
'numeric-string' => is_numeric($value),
default => true,
};
}
/**
* @return array the arguments of the method with key => default value
*/

@ -0,0 +1,41 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCP\AppFramework\Http;
/**
* @since 35.0.0
*/
class InvalidStringParameterException extends \InvalidArgumentException {
/**
* @since 35.0.0
*/
public function __construct(
protected string $parameterName,
protected string $constraint,
) {
parent::__construct(
sprintf('Parameter %s must be a %s', $this->parameterName, $this->constraint)
);
}
/**
* @since 35.0.0
*/
public function getParameterName(): string {
return $this->parameterName;
}
/**
* @since 35.0.0
*/
public function getConstraint(): string {
return $this->constraint;
}
}

@ -16,6 +16,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\InvalidStringParameterException;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\ParameterOutOfRangeException;
use OCP\AppFramework\Http\Response;
@ -586,4 +587,45 @@ class DispatcherTest extends \Test\TestCase {
$this->assertTrue(true);
}
}
public static function stringConstraintDataProvider(): array {
return [
[true, null, false],
[true, 'non-empty-string', false],
[false, 'non-empty-string', true],
];
}
#[\PHPUnit\Framework\Attributes\DataProvider('stringConstraintDataProvider')]
public function testEnsureParameterValueSatisfiesStringConstraint(bool $satisfies, ?string $constraint, bool $throw): void {
$this->reflector = $this->createMock(ControllerMethodReflector::class);
$this->reflector->expects($this->any())
->method('satisfiesStringConstraint')
->willReturn($satisfies);
$this->reflector->expects($this->any())
->method('getStringConstraint')
->willReturn($constraint);
$this->dispatcher = new Dispatcher(
$this->http,
$this->middlewareDispatcher,
$this->reflector,
$this->request,
$this->config,
Server::get(IDBConnection::class),
$this->logger,
$this->eventLogger,
$this->container,
);
if ($throw) {
$this->expectException(InvalidStringParameterException::class);
}
self::invokePrivate($this->dispatcher, 'ensureParameterValueSatisfiesStringConstraint', ['myArgument', '']);
if (!$throw) {
// do not mark this test risky
$this->assertTrue(true);
}
}
}

@ -63,6 +63,41 @@ class MiddleController extends BaseController {
*/
public function test5(int $rangedOne, int $rangedTwo, ?int $rangedThree, ?int $rangedFour) {
}
/**
* @psalm-param positive-int $positive
* @psalm-param non-negative-int $nonNegative
* @psalm-param negative-int $negative
* @psalm-param non-positive-int $nonPositive
* @psalm-param positive-int|null $positiveOrNull
* @psalm-param ?non-negative-int $nonNegativeOrNull
* @return void
*/
public function test6(int $positive, int $nonNegative, int $negative, int $nonPositive, ?int $positiveOrNull, ?int $nonNegativeOrNull) {
}
/**
* @psalm-param non-empty-string $nonEmpty
* @psalm-param non-empty-lowercase-string $nonEmptyLowercase
* @psalm-param lowercase-string $lowercase
* @psalm-param non-falsy-string $nonFalsy
* @psalm-param numeric-string $numeric
* @psalm-param non-empty-string|null $nonEmptyOrNull
* @psalm-param ?non-empty-string $nonEmptyOrNullPrefix
* @psalm-param string $plain
* @return void
*/
public function test7(
string $nonEmpty,
string $nonEmptyLowercase,
string $lowercase,
string $nonFalsy,
string $numeric,
?string $nonEmptyOrNull,
?string $nonEmptyOrNullPrefix,
string $plain,
) {
}
}
class EndController extends MiddleController {
@ -275,4 +310,86 @@ class ControllerMethodReflectorTest extends \Test\TestCase {
$this->assertSame(-70, $rangeInfo3['min']);
$this->assertSame(-30, $rangeInfo3['max']);
}
public function testRangeDetectionIntAliases(): void {
$reader = new ControllerMethodReflector(Server::get(LoggerInterface::class));
$reader->reflect('Test\AppFramework\Utility\EndController', 'test6');
$positive = $reader->getRange('positive');
$this->assertSame(1, $positive['min']);
$this->assertSame(PHP_INT_MAX, $positive['max']);
$nonNegative = $reader->getRange('nonNegative');
$this->assertSame(0, $nonNegative['min']);
$this->assertSame(PHP_INT_MAX, $nonNegative['max']);
$negative = $reader->getRange('negative');
$this->assertSame(PHP_INT_MIN, $negative['min']);
$this->assertSame(-1, $negative['max']);
$nonPositive = $reader->getRange('nonPositive');
$this->assertSame(PHP_INT_MIN, $nonPositive['min']);
$this->assertSame(0, $nonPositive['max']);
$positiveOrNull = $reader->getRange('positiveOrNull');
$this->assertSame(1, $positiveOrNull['min']);
$this->assertSame(PHP_INT_MAX, $positiveOrNull['max']);
$nonNegativeOrNull = $reader->getRange('nonNegativeOrNull');
$this->assertSame(0, $nonNegativeOrNull['min']);
$this->assertSame(PHP_INT_MAX, $nonNegativeOrNull['max']);
}
public function testStringConstraintDetection(): void {
$reader = new ControllerMethodReflector(Server::get(LoggerInterface::class));
$reader->reflect('Test\AppFramework\Utility\EndController', 'test7');
$this->assertSame('non-empty-string', $reader->getStringConstraint('nonEmpty'));
$this->assertSame('non-empty-lowercase-string', $reader->getStringConstraint('nonEmptyLowercase'));
$this->assertSame('lowercase-string', $reader->getStringConstraint('lowercase'));
$this->assertSame('non-falsy-string', $reader->getStringConstraint('nonFalsy'));
$this->assertSame('numeric-string', $reader->getStringConstraint('numeric'));
$this->assertSame('non-empty-string', $reader->getStringConstraint('nonEmptyOrNull'));
$this->assertSame('non-empty-string', $reader->getStringConstraint('nonEmptyOrNullPrefix'));
$this->assertNull($reader->getStringConstraint('plain'));
}
public static function stringConstraintDataProvider(): array {
return [
['non-empty-string', '', false],
['non-empty-string', 'a', true],
['non-empty-lowercase-string', '', false],
['non-empty-lowercase-string', 'ABC', false],
['non-empty-lowercase-string', 'abc', true],
['lowercase-string', '', true],
['lowercase-string', 'ABC', false],
['lowercase-string', 'abc', true],
['non-falsy-string', '', false],
['non-falsy-string', '0', false],
['non-falsy-string', '0.0', true],
['non-falsy-string', 'a', true],
['numeric-string', 'abc', false],
['numeric-string', '42', true],
['numeric-string', '4.2', true],
[null, '', true],
[null, 'anything', true],
];
}
#[\PHPUnit\Framework\Attributes\DataProvider('stringConstraintDataProvider')]
public function testSatisfiesStringConstraint(?string $constraint, string $value, bool $expected): void {
$reader = new ControllerMethodReflector(Server::get(LoggerInterface::class));
$reader->reflect('Test\AppFramework\Utility\EndController', 'test7');
$parameter = match ($constraint) {
'non-empty-string' => 'nonEmpty',
'non-empty-lowercase-string' => 'nonEmptyLowercase',
'lowercase-string' => 'lowercase',
'non-falsy-string' => 'nonFalsy',
'numeric-string' => 'numeric',
default => 'plain',
};
$this->assertSame($expected, $reader->satisfiesStringConstraint($parameter, $value));
}
}

Loading…
Cancel
Save