Catch missing non-optional controller parameter

core/Middleware/ApiErrorMiddleware.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * @copyright 2022 Christoph Wurst <[email protected]>
+ *
+ * @author 2022 Christoph Wurst <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace OC\Core\Middleware;
+
+use Exception;
+use OC\AppFramework\Exceptions\ParameterMissingException;
+use OC\Http\ApiResponse;
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Middleware;
+use OCP\IRequest;
+
+class ApiErrorMiddleware extends Middleware {
+	private IRequest $request;
+
+	public function __construct(IRequest $request) {
+		$this->request = $request;
+	}
+
+	public function afterException($controller, $methodName, Exception $exception) {
+		$sendsJson = $this->request->getHeader('Content-Type') === 'application/json';
+		$acceptsJson = $this->request->getHeader('Accept') === 'application/json';
+		if (($sendsJson || $acceptsJson) && $exception instanceof ParameterMissingException) {
+			return ApiResponse::fail(
+				['error' => 'Required parameter ' . $exception->getParameterName() . ' missing'],
+				Http::STATUS_UNPROCESSABLE_ENTITY,
+			);
+		}
+		return parent::afterException($controller, $methodName, $exception);
+	}
+}

lib/composer/composer/autoload_classmap.php

@@ -678,6 +678,7 @@
     'OC\\AppFramework\\Bootstrap\\ServiceFactoryRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/ServiceFactoryRegistration.php',
     'OC\\AppFramework\\Bootstrap\\ServiceRegistration' => $baseDir . '/lib/private/AppFramework/Bootstrap/ServiceRegistration.php',
     'OC\\AppFramework\\DependencyInjection\\DIContainer' => $baseDir . '/lib/private/AppFramework/DependencyInjection/DIContainer.php',
+    'OC\\AppFramework\\Exceptions\\ParameterMissingException' => $baseDir . '/lib/private/AppFramework/Exceptions/ParameterMissingException.php',
     'OC\\AppFramework\\Http' => $baseDir . '/lib/private/AppFramework/Http.php',
     'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php',
     'OC\\AppFramework\\Http\\Output' => $baseDir . '/lib/private/AppFramework/Http/Output.php',
@@ -1021,6 +1022,7 @@
     'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => $baseDir . '/core/Exception/LoginFlowV2NotFoundException.php',
     'OC\\Core\\Exception\\ResetPasswordException' => $baseDir . '/core/Exception/ResetPasswordException.php',
     'OC\\Core\\Listener\\BeforeTemplateRenderedListener' => $baseDir . '/core/Listener/BeforeTemplateRenderedListener.php',
+    'OC\\Core\\Middleware\\ApiErrorMiddleware' => $baseDir . '/core/Middleware/ApiErrorMiddleware.php',
     'OC\\Core\\Middleware\\TwoFactorMiddleware' => $baseDir . '/core/Middleware/TwoFactorMiddleware.php',
     'OC\\Core\\Migrations\\Version13000Date20170705121758' => $baseDir . '/core/Migrations/Version13000Date20170705121758.php',
     'OC\\Core\\Migrations\\Version13000Date20170718121200' => $baseDir . '/core/Migrations/Version13000Date20170718121200.php',
@@ -1290,6 +1292,7 @@
     'OC\\Hooks\\Emitter' => $baseDir . '/lib/private/Hooks/Emitter.php',
     'OC\\Hooks\\EmitterTrait' => $baseDir . '/lib/private/Hooks/EmitterTrait.php',
     'OC\\Hooks\\PublicEmitter' => $baseDir . '/lib/private/Hooks/PublicEmitter.php',
+    'OC\\Http\\ApiResponse' => $baseDir . '/lib/private/Http/ApiResponse.php',
     'OC\\Http\\Client\\Client' => $baseDir . '/lib/private/Http/Client/Client.php',
     'OC\\Http\\Client\\ClientService' => $baseDir . '/lib/private/Http/Client/ClientService.php',
     'OC\\Http\\Client\\DnsPinMiddleware' => $baseDir . '/lib/private/Http/Client/DnsPinMiddleware.php',

lib/composer/composer/autoload_static.php

@@ -711,6 +711,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\AppFramework\\Bootstrap\\ServiceFactoryRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/ServiceFactoryRegistration.php',
         'OC\\AppFramework\\Bootstrap\\ServiceRegistration' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Bootstrap/ServiceRegistration.php',
         'OC\\AppFramework\\DependencyInjection\\DIContainer' => __DIR__ . '/../../..' . '/lib/private/AppFramework/DependencyInjection/DIContainer.php',
