Merge pull request #59600 from nextcloud/bugfix/noid/add-default-limit-restrictions

fix(controller): Add default range to $limit parameter

Author: nickvergessen

lib/private/AppFramework/Http/Dispatcher.php

@@ -26,6 +26,9 @@
  * Class to dispatch the request to the middleware dispatcher
  */
 class Dispatcher {
+	public const DEFAULT_MIN = 1;
+	public const DEFAULT_MAX = 500;
+
 	/**
 	 * @param Http $protocol the http protocol with contains all status headers
 	 * @param MiddlewareDispatcher $middlewareDispatcher the dispatcher which
@@ -149,7 +152,7 @@ private function executeController(Controller $controller, string $methodName):
 				$value = false;
 			} elseif ($value !== null && \in_array($type, $types, true)) {
 				settype($value, $type);
-				$this->ensureParameterValueSatisfiesRange($param, $value);
+				$this->ensureParameterValueSatisfiesRange($param, $value, $default);
 			} elseif ($value === null && $type !== null && $this->appContainer->has($type)) {
 				$value = $this->appContainer->get($type);
 			}
@@ -193,7 +196,7 @@ private function executeController(Controller $controller, string $methodName):
 	 * @psalm-param mixed $value
 	 * @throws ParameterOutOfRangeException
 	 */
-	private function ensureParameterValueSatisfiesRange(string $param, $value): void {
+	private function ensureParameterValueSatisfiesRange(string $param, $value, $default): void {
 		$rangeInfo = $this->reflector->getRange($param);
 		if ($rangeInfo) {
 			if ($value < $rangeInfo['min'] || $value > $rangeInfo['max']) {
@@ -204,6 +207,15 @@ private function ensureParameterValueSatisfiesRange(string $param, $value): void
 					$rangeInfo['max'],
 				);
 			}
+		} elseif ($param === 'limit') {
+			if ($value !== $default && ($value < self::DEFAULT_MIN || $value > self::DEFAULT_MAX)) {
+				throw new ParameterOutOfRangeException(
+					$param,
+					$value,
+					self::DEFAULT_MIN,
+					self::DEFAULT_MAX,
+				);
+			}
 		}
 	}
 }

tests/lib/AppFramework/Http/DispatcherTest.php

@@ -549,18 +549,34 @@ public static function rangeDataProvider(): array {
 			[7, 14, 5, true],
 			[7, 14, 10, false],
 			[-14, -7, -10, false],
+			[null, null, -1, false],
+
+			// $limit comes with default limits of self::DEFAULT_MIN (1) <= $limit <= self::DEFAULT_MAX (500)
+			[null, null, -1, true, 'limit'],
+			[null, null, -1, false, 'limit', -1],
+			[null, null, 0, true, 'limit'],
+			[null, null, 0, true, 'limit', -1],
+			[null, null, 1, false, 'limit'],
+			[null, null, 500, false, 'limit'],
+			[null, null, 501, true, 'limit'],
 		];
 	}
 
 	#[\PHPUnit\Framework\Attributes\DataProvider('rangeDataProvider')]
-	public function testEnsureParameterValueSatisfiesRange(int $min, int $max, int $input, bool $throw): void {
+	public function testEnsureParameterValueSatisfiesRange(?int $min, ?int $max, int $input, bool $throw, string $param = 'myArgument', ?int $default = null): void {
 		$this->reflector = $this->createMock(ControllerMethodReflector::class);
-		$this->reflector->expects($this->any())
-			->method('getRange')
-			->willReturn([
-				'min' => $min,
-				'max' => $max,
-			]);
+		if ($min === null && $max === null) {
+			$this->reflector->expects($this->any())
+				->method('getRange')
+				->willReturn(null);
+		} else {
+			$this->reflector->expects($this->any())
+				->method('getRange')
+				->willReturn([
+					'min' => $min,
+					'max' => $max,
+				]);
+		}
 
 		$this->dispatcher = new Dispatcher(
 			$this->http,
@@ -578,7 +594,7 @@ public function testEnsureParameterValueSatisfiesRange(int $min, int $max, int $
 			$this->expectException(ParameterOutOfRangeException::class);
 		}
 
-		$this->invokePrivate($this->dispatcher, 'ensureParameterValueSatisfiesRange', ['myArgument', $input]);
+		self::invokePrivate($this->dispatcher, 'ensureParameterValueSatisfiesRange', [$param, $input, $default]);
 		if (!$throw) {
 			// do not mark this test risky
 			$this->assertTrue(true);