+        'OC\\AppFramework\\Exceptions\\ParameterMissingException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Exceptions/ParameterMissingException.php',
         'OC\\AppFramework\\Http' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http.php',
         'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php',
         'OC\\AppFramework\\Http\\Output' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Output.php',
@@ -1054,6 +1055,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2NotFoundException.php',
         'OC\\Core\\Exception\\ResetPasswordException' => __DIR__ . '/../../..' . '/core/Exception/ResetPasswordException.php',
         'OC\\Core\\Listener\\BeforeTemplateRenderedListener' => __DIR__ . '/../../..' . '/core/Listener/BeforeTemplateRenderedListener.php',
+        'OC\\Core\\Middleware\\ApiErrorMiddleware' => __DIR__ . '/../../..' . '/core/Middleware/ApiErrorMiddleware.php',
         'OC\\Core\\Middleware\\TwoFactorMiddleware' => __DIR__ . '/../../..' . '/core/Middleware/TwoFactorMiddleware.php',
         'OC\\Core\\Migrations\\Version13000Date20170705121758' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170705121758.php',
         'OC\\Core\\Migrations\\Version13000Date20170718121200' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170718121200.php',
@@ -1323,6 +1325,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Hooks\\Emitter' => __DIR__ . '/../../..' . '/lib/private/Hooks/Emitter.php',
         'OC\\Hooks\\EmitterTrait' => __DIR__ . '/../../..' . '/lib/private/Hooks/EmitterTrait.php',
         'OC\\Hooks\\PublicEmitter' => __DIR__ . '/../../..' . '/lib/private/Hooks/PublicEmitter.php',
+        'OC\\Http\\ApiResponse' => __DIR__ . '/../../..' . '/lib/private/Http/ApiResponse.php',
         'OC\\Http\\Client\\Client' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Client.php',
         'OC\\Http\\Client\\ClientService' => __DIR__ . '/../../..' . '/lib/private/Http/Client/ClientService.php',
         'OC\\Http\\Client\\DnsPinMiddleware' => __DIR__ . '/../../..' . '/lib/private/Http/Client/DnsPinMiddleware.php',

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -300,6 +300,9 @@ public function __construct($appName, $urlParams = [], ServerContainer $server =
 					$c->get(OC\Security\RateLimiting\Limiter::class)
 				)
 			);
+			$dispatcher->registerMiddleware(
+				$c->get(\OC\Core\Middleware\ApiErrorMiddleware::class)
+			);
 			$dispatcher->registerMiddleware(
 				new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware(
 					$c->get(IRequest::class),

lib/private/AppFramework/Exceptions/ParameterMissingException.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * @copyright 2022 Christoph Wurst <[email protected]>
+ *
+ * @author 2022 Christoph Wurst <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace OC\AppFramework\Exceptions;
+
+use Exception;
+
+class ParameterMissingException extends Exception {
+	private string $parameterName;
+
+	public function __construct(string $controllerName, string $methodName, string $parameterName) {
+		parent::__construct("Parameter $parameterName missing for $controllerName::$methodName");
+		$this->parameterName = $parameterName;
+	}
+
+	public function getParameterName(): string {
+		return $this->parameterName;
+	}
+}

lib/private/AppFramework/Http/Dispatcher.php

@@ -32,6 +32,7 @@
  */
 namespace OC\AppFramework\Http;
 
+use OC\AppFramework\Exceptions\ParameterMissingException;
 use OC\AppFramework\Http;
 use OC\AppFramework\Middleware\MiddlewareDispatcher;
 use OC\AppFramework\Utility\ControllerMethodReflector;
@@ -43,6 +44,9 @@
 use OCP\IConfig;
 use OCP\IRequest;
 use Psr\Log\LoggerInterface;
+use ReflectionMethod;
+use ReflectionNamedType;
+use function get_class;
 
 /**
  * Class to dispatch the request to the middleware dispatcher
@@ -192,20 +196,31 @@ public function dispatch(Controller $controller, string $methodName): array {
 	 */
 	private function executeController(Controller $controller, string $methodName): Response {
 		$arguments = [];
+		$reflection = new ReflectionMethod($controller, $methodName);
 
 		// valid types that will be casted
 		$types = ['int', 'integer', 'bool', 'boolean', 'float'];
 
-		foreach ($this->reflector->getParameters() as $param => $default) {
+		foreach ($reflection->getParameters() as $param) {
+			$paramName = $param->name;
 
 			// try to get the parameter from the request object and cast
-			// it to the type annotated in the @param annotation
-			$value = $this->request->getParam($param, $default);
-			$type = $this->reflector->getType($param);
+			// it to the type annotated in the @paramName annotation
+			$defaultValue = null;
+			if ($param->isDefaultValueAvailable()) {
+				$defaultValue = $param->getDefaultValue();
+			}
+			$value = $this->request->getParam($paramName, $defaultValue);
+			$type = $param->getType();
+			if ($type instanceof ReflectionNamedType) {
+				$typeName = $type->getName();
+			} else {
+				$typeName = $this->reflector->getType($paramName);
+			}
 
 			// if this is submitted using GET or a POST form, 'false' should be
 			// converted to false
-			if (($type === 'bool' || $type === 'boolean') &&
+			if (($typeName === 'bool' || $typeName === 'boolean') &&
 				$value === 'false' &&
 				(
 					$this->request->method === 'GET' ||
@@ -214,8 +229,12 @@ private function executeController(Controller $controller, string $methodName):
 				)
 			) {
 				$value = false;
-			} elseif ($value !== null && \in_array($type, $types, true)) {
-				settype($value, $type);
+			} elseif ($value !== null && \in_array($typeName, $types, true)) {
+				settype($value, $typeName);
+			}
+
+			if ($value === null && !$param->allowsNull()) {
+				throw new ParameterMissingException(get_class($controller), $methodName, $paramName);
 			}
 
 			$arguments[] = $value;

lib/private/Http/ApiResponse.php

@@ -0,0 +1,75 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * @copyright 2022 Christoph Wurst <[email protected]>
+ *
+ * @author 2022 Christoph Wurst <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace OC\Http;
+
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\JSONResponse;
+
+/**
+ * JSend-link API response
+ *
+ * @see https://github.com/omniti-labs/jsend
+ */
+class ApiResponse extends JSONResponse {
+	public static function success($data = null, int $statusCode = Http::STATUS_OK): self {
+		return new self(
+			[
+				'status' => 'success',
+				'data' => $data,
+			],
+			$statusCode
+		);
+	}
+
+	public static function fail($data = null, int $statusCode = Http::STATUS_BAD_REQUEST): self {
+		return new self(
+			[
+				'status' => 'fail',
+				'data' => $data,
+			],
+			$statusCode
+		);
+	}
+
+	public static function error(string $message,
+		int $statusCode = Http::STATUS_INTERNAL_SERVER_ERROR,
+		int $code = null,
+		$data = null): self {
+		return new self(
+			[
+				'status' => 'error',
+				'message' => $message,
+				'code' => $code,
+				'data' => $data,
+			],
+			$statusCode
+		);
+	}
+
+	private function __construct($data = [], $statusCode = Http::STATUS_OK) {
+		parent::__construct($data, $statusCode);
+	}
+}

tests/lib/AppFramework/Http/DispatcherTest.php

@@ -62,6 +62,9 @@ public function exec($int, $bool, $test = 4, $test2 = 1) {
 		return [$int, $bool, $test, $test2];
 	}
 
+	public function execTyped(int $int, bool $bool, string $str, ?int $opt = 42): array {
+		return [$int, $bool, $str, $opt];
+	}
 
 	/**
 	 * @param int $int
@@ -370,7 +373,35 @@ public function testControllerParametersInjectedDefaultOverwritten() {
 		$this->assertEquals('[3,true,4,7]', $response[3]);
 	}
 
+	public function testTypedControllerParameters(): void {
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false',
+					'str' => '7',
+				],
+				'method' => 'POST',
+			],
+			$this->createMock(IRequestId::class),
+			$this->createMock(IConfig::class)
+		);
+		$this->dispatcher = new Dispatcher(
+			$this->http, $this->middlewareDispatcher, $this->reflector,
+			$this->request,
+			$this->config,
+			\OC::$server->getDatabaseConnection(),
+			$this->logger,
+			$this->eventLogger
+		);
+		$controller = new TestController('app', $this->request);
+
+		// reflector is supposed to be called once
+		$this->dispatcherPassthrough();
+		$response = $this->dispatcher->dispatch($controller, 'execTyped');
 
+		$this->assertEquals('[3,true,"7",42]', $response[3]);
+	}
 
 	public function testResponseTransformedByUrlFormat() {
 		$this->request = new Request(