From 8ca36d5d662ee5ed0ad62214bb37c4ee68991bbc Mon Sep 17 00:00:00 2001 From: michalsn Date: Thu, 5 Mar 2026 19:22:35 +0100 Subject: [PATCH 1/3] feat: add FormRequest for encapsulating validation and authorization --- system/CodeIgniter.php | 118 ++- .../Generators/FormRequestGenerator.php | 86 +++ .../Generators/Views/formrequest.tpl.php | 38 + .../HTTP/Exceptions/FormRequestException.php | 34 + system/HTTP/FormRequest.php | 281 +++++++ system/Language/en/CLI.php | 1 + system/Router/AutoRouterImproved.php | 10 +- system/Router/RouteCollection.php | 24 +- system/Router/RouteCollectionInterface.php | 9 +- system/Router/Router.php | 6 +- system/Router/RouterInterface.php | 5 +- .../Controllers/FormRequestController.php | 74 ++ .../HTTP/Requests/UnauthorizedFormRequest.php | 32 + .../HTTP/Requests/ValidPostFormRequest.php | 33 + tests/system/HTTP/FormRequestTest.php | 710 ++++++++++++++++++ .../system/Router/AutoRouterImprovedTest.php | 25 + .../Router/Controllers/Mycontroller.php | 9 + .../Controllers/Requests/MyFormRequest.php | 24 + user_guide_src/source/changelogs/v4.8.0.rst | 2 + .../source/incoming/form_requests.rst | 242 ++++++ .../source/incoming/form_requests/001.php | 16 + .../source/incoming/form_requests/002.php | 18 + .../source/incoming/form_requests/003.php | 18 + .../source/incoming/form_requests/004.php | 29 + .../source/incoming/form_requests/005.php | 22 + .../source/incoming/form_requests/006.php | 25 + .../source/incoming/form_requests/007.php | 22 + .../source/incoming/form_requests/008.php | 29 + .../source/incoming/form_requests/009.php | 4 + .../source/incoming/form_requests/010.php | 16 + .../source/incoming/form_requests/011.php | 24 + .../source/incoming/form_requests/012.php | 37 + .../source/incoming/form_requests/013.php | 44 ++ .../source/incoming/form_requests/014.php | 8 + user_guide_src/source/incoming/index.rst | 1 + .../source/libraries/validation.rst | 12 + utils/phpstan-baseline/argument.type.neon | 22 +- utils/phpstan-baseline/loader.neon | 2 +- .../missingType.callable.neon | 97 ++- 39 files changed, 2159 insertions(+), 50 deletions(-) create mode 100644 system/Commands/Generators/FormRequestGenerator.php create mode 100644 system/Commands/Generators/Views/formrequest.tpl.php create mode 100644 system/HTTP/Exceptions/FormRequestException.php create mode 100644 system/HTTP/FormRequest.php create mode 100644 tests/_support/Controllers/FormRequestController.php create mode 100644 tests/_support/HTTP/Requests/UnauthorizedFormRequest.php create mode 100644 tests/_support/HTTP/Requests/ValidPostFormRequest.php create mode 100644 tests/system/HTTP/FormRequestTest.php create mode 100644 tests/system/Router/Controllers/Requests/MyFormRequest.php create mode 100644 user_guide_src/source/incoming/form_requests.rst create mode 100644 user_guide_src/source/incoming/form_requests/001.php create mode 100644 user_guide_src/source/incoming/form_requests/002.php create mode 100644 user_guide_src/source/incoming/form_requests/003.php create mode 100644 user_guide_src/source/incoming/form_requests/004.php create mode 100644 user_guide_src/source/incoming/form_requests/005.php create mode 100644 user_guide_src/source/incoming/form_requests/006.php create mode 100644 user_guide_src/source/incoming/form_requests/007.php create mode 100644 user_guide_src/source/incoming/form_requests/008.php create mode 100644 user_guide_src/source/incoming/form_requests/009.php create mode 100644 user_guide_src/source/incoming/form_requests/010.php create mode 100644 user_guide_src/source/incoming/form_requests/011.php create mode 100644 user_guide_src/source/incoming/form_requests/012.php create mode 100644 user_guide_src/source/incoming/form_requests/013.php create mode 100644 user_guide_src/source/incoming/form_requests/014.php diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index dc2ff1dcf05b..a43e79f8c265 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -19,7 +19,9 @@ use CodeIgniter\Exceptions\PageNotFoundException; use CodeIgniter\Filters\Filters; use CodeIgniter\HTTP\CLIRequest; +use CodeIgniter\HTTP\Exceptions\FormRequestException; use CodeIgniter\HTTP\Exceptions\RedirectException; +use CodeIgniter\HTTP\FormRequest; use CodeIgniter\HTTP\IncomingRequest; use CodeIgniter\HTTP\Method; use CodeIgniter\HTTP\NonBufferedResponseInterface; @@ -36,6 +38,9 @@ use Config\Feature; use Config\Services; use Locale; +use ReflectionFunction; +use ReflectionFunctionAbstract; +use ReflectionMethod; use Throwable; /** @@ -104,7 +109,7 @@ class CodeIgniter /** * Controller to use. * - * @var (Closure(mixed...): ResponseInterface|string)|string|null + * @var Closure|string|null */ protected $controller; @@ -584,8 +589,9 @@ protected function startController() // Is it routed to a Closure? if (is_object($this->controller) && ($this->controller::class === 'Closure')) { $controller = $this->controller; + $resolved = $this->resolveCallableParams(new ReflectionFunction($controller), $this->router->params()); - return $controller(...$this->router->params()); + return $controller(...$resolved); } // No controller specified - we don't know what to do now. @@ -662,15 +668,117 @@ protected function runController($class) // The controller method param types may not be string. // So cannot set `declare(strict_types=1)` in this file. - $output = method_exists($class, '_remap') - ? $class->_remap($this->method, ...$params) - : $class->{$this->method}(...$params); + if (method_exists($class, '_remap')) { + // FormRequest injection is not supported for _remap() because its + // signature is fixed to ($method, ...$params). Instantiate the + // FormRequest manually inside _remap() if needed. + $output = $class->_remap($this->method, ...$params); + } else { + $resolved = $this->resolveMethodParams($class, $this->method, $params); + $output = $class->{$this->method}(...$resolved); + } $this->benchmark->stop('controller'); return $output; } + /** + * Resolves the final parameter list for a controller method call. + * + * @param list $routeParams URI segments from the router. + * + * @return list + */ + private function resolveMethodParams(object $class, string $method, array $routeParams): array + { + return $this->resolveCallableParams(new ReflectionMethod($class, $method), $routeParams); + } + + /** + * Shared FormRequest resolver for both controller methods and closures. + * + * Builds a sequential positional argument list for the call site. + * The supported signature shape is: required scalar route params first, + * then the FormRequest, then optional scalar params. + * + * - FormRequest subclasses are instantiated, authorized, and validated + * before being injected. + * - Variadic non-FormRequest parameters consume all remaining URI segments. + * - Scalar non-FormRequest parameters consume one URI segment each. + * - When route segments run out, a required non-FormRequest parameter stops + * iteration so PHP throws an ArgumentCountError on the call site. + * - Optional non-FormRequest parameters with no remaining segment are omitted + * from the list; PHP then applies their declared default values. + * + * @param list $routeParams URI segments from the router. + * + * @return list + */ + private function resolveCallableParams(ReflectionFunctionAbstract $reflection, array $routeParams): array + { + $resolved = []; + $routeIndex = 0; + + foreach ($reflection->getParameters() as $param) { + // Inject FormRequest subclasses regardless of position. + $formRequestClass = FormRequest::getFormRequestClass($param); + + if ($formRequestClass !== null) { + $resolved[] = $this->resolveFormRequest($formRequestClass); + + continue; + } + + // Variadic parameter - consume all remaining route segments. + if ($param->isVariadic()) { + while (array_key_exists($routeIndex, $routeParams)) { + $resolved[] = $routeParams[$routeIndex++]; + } + + break; + } + + // Consume the next route segment if one is available. + if (array_key_exists($routeIndex, $routeParams)) { + $resolved[] = $routeParams[$routeIndex++]; + + continue; + } + + // No more route segments. Required params stop iteration so that + // PHP throws an ArgumentCountError on the call site. Optional + // params are omitted - PHP then applies their declared default value. + if (! $param->isOptional()) { + break; + } + } + + return $resolved; + } + + /** + * Instantiates, authorizes, and validates a FormRequest class. + * + * If authorization or validation fails, the FormRequest returns a + * ResponseInterface. The framework wraps it in a FormRequestException + * (which implements ResponsableInterface) so the response is sent + * without reaching the controller method. + * + * @param class-string $className + */ + private function resolveFormRequest(string $className): FormRequest + { + $formRequest = new $className($this->request); + $response = $formRequest->resolveRequest(); + + if ($response !== null) { + throw new FormRequestException($response); + } + + return $formRequest; + } + /** * Displays a 404 Page Not Found error. If set, will try to * call the 404Override controller/method that was set in routing config. diff --git a/system/Commands/Generators/FormRequestGenerator.php b/system/Commands/Generators/FormRequestGenerator.php new file mode 100644 index 000000000000..e4e76c38749d --- /dev/null +++ b/system/Commands/Generators/FormRequestGenerator.php @@ -0,0 +1,86 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Commands\Generators; + +use CodeIgniter\CLI\BaseCommand; +use CodeIgniter\CLI\GeneratorTrait; + +/** + * Generates a skeleton FormRequest file. + */ +class FormRequestGenerator extends BaseCommand +{ + use GeneratorTrait; + + /** + * The Command's Group + * + * @var string + */ + protected $group = 'Generators'; + + /** + * The Command's Name + * + * @var string + */ + protected $name = 'make:request'; + + /** + * The Command's Description + * + * @var string + */ + protected $description = 'Generates a new FormRequest file.'; + + /** + * The Command's Usage + * + * @var string + */ + protected $usage = 'make:request [options]'; + + /** + * The Command's Arguments + * + * @var array + */ + protected $arguments = [ + 'name' => 'The FormRequest class name.', + ]; + + /** + * The Command's Options + * + * @var array + */ + protected $options = [ + '--namespace' => 'Set root namespace. Default: "APP_NAMESPACE".', + '--suffix' => 'Append the component title to the class name (e.g. User => UserRequest).', + '--force' => 'Force overwrite existing file.', + ]; + + /** + * Actually execute a command. + */ + public function run(array $params) + { + $this->component = 'Request'; + $this->directory = 'Requests'; + $this->template = 'formrequest.tpl.php'; + + $this->classNameLang = 'CLI.generator.className.request'; + $this->generateClass($params); + } +} diff --git a/system/Commands/Generators/Views/formrequest.tpl.php b/system/Commands/Generators/Views/formrequest.tpl.php new file mode 100644 index 000000000000..cdcbeae966d5 --- /dev/null +++ b/system/Commands/Generators/Views/formrequest.tpl.php @@ -0,0 +1,38 @@ +<@php + +namespace {namespace}; + +use CodeIgniter\HTTP\FormRequest; + +class {class} extends FormRequest +{ + /** + * Returns the validation rules that apply to this request. + * + * @return array|string> + */ + public function rules(): array + { + return [ + // 'field' => 'required', + ]; + } + + // /** + // * Custom error messages keyed by field.rule. + // * + // * @return array|string> + // */ + // public function messages(): array + // { + // return []; + // } + + // /** + // * Determines if the current user is authorized to make this request. + // */ + // public function authorize(): bool + // { + // return true; + // } +} diff --git a/system/HTTP/Exceptions/FormRequestException.php b/system/HTTP/Exceptions/FormRequestException.php new file mode 100644 index 000000000000..5daae9a48d1d --- /dev/null +++ b/system/HTTP/Exceptions/FormRequestException.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP\Exceptions; + +use CodeIgniter\Exceptions\RuntimeException; +use CodeIgniter\HTTP\ResponsableInterface; +use CodeIgniter\HTTP\ResponseInterface; + +/** + * @internal + */ +final class FormRequestException extends RuntimeException implements ResponsableInterface +{ + public function __construct(private readonly ResponseInterface $response) + { + parent::__construct('FormRequest authorization or validation failed.'); + } + + public function getResponse(): ResponseInterface + { + return $this->response; + } +} diff --git a/system/HTTP/FormRequest.php b/system/HTTP/FormRequest.php new file mode 100644 index 000000000000..cdaa98c30112 --- /dev/null +++ b/system/HTTP/FormRequest.php @@ -0,0 +1,281 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\Exceptions\RuntimeException; +use ReflectionNamedType; +use ReflectionParameter; + +/** + * @see \CodeIgniter\HTTP\FormRequestTest + */ +abstract class FormRequest +{ + /** + * The underlying HTTP request instance. + */ + protected IncomingRequest $request; + + /** + * Data that passed validation (only the fields covered by rules()). + * + * @var array + */ + private array $validatedData = []; + + /** + * When called by the framework, the current IncomingRequest is injected + * explicitly. When instantiated manually (e.g. in tests or commands), + * the constructor falls back to service('request'). + * + * @throws RuntimeException if used outside of an HTTP request context. + */ + final public function __construct(?IncomingRequest $request = null) + { + $resolved = $request ?? service('request'); + + if (! $resolved instanceof IncomingRequest) { + throw new RuntimeException( + static::class . ' requires an IncomingRequest instance, got ' . $resolved::class . '.', + ); + } + + $this->request = $resolved; + } + + /** + * Validation rules for this request. + * + * Return an array of field => rules pairs, identical to what you would + * pass to $this->validate() in a controller: + * + * return [ + * 'title' => 'required|min_length[5]', + * 'body' => ['required', 'max_length[10000]'], + * ]; + * + * @return array|string> + */ + abstract public function rules(): array; + + /** + * Custom error messages keyed by field.rule. + * + * return [ + * 'title' => ['required' => 'Post title cannot be empty.'], + * ]; + * + * @return array|string> + */ + public function messages(): array + { + return []; + } + + /** + * Determine if the current user is authorized to make this request. + * + * Override in subclasses to add authorization logic: + * + * public function authorize(): bool + * { + * return auth()->user()->can('create-posts'); + * } + */ + public function authorize(): bool + { + return true; + } + + /** + * Returns the class name when the given reflection parameter is typed as a + * FormRequest subclass, or null otherwise. Used by the dispatcher and + * auto-router to distinguish injectable parameters from URI-segment parameters. + * + * @internal + * + * @return class-string|null + */ + public static function getFormRequestClass(ReflectionParameter $param): ?string + { + $type = $param->getType(); + + if ( + $type instanceof ReflectionNamedType + && ! $type->isBuiltin() + && is_subclass_of($type->getName(), self::class) + ) { + return $type->getName(); + } + + return null; + } + + /** + * Modify the request data before validation rules are applied. + * Override to normalize or cast input values: + * + * protected function prepareForValidation(array $data): array + * { + * $data['slug'] = url_title($data['title'] ?? '', '-', true); + * return $data; + * } + * + * The $data array is the same payload that will be passed to the + * validator. Return the (possibly modified) array. + * + * @param array $data + * + * @return array + */ + protected function prepareForValidation(array $data): array + { + return $data; + } + + /** + * Called when validation fails. Override to customize the failure response. + * + * The default implementation redirects back with input and flashes validation + * errors via the standard ``_ci_validation_errors`` channel (the same channel + * used by controller-level validation and readable by ``validation_errors()`` + * helpers). For JSON/AJAX requests it returns a 422 JSON response instead. + * + * @param array $errors + */ + protected function failedValidation(array $errors): ResponseInterface + { + if ($this->request->is('json') || $this->request->isAJAX()) { + return service('response')->setStatusCode(422)->setJSON(['errors' => $errors]); + } + + return redirect()->back()->withInput(); + } + + /** + * Called when the authorize() check returns false. Override to customize. + */ + protected function failedAuthorization(): ResponseInterface + { + return service('response')->setStatusCode(403); + } + + /** + * Returns only the fields that passed validation (those covered by rules()). + * + * Prefer this over $this->request->getPost() in controllers to avoid + * processing fields that were not declared in the rules. + * + * @return array + */ + public function validated(): array + { + return $this->validatedData; + } + + /** + * Returns a single validated field value by name, or null if the field + * is not present in the validated data (either not declared in rules() or + * validation has not yet run). + * + * Allows accessing individual fields as object properties: + * + * $title = $request->title; + * + * @return mixed + */ + public function __get(string $name) + { + return $this->validatedData[$name] ?? null; + } + + /** + * Returns true when the named field exists in the validated data and its + * value is not null. Mirrors standard PHP isset() semantics on properties: + * + * if (isset($request->title)) { ... } + */ + public function __isset(string $name): bool + { + return isset($this->validatedData[$name]); + } + + /** + * Returns the data to be validated. + * + * Override this method to provide custom data or to merge data from + * multiple sources. By default, data is sourced from the appropriate + * part of the request based on HTTP method and Content-Type: + * + * - JSON (any method) - decoded JSON body + * - PUT / PATCH / DELETE - raw body (unless multipart/form-data) + * - GET / HEAD - query-string parameters + * - Everything else (POST) - POST body + * + * @return array + */ + protected function validationData(): array + { + $contentType = $this->request->getHeaderLine('Content-Type'); + + if (str_contains($contentType, 'application/json')) { + return $this->request->getJSON(true) ?? []; + } + + if ( + in_array($this->request->getMethod(), [Method::PUT, Method::PATCH, Method::DELETE], true) + && ! str_contains($contentType, 'multipart/form-data') + ) { + return $this->request->getRawInput() ?? []; + } + + if (in_array($this->request->getMethod(), [Method::GET, Method::HEAD], true)) { + return $this->request->getGet() ?? []; + } + + return $this->request->getPost() ?? []; + } + + /** + * Runs authorization and validation. Called by the framework before + * injecting the FormRequest into the controller method. + * + * Returns null on success, or a ResponseInterface to short-circuit the + * request when authorization or validation fails. + * + * Do not call this method directly unless you are inside a ``_remap()`` + * method, where automatic injection is not available. + * + * @internal + */ + final public function resolveRequest(): ?ResponseInterface + { + if (! $this->authorize()) { + return $this->failedAuthorization(); + } + + $data = $this->prepareForValidation($this->validationData()); + + $validation = service('validation') + ->setRules($this->rules(), $this->messages()); + + if (! $validation->run($data)) { + return $this->failedValidation($validation->getErrors()); + } + + $this->validatedData = $validation->getValidated(); + + return null; + } +} diff --git a/system/Language/en/CLI.php b/system/Language/en/CLI.php index 01e60c402955..f740688e30d7 100644 --- a/system/Language/en/CLI.php +++ b/system/Language/en/CLI.php @@ -26,6 +26,7 @@ 'default' => 'Class name', 'entity' => 'Entity class name', 'filter' => 'Filter class name', + 'request' => 'FormRequest class name', 'migration' => 'Migration class name', 'model' => 'Model class name', 'seeder' => 'Seeder class name', diff --git a/system/Router/AutoRouterImproved.php b/system/Router/AutoRouterImproved.php index a00c04d265d7..ab33a412b2bb 100644 --- a/system/Router/AutoRouterImproved.php +++ b/system/Router/AutoRouterImproved.php @@ -14,6 +14,7 @@ namespace CodeIgniter\Router; use CodeIgniter\Exceptions\PageNotFoundException; +use CodeIgniter\HTTP\FormRequest; use CodeIgniter\Router\Exceptions\MethodNotFoundException; use Config\Routing; use ReflectionClass; @@ -442,7 +443,14 @@ private function checkParameters(): void throw new MethodNotFoundException(); } - if (count($refParams) < count($this->params)) { + // FormRequest parameters are injected by the framework and do not + // consume URI segments, so exclude them from the count. + $uriParamCount = count(array_filter( + $refParams, + static fn ($p): bool => FormRequest::getFormRequestClass($p) === null, + )); + + if ($uriParamCount < count($this->params)) { throw new PageNotFoundException( 'The param count in the URI are greater than the controller method params.' . ' Handler:' . $this->controller . '::' . $this->method diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 95d50a723d8e..0ca1291b9120 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -667,7 +667,7 @@ public function map(array $routes = [], ?array $options = null): RouteCollection * Example: * $routes->add('news', 'Posts::index'); * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function add(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1013,7 +1013,7 @@ public function presenter(string $name, ?array $options = null): RouteCollection * Example: * $route->match( ['GET', 'POST'], 'users/(:num)', 'users/$1); * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function match(array $verbs = [], string $from = '', $to = '', ?array $options = null): RouteCollectionInterface { @@ -1045,7 +1045,7 @@ public function match(array $verbs = [], string $from = '', $to = '', ?array $op /** * Specifies a route that is only available to GET requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function get(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1057,7 +1057,7 @@ public function get(string $from, $to, ?array $options = null): RouteCollectionI /** * Specifies a route that is only available to POST requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function post(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1069,7 +1069,7 @@ public function post(string $from, $to, ?array $options = null): RouteCollection /** * Specifies a route that is only available to PUT requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function put(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1081,7 +1081,7 @@ public function put(string $from, $to, ?array $options = null): RouteCollectionI /** * Specifies a route that is only available to DELETE requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function delete(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1093,7 +1093,7 @@ public function delete(string $from, $to, ?array $options = null): RouteCollecti /** * Specifies a route that is only available to HEAD requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function head(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1105,7 +1105,7 @@ public function head(string $from, $to, ?array $options = null): RouteCollection /** * Specifies a route that is only available to PATCH requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function patch(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1117,7 +1117,7 @@ public function patch(string $from, $to, ?array $options = null): RouteCollectio /** * Specifies a route that is only available to OPTIONS requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function options(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1129,7 +1129,7 @@ public function options(string $from, $to, ?array $options = null): RouteCollect /** * Specifies a route that is only available to command-line requests. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to */ public function cli(string $from, $to, ?array $options = null): RouteCollectionInterface { @@ -1435,7 +1435,7 @@ private function replaceLocale(string $route, ?string $locale = null): string * the request method(s) that this route will work for. They can be separated * by a pipe character "|" if there is more than one. * - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to + * @param array|Closure|string $to * * @return void */ @@ -1772,7 +1772,7 @@ public function getRegisteredControllers(?string $verb = '*'): array } /** - * @param (Closure(mixed...): (ResponseInterface|string|void))|string $handler Handler + * @param Closure|string $handler Handler * * @return string|null Controller classname */ diff --git a/system/Router/RouteCollectionInterface.php b/system/Router/RouteCollectionInterface.php index 3c34a9be2d8a..d4f28197fa01 100644 --- a/system/Router/RouteCollectionInterface.php +++ b/system/Router/RouteCollectionInterface.php @@ -28,9 +28,12 @@ interface RouteCollectionInterface /** * Adds a single route to the collection. * - * @param string $from The route path (with placeholders or regex) - * @param array|(Closure(mixed...): (ResponseInterface|string|void))|string $to The route handler - * @param array|null $options The route options + * Route handler closure parameters are resolved dynamically via reflection, + * so their callable signatures cannot be expressed precisely in PHPDoc. + * + * @param string $from The route path (with placeholders or regex) + * @param array|Closure|string $to The route handler + * @param array|null $options The route options * * @return RouteCollectionInterface */ diff --git a/system/Router/Router.php b/system/Router/Router.php index ce71173ac814..be98c3b23945 100644 --- a/system/Router/Router.php +++ b/system/Router/Router.php @@ -71,7 +71,7 @@ class Router implements RouterInterface /** * The name of the controller class. * - * @var (Closure(mixed...): (ResponseInterface|string|void))|string + * @var Closure|string */ protected $controller; @@ -198,7 +198,7 @@ public function __construct(RouteCollectionInterface $routes, ?Request $request * * @param string|null $uri URI path relative to baseURL * - * @return (Closure(mixed...): (ResponseInterface|string|void))|string Controller classname or Closure + * @return Closure|string Controller classname or Closure * * @throws BadRequestException * @throws PageNotFoundException @@ -271,7 +271,7 @@ public function getFilters(): array /** * Returns the name of the matched controller or closure. * - * @return (Closure(mixed...): (ResponseInterface|string|void))|string Controller classname or Closure + * @return Closure|string Controller classname or Closure */ public function controllerName() { diff --git a/system/Router/RouterInterface.php b/system/Router/RouterInterface.php index c50de86d43df..ef84eeade8e4 100644 --- a/system/Router/RouterInterface.php +++ b/system/Router/RouterInterface.php @@ -15,7 +15,6 @@ use Closure; use CodeIgniter\HTTP\Request; -use CodeIgniter\HTTP\ResponseInterface; /** * Expected behavior of a Router. @@ -32,14 +31,14 @@ public function __construct(RouteCollectionInterface $routes, ?Request $request * * @param string|null $uri URI path relative to baseURL * - * @return (Closure(mixed...): (ResponseInterface|string|void))|string Controller classname or Closure + * @return Closure|string Controller classname or Closure */ public function handle(?string $uri = null); /** * Returns the name of the matched controller. * - * @return (Closure(mixed...): (ResponseInterface|string|void))|string Controller classname or Closure + * @return Closure|string Controller classname or Closure */ public function controllerName(); diff --git a/tests/_support/Controllers/FormRequestController.php b/tests/_support/Controllers/FormRequestController.php new file mode 100644 index 000000000000..58859e6e0499 --- /dev/null +++ b/tests/_support/Controllers/FormRequestController.php @@ -0,0 +1,74 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\Controllers; + +use CodeIgniter\Controller; +use Tests\Support\HTTP\Requests\UnauthorizedFormRequest; +use Tests\Support\HTTP\Requests\ValidPostFormRequest; + +/** + * Controller used in FormRequest integration tests. + */ +class FormRequestController extends Controller +{ + /** + * Optional trailing param after a FormRequest - verifies that the optional + * param gets its default value when the corresponding URI segment is absent. + */ + public function index(string $id, ValidPostFormRequest $request, string $format = 'json'): string + { + return json_encode(['id' => $id, 'format' => $format, 'data' => $request->validated()]); + } + + /** + * Receives only a FormRequest (no route params). + */ + public function store(ValidPostFormRequest $request): string + { + return json_encode($request->validated()); + } + + /** + * Receives a route param alongside a FormRequest. + */ + public function update(string $id, ValidPostFormRequest $request): string + { + return json_encode(['id' => $id, 'data' => $request->validated()]); + } + + /** + * No FormRequest - verifies BC with plain route params. + */ + public function show(string $id): string + { + return 'item-' . $id; + } + + /** + * Variadic route params alongside a FormRequest - verifies that all extra + * URI segments are collected into the variadic array. + */ + public function search(ValidPostFormRequest $request, string ...$tags): string + { + return json_encode(['tags' => $tags, 'data' => $request->validated()]); + } + + /** + * Uses an always-unauthorized FormRequest. + */ + public function restricted(UnauthorizedFormRequest $request): string + { + return 'should-not-reach'; + } +} diff --git a/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php b/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php new file mode 100644 index 000000000000..701b21d5de03 --- /dev/null +++ b/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\HTTP\Requests; + +use CodeIgniter\HTTP\FormRequest; + +/** + * A FormRequest that always denies authorization. + */ +class UnauthorizedFormRequest extends FormRequest +{ + public function rules(): array + { + return ['title' => 'required']; + } + + public function authorize(): bool + { + return false; + } +} diff --git a/tests/_support/HTTP/Requests/ValidPostFormRequest.php b/tests/_support/HTTP/Requests/ValidPostFormRequest.php new file mode 100644 index 000000000000..636f296b088d --- /dev/null +++ b/tests/_support/HTTP/Requests/ValidPostFormRequest.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\HTTP\Requests; + +use CodeIgniter\HTTP\FormRequest; + +/** + * A FormRequest used in integration tests requiring title + body. + * + * @property-read string $body + * @property-read string $title + */ +class ValidPostFormRequest extends FormRequest +{ + public function rules(): array + { + return [ + 'title' => 'required|min_length[3]', + 'body' => 'required', + ]; + } +} diff --git a/tests/system/HTTP/FormRequestTest.php b/tests/system/HTTP/FormRequestTest.php new file mode 100644 index 000000000000..588472d504dc --- /dev/null +++ b/tests/system/HTTP/FormRequestTest.php @@ -0,0 +1,710 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\Config\Services; +use CodeIgniter\Superglobals; +use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\Mock\MockCodeIgniter; +use Config\App; +use PHPUnit\Framework\Attributes\BackupGlobals; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\Attributes\RunInSeparateProcess; +use Tests\Support\Controllers\FormRequestController; +use Tests\Support\HTTP\Requests\ValidPostFormRequest; + +/** + * @internal + */ +#[BackupGlobals(true)] +#[Group('Others')] +final class FormRequestTest extends CIUnitTestCase +{ + private MockCodeIgniter $codeigniter; + + protected function setUp(): void + { + parent::setUp(); + + $this->resetServices(); + Services::injectMock('superglobals', new Superglobals()); + service('superglobals')->setServer('REQUEST_METHOD', 'POST'); + service('superglobals')->setServer('SERVER_PROTOCOL', 'HTTP/1.1'); + service('superglobals')->setServer('SERVER_NAME', 'example.com'); + service('superglobals')->setServer('HTTP_HOST', 'example.com'); + /** @var Response $response */ + $response = service('response'); + $response->pretend(true); + $this->codeigniter = new MockCodeIgniter(new App()); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + /** + * Creates an IncomingRequest. + * POST data must be set on the superglobals BEFORE calling this. + */ + private function makeRequest(?string $body = null): IncomingRequest + { + $config = new App(); + + return new IncomingRequest($config, new SiteURI($config), $body, new UserAgent()); + } + + /** + * Returns a concrete FormRequest subclass instance that requires + * 'title' (required|min_length[3]) and 'body' (required). + */ + private function makeFormRequest(IncomingRequest $request): FormRequest + { + return new class ($request) extends FormRequest { + public function rules(): array + { + return [ + 'title' => 'required|min_length[3]', + 'body' => 'required', + ]; + } + }; + } + + /** + * Injects a router pointing at FormRequestController::$method + * for the given URI, then runs the app and returns the response. + */ + private function runRequest(string $uri, string $method, string $httpMethod = 'GET'): ResponseInterface + { + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', $httpMethod); + $superglobals->setServer('REQUEST_URI', $uri); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', ltrim($uri, '/')]); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + $routes->add(ltrim($uri, '/'), '\\' . FormRequestController::class . '::' . $method); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + return $this->codeigniter->run(null, true); + } + + // ------------------------------------------------------------------------- + // Default behaviours + // ------------------------------------------------------------------------- + + public function testDefaultMessagesReturnsEmptyArray(): void + { + $formRequest = $this->makeFormRequest($this->makeRequest()); + + $this->assertSame([], $formRequest->messages()); + } + + public function testDefaultAuthorizeReturnsTrue(): void + { + $formRequest = $this->makeFormRequest($this->makeRequest()); + + $this->assertTrue($formRequest->authorize()); + } + + public function testValidatedReturnsEmptyArrayBeforeResolution(): void + { + $formRequest = $this->makeFormRequest($this->makeRequest()); + + $this->assertSame([], $formRequest->validated()); + } + + // ------------------------------------------------------------------------- + // Successful resolution + // ------------------------------------------------------------------------- + + public function testResolveRequestPassesWithValidData(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + + $formRequest = $this->makeFormRequest($this->makeRequest()); + $this->assertNotInstanceOf(ResponseInterface::class, $formRequest->resolveRequest()); + + $this->assertSame( + ['title' => 'Hello World', 'body' => 'Some body text'], + $formRequest->validated(), + ); + } + + public function testValidatedReturnsOnlyFieldsCoveredByRules(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + service('superglobals')->setPost('extra_field', 'should be excluded'); + + $formRequest = $this->makeFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $validated = $formRequest->validated(); + + $this->assertArrayHasKey('title', $validated); + $this->assertArrayHasKey('body', $validated); + $this->assertArrayNotHasKey('extra_field', $validated); + } + + // ------------------------------------------------------------------------- + // __get / __isset - property-style access to validated fields + // ------------------------------------------------------------------------- + + public function testMagicGetReturnsValidatedFieldValue(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + + $formRequest = new ValidPostFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $this->assertSame('Hello World', $formRequest->title); + $this->assertSame('Some body text', $formRequest->body); + } + + public function testMagicGetReturnsNullForMissingField(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + + $formRequest = new ValidPostFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $this->assertNull($formRequest->nonexistent); // @phpstan-ignore property.notFound + } + + public function testMagicGetReturnsNullBeforeValidationRuns(): void + { + $formRequest = new ValidPostFormRequest($this->makeRequest()); + + $this->assertNull($formRequest->title); + } + + public function testMagicIssetReturnsTrueForValidatedField(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + + $formRequest = new ValidPostFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $this->assertTrue(isset($formRequest->title)); + } + + public function testMagicIssetReturnsFalseForMissingField(): void + { + service('superglobals')->setPost('title', 'Hello World'); + service('superglobals')->setPost('body', 'Some body text'); + + $formRequest = new ValidPostFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $this->assertFalse(isset($formRequest->nonexistent)); // @phpstan-ignore property.notFound + } + + // ------------------------------------------------------------------------- + // prepareForValidation hook + // ------------------------------------------------------------------------- + + public function testPrepareForValidationIsCalledBeforeValidation(): void + { + service('superglobals')->setPost('title', 'Hi'); // Too short - min_length[3] would normally fail. + + // This FormRequest normalises the title in prepareForValidation so it passes. + $formRequest = new class ($this->makeRequest()) extends FormRequest { + public static bool $prepareCalled = false; + + public function rules(): array + { + return ['title' => 'required|min_length[3]']; + } + + protected function prepareForValidation(array $data): array + { + self::$prepareCalled = true; + // Extend the title so it meets the min_length rule. + $data['title'] = 'Hi extended'; + + return $data; + } + }; + + $this->assertNotInstanceOf(ResponseInterface::class, $formRequest->resolveRequest()); + + $this->assertTrue($formRequest::$prepareCalled); + $this->assertSame('Hi extended', $formRequest->validated()['title']); + } + + // ------------------------------------------------------------------------- + // Validation failure + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testFailedValidationFlashesErrorsToSession(): void + { + /** @var array $_SESSION */ + $_SESSION = []; + + // No POST data - both required rules will fail and the default + // failedValidation() should flash errors to _ci_validation_errors. + $formRequest = $this->makeFormRequest($this->makeRequest()); + $formRequest->resolveRequest(); + + $this->assertArrayHasKey('_ci_validation_errors', $_SESSION); + $this->assertArrayHasKey('title', $_SESSION['_ci_validation_errors']); + $this->assertArrayHasKey('body', $_SESSION['_ci_validation_errors']); + } + + public function testResolveRequestReturnsResponseOnInvalidData(): void + { + // No POST data - both required rules will fail. + $formRequest = $this->makeFormRequest($this->makeRequest()); + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertInstanceOf(ResponseInterface::class, $response); + } + + public function testFailedValidationResponseContainsErrors(): void + { + service('superglobals')->setServer('CONTENT_TYPE', 'application/json'); + + $formRequest = new class ($this->makeRequest('{}')) extends FormRequest { + public function rules(): array + { + return ['title' => 'required']; + } + }; + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(422, $response->getStatusCode()); + + $body = json_decode($response->getBody(), true); + $this->assertArrayHasKey('title', $body['errors']); + } + + public function testResolveRequestReturns422ForJsonRequest(): void + { + service('superglobals')->setServer('CONTENT_TYPE', 'application/json'); + // Body is an empty JSON object - no fields provided. + $formRequest = $this->makeFormRequest($this->makeRequest('{}')); + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(422, $response->getStatusCode()); + } + + public function testResolveRequestReturns422ForAjaxRequest(): void + { + service('superglobals')->setServer('HTTP_X_REQUESTED_WITH', 'XMLHttpRequest'); + $formRequest = $this->makeFormRequest($this->makeRequest()); + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(422, $response->getStatusCode()); + } + + // ------------------------------------------------------------------------- + // Authorization failure + // ------------------------------------------------------------------------- + + public function testResolveRequestReturns403WhenUnauthorized(): void + { + $formRequest = new class ($this->makeRequest()) extends FormRequest { + public function rules(): array + { + return []; + } + + public function authorize(): bool + { + return false; + } + }; + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(403, $response->getStatusCode()); + } + + public function testAuthorizationIsCheckedBeforeValidation(): void + { + // Use a static property to record call order without needing a custom constructor. + $formRequest = new class ($this->makeRequest()) extends FormRequest { + /** + * @var list + */ + public static array $order = []; + + public function rules(): array + { + return ['title' => 'required']; + } + + public function authorize(): bool + { + self::$order[] = 'authorize'; + + return false; + } + + protected function prepareForValidation(array $data): array + { + self::$order[] = 'prepare'; + + return $data; + } + }; + + $formRequest->resolveRequest(); + + // authorize() must fire before prepareForValidation(); validation never runs. + $this->assertSame(['authorize'], $formRequest::$order); + } + + // ------------------------------------------------------------------------- + // Custom overrides + // ------------------------------------------------------------------------- + + public function testValidationDataOverrideIsUsed(): void + { + // No POST data at all - but validationData() supplies its own. + $formRequest = new class ($this->makeRequest()) extends FormRequest { + public function rules(): array + { + return ['title' => 'required|min_length[3]']; + } + + protected function validationData(): array + { + return ['title' => 'Injected Title']; + } + }; + + $this->assertNotInstanceOf(ResponseInterface::class, $formRequest->resolveRequest()); + + $this->assertSame('Injected Title', $formRequest->validated()['title']); + } + + public function testCustomFailedValidationIsRespected(): void + { + $formRequest = new class ($this->makeRequest()) extends FormRequest { + public static bool $called = false; + + public function rules(): array + { + return ['title' => 'required']; + } + + protected function failedValidation(array $errors): ResponseInterface + { + self::$called = true; + + return service('response')->setStatusCode(422)->setJSON(['errors' => $errors]); + } + }; + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertTrue($formRequest::$called); + } + + public function testCustomFailedAuthorizationIsRespected(): void + { + $formRequest = new class ($this->makeRequest()) extends FormRequest { + public static bool $called = false; + + public function rules(): array + { + return []; + } + + public function authorize(): bool + { + return false; + } + + protected function failedAuthorization(): ResponseInterface + { + self::$called = true; + + return service('response')->setStatusCode(403); + } + }; + + $response = $formRequest->resolveRequest(); + + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertTrue($formRequest::$called); + } + + // ------------------------------------------------------------------------- + // Integration: BC - methods without FormRequest are unaffected + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testControllerMethodWithoutFormRequestReceivesRouteParam(): void + { + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', 'GET'); + $superglobals->setServer('REQUEST_URI', '/items/42'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', 'items/42']); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + $routes->add('items/(:segment)', '\\' . FormRequestController::class . '::show/$1'); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + $response = $this->codeigniter->run(null, true); + $this->assertInstanceOf(ResponseInterface::class, $response); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertStringContainsString('item-42', (string) $response->getBody()); + } + + // ------------------------------------------------------------------------- + // Integration: valid FormRequest + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testValidFormRequestInjectedAndControllerReceivesValidatedData(): void + { + service('superglobals')->setPost('title', 'My Post'); + service('superglobals')->setPost('body', 'Post content here'); + + $response = $this->runRequest('/posts', 'store', 'POST'); + + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertSame('My Post', $body['title']); + $this->assertSame('Post content here', $body['body']); + } + + #[RunInSeparateProcess] + public function testRouteParamAndFormRequestBothReachController(): void + { + service('superglobals')->setPost('title', 'Updated Title'); + service('superglobals')->setPost('body', 'Updated body content'); + + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', 'POST'); + $superglobals->setServer('REQUEST_URI', '/posts/99'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', 'posts/99']); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + $routes->add('posts/(:segment)', '\\' . FormRequestController::class . '::update/$1'); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + $response = $this->codeigniter->run(null, true); + $this->assertInstanceOf(ResponseInterface::class, $response); + + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertSame('99', $body['id']); + $this->assertSame('Updated Title', $body['data']['title']); + $this->assertSame('Updated body content', $body['data']['body']); + } + + #[RunInSeparateProcess] + public function testOptionalTrailingParamReceivesDefaultWhenSegmentAbsent(): void + { + service('superglobals')->setPost('title', 'Hello'); + service('superglobals')->setPost('body', 'Body text'); + + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', 'POST'); + $superglobals->setServer('REQUEST_URI', '/posts/42'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', 'posts/42']); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + // Route provides only $id; $format is absent so its default 'json' applies. + $routes->add('posts/(:segment)', '\\' . FormRequestController::class . '::index/$1'); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + $response = $this->codeigniter->run(null, true); + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertSame('42', $body['id']); + $this->assertSame('json', $body['format']); + $this->assertSame('Hello', $body['data']['title']); + } + + #[RunInSeparateProcess] + public function testVariadicRouteParamsAlongsideFormRequestAreAllCollected(): void + { + service('superglobals')->setPost('title', 'Tagged Post'); + service('superglobals')->setPost('body', 'Post body'); + + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', 'POST'); + $superglobals->setServer('REQUEST_URI', '/search/php/ci4'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', 'search/php/ci4']); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + $routes->add('search/(:segment)/(:segment)', '\\' . FormRequestController::class . '::search/$1/$2'); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + $response = $this->codeigniter->run(null, true); + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertSame(['php', 'ci4'], $body['tags']); + $this->assertSame('Tagged Post', $body['data']['title']); + } + + #[RunInSeparateProcess] + public function testClosureRouteWithFormRequestIsInjected(): void + { + service('superglobals')->setPost('title', 'Closure Title'); + service('superglobals')->setPost('body', 'Closure body text'); + + $superglobals = service('superglobals'); + $superglobals->setServer('REQUEST_METHOD', 'POST'); + $superglobals->setServer('REQUEST_URI', '/closure/55'); + $superglobals->setServer('SCRIPT_NAME', '/index.php'); + $superglobals->setServer('argv', ['index.php', 'closure/55']); + $superglobals->setServer('argc', 2); + + $routes = service('routes'); + $routes->setAutoRoute(false); + $routes->add('closure/(:segment)', static fn (string $id, ValidPostFormRequest $request): string => json_encode(['id' => $id, 'data' => $request->validated()])); + + $router = service('router', $routes, service('incomingrequest')); + Services::injectMock('router', $router); + + $response = $this->codeigniter->run(null, true); + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertSame('55', $body['id']); + $this->assertSame('Closure Title', $body['data']['title']); + $this->assertSame('Closure body text', $body['data']['body']); + } + + // ------------------------------------------------------------------------- + // Integration: validation failure - web redirect + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testInvalidFormRequestRedirectsWebRequest(): void + { + service('superglobals')->setServer('HTTP_REFERER', 'http://example.com/posts/create'); + + // No POST data → required rules fail. + $response = $this->runRequest('/posts', 'store', 'POST'); + + // For POST requests under HTTP/1.1, CI4 correctly issues 303 See Other (Post/Redirect/Get). + $this->assertSame(303, $response->getStatusCode()); + } + + // ------------------------------------------------------------------------- + // Integration: validation failure - JSON / AJAX + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testInvalidFormRequestReturns422ForJsonRequest(): void + { + service('superglobals')->setServer('CONTENT_TYPE', 'application/json'); + + // No valid POST/JSON data → required rules fail. + $response = $this->runRequest('/posts', 'store', 'POST'); + + $this->assertSame(422, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertArrayHasKey('errors', $body); + $this->assertArrayHasKey('title', $body['errors']); + } + + #[RunInSeparateProcess] + public function testInvalidFormRequestReturns422ForAjaxRequest(): void + { + service('superglobals')->setServer('HTTP_X_REQUESTED_WITH', 'XMLHttpRequest'); + + $response = $this->runRequest('/posts', 'store', 'POST'); + + $this->assertSame(422, $response->getStatusCode()); + } + + // ------------------------------------------------------------------------- + // Integration: authorization failure + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testUnauthorizedFormRequestReturns403(): void + { + $response = $this->runRequest('/admin/resource', 'restricted', 'POST'); + + $this->assertSame(403, $response->getStatusCode()); + } + + // ------------------------------------------------------------------------- + // Integration: validated() only returns fields declared in rules() + // ------------------------------------------------------------------------- + + #[RunInSeparateProcess] + public function testValidatedExcludesFieldsNotInRules(): void + { + service('superglobals')->setPost('title', 'A title that is long enough'); + service('superglobals')->setPost('body', 'The body'); + service('superglobals')->setPost('__csrf_token', 'secret'); + service('superglobals')->setPost('extra_noise', 'ignored'); + + $response = $this->runRequest('/posts', 'store', 'POST'); + + $this->assertSame(200, $response->getStatusCode()); + + $body = json_decode((string) $response->getBody(), true); + $this->assertArrayNotHasKey('__csrf_token', $body); + $this->assertArrayNotHasKey('extra_noise', $body); + } +} diff --git a/tests/system/Router/AutoRouterImprovedTest.php b/tests/system/Router/AutoRouterImprovedTest.php index 8a4132a8f297..c1dfe894f03d 100644 --- a/tests/system/Router/AutoRouterImprovedTest.php +++ b/tests/system/Router/AutoRouterImprovedTest.php @@ -159,6 +159,31 @@ public function testUriParamCountIsGreaterThanMethodParams(): void $router->getRoute('mycontroller/somemethod/a/b', Method::GET); } + public function testFormRequestParamDoesNotCountAsUriSegment(): void + { + $router = $this->createNewAutoRouter(); + + // FormRequest does not consume a URI segment, so this should route fine + // with zero URI params after the method name. + [$directory, $controller, $method, $params] + = $router->getRoute('mycontroller/formmethod', Method::GET); + + $this->assertSame('\\' . Mycontroller::class, $controller); + $this->assertSame('getFormmethod', $method); + $this->assertSame([], $params); + } + + public function testUriParamCountExceedsNonFormRequestParams(): void + { + $this->expectException(PageNotFoundException::class); + + $router = $this->createNewAutoRouter(); + + // Only $id absorbs a URI segment; the FormRequest does not. + // Passing two segments after the method name must be rejected. + $router->getRoute('mycontroller/formmethodwithparam/42/extra', Method::GET); + } + public function testAutoRouteFindsControllerWithFile(): void { $router = $this->createNewAutoRouter(); diff --git a/tests/system/Router/Controllers/Mycontroller.php b/tests/system/Router/Controllers/Mycontroller.php index 75ad1eae026e..e5889da99c60 100644 --- a/tests/system/Router/Controllers/Mycontroller.php +++ b/tests/system/Router/Controllers/Mycontroller.php @@ -14,6 +14,7 @@ namespace CodeIgniter\Router\Controllers; use CodeIgniter\Controller; +use CodeIgniter\Router\Controllers\Requests\MyFormRequest; class Mycontroller extends Controller { @@ -24,4 +25,12 @@ public function getIndex(): void public function getSomemethod($first = ''): void { } + + public function getFormmethod(MyFormRequest $request): void + { + } + + public function getFormmethodWithParam(string $id, MyFormRequest $request): void + { + } } diff --git a/tests/system/Router/Controllers/Requests/MyFormRequest.php b/tests/system/Router/Controllers/Requests/MyFormRequest.php new file mode 100644 index 000000000000..c2e4805d7e27 --- /dev/null +++ b/tests/system/Router/Controllers/Requests/MyFormRequest.php @@ -0,0 +1,24 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Router\Controllers\Requests; + +use CodeIgniter\HTTP\FormRequest; + +class MyFormRequest extends FormRequest +{ + public function rules(): array + { + return []; + } +} diff --git a/user_guide_src/source/changelogs/v4.8.0.rst b/user_guide_src/source/changelogs/v4.8.0.rst index 045c38f11ce5..c8f3a541ac24 100644 --- a/user_guide_src/source/changelogs/v4.8.0.rst +++ b/user_guide_src/source/changelogs/v4.8.0.rst @@ -179,6 +179,7 @@ Commands - ``CLI`` now supports parsing array options written multiple times (e.g., ``--option=value1 --option=value2``) into an array of values. This allows you to easily pass multiple values for the same option without needing to use a comma-separated string. When used with ``CLI::getOption()``, an array option will return its last value (for example, in this case, ``value2``). To retrieve all values for an array option, use ``CLI::getRawOption()``. - Likewise, the ``command()`` function now also supports the above enhancements for command-line option parsing when using the function to run commands from code. +- Added ``make:request`` generator command to scaffold :ref:`Form Request ` classes. Testing ======= @@ -226,6 +227,7 @@ Helpers and Functions HTTP ==== +- Added :ref:`Form Requests ` - a new ``FormRequest`` base class that encapsulates validation rules, custom error messages, and authorization logic for a single HTTP request. - Added ``SSEResponse`` class for streaming Server-Sent Events (SSE) over HTTP. See :ref:`server-sent-events`. - ``Response`` and its child classes no longer require ``Config\App`` passed to their constructors. Consequently, ``CURLRequest``'s ``$config`` parameter is unused and will be removed in a future release. diff --git a/user_guide_src/source/incoming/form_requests.rst b/user_guide_src/source/incoming/form_requests.rst new file mode 100644 index 000000000000..1bda12d8c88e --- /dev/null +++ b/user_guide_src/source/incoming/form_requests.rst @@ -0,0 +1,242 @@ +.. _form-requests: + +############# +Form Requests +############# + +.. versionadded:: 4.8.0 + +A **FormRequest** is a dedicated class that encapsulates the validation rules, +custom error messages, and authorization logic for a single HTTP request. +Instead of writing validation code inside every controller method, you define +it once in a FormRequest class and type-hint it in the controller method +signature - the framework resolves, authorizes, and validates the request +automatically before your method body runs. + +.. contents:: + :local: + :depth: 2 + +*********************** +Creating a Form Request +*********************** + +Use the ``make:request`` Spark command to generate a new FormRequest class:: + + php spark make:request StorePostRequest + +This creates **app/Requests/StorePostRequest.php**. Fill in the ``rules()`` +method with the same field/rule pairs you would normally pass to +:ref:`validation-running`: + +.. literalinclude:: form_requests/001.php + :lines: 2- + +************************************ +Using a Form Request in a Controller +************************************ + +Type-hint the FormRequest class as a parameter of your controller method. The +framework instantiates it, runs authorization and validation, and passes the +resolved object to your method. If validation fails, the default behavior +redirects back with errors for web requests or returns a 422 JSON response for +JSON/AJAX requests - the method body is never reached. + +.. literalinclude:: form_requests/002.php + :lines: 2- + +************************ +Accessing Validated Data +************************ + +``validated()`` returns an array containing only the fields that were declared +in ``rules()``. Fields submitted by the client that are not covered by a rule +are silently discarded, protecting against mass-assignment. + +.. literalinclude:: form_requests/009.php + :lines: 2- + +Individual fields can also be read as object properties. This is equivalent to +``$request->validated()['field'] ?? null``: + +.. literalinclude:: form_requests/014.php + :lines: 2- + +``isset($request->field)`` returns ``true`` when the field was validated and +has a non-null value, following standard PHP ``isset()`` semantics. + +Accessing Other Request Data +============================ + +For anything not covered by ``validated()`` - uploaded files, request headers, +the client IP address, raw input, and so on - use ``$this->request`` as usual. +It is the same :doc:`IncomingRequest ` instance that +the FormRequest uses internally: + +.. literalinclude:: form_requests/010.php + :lines: 2- + +******************************************* +Route Parameters Alongside a Form Request +******************************************* + +Required scalar route parameters come first, then the FormRequest, then any optional +scalar parameters. The framework matches URI segments to scalar parameters positionally +and injects the FormRequest wherever the type hint appears. Parameters declared with +PHP's ``...`` syntax captures all remaining URI segments. Optional scalar parameters +after the FormRequest get their default values when no URI segment is left to fill them. + +.. literalinclude:: form_requests/003.php + :lines: 2- + +************** +Closure Routes +************** + +FormRequest injection works identically in closure routes and follows the same +signature shape: route parameters first, then the FormRequest, then optional +scalar parameters. The framework resolves it the same way it does for controller +methods. + +.. literalinclude:: form_requests/011.php + :lines: 2- + +******************** +``_remap()`` Methods +******************** + +Automatic FormRequest injection is **not** supported for controller methods that +use ``_remap()``. Because ``_remap()`` has a fixed signature +``($method, ...$params)``, there is no typed position for the framework to +inject a FormRequest into. + +Instantiate the FormRequest manually inside ``_remap()`` and call +``resolveRequest()`` yourself. The method returns ``null`` on success or a +``ResponseInterface`` when authorization or validation fails: + +.. literalinclude:: form_requests/012.php + :lines: 2- + +********************* +Custom Error Messages +********************* + +Override ``messages()`` to return field-specific error messages. The format is +identical to the ``$errors`` argument of :ref:`saving-validation-rules-to-config-file`: + +.. literalinclude:: form_requests/004.php + :lines: 2- + +************* +Authorization +************* + +Override ``authorize()`` to control whether the current user is allowed to make +this request. Return ``false`` to reject the request with a 403 Forbidden +response before validation even runs. + +.. literalinclude:: form_requests/005.php + :lines: 2- + +.. note:: The ``authorize()`` check runs before ``prepareForValidation()`` and + before validation itself. An unauthorized request never reaches the + validation stage. + +********************************* +Preparing Data Before Validation +********************************* + +Override ``prepareForValidation(array $data): array`` to normalize or derive +input values before the validation rules are applied. The method receives the +same data array that will be passed to the validator and must return the +(possibly modified) array. This is useful for computed fields such as slugs, +normalized phone numbers, or trimmed strings. + +.. literalinclude:: form_requests/006.php + :lines: 2- + +.. note:: ``old()`` returns the original submitted input, not the normalized + values. Use ``validated()`` to access the processed data after a successful + request. If you need ``old()`` to reflect normalized values, see + :ref:`form-request-flash-normalized`. + +.. _form-request-validation-data: + +**************************** +Customizing the Data Source +**************************** + +By default ``validationData()`` selects the appropriate data source +automatically based on the HTTP method and ``Content-Type`` header: + +* **JSON request** (``Content-Type: application/json``) -> decoded JSON body +* **PUT / PATCH / DELETE** (non-multipart) -> raw body via ``getRawInput()`` +* **GET / HEAD** -> query-string parameters via ``getGet()`` +* **Everything else** (POST, multipart) -> POST body via ``getPost()`` + +This avoids the pitfalls of :ref:`validation-withrequest`, which mixes GET and +POST data via ``getVar()``. + +Override ``validationData()`` when you need a different data source - for +example, to merge GET and POST parameters: + +.. literalinclude:: form_requests/007.php + :lines: 2- + +**************************** +Customizing Failure Behavior +**************************** + +Override ``failedValidation()`` and ``failedAuthorization()`` to take full +control of what happens when a request is rejected. Both methods return a +``ResponseInterface`` that the framework sends to the client: + +.. literalinclude:: form_requests/008.php + :lines: 2- + +The default ``failedValidation()`` already detects JSON and AJAX requests and +returns the appropriate 422 response automatically. Override it only when you +need a different behavior, such as always responding with JSON even for ordinary +browser requests. + +.. _form-request-flash-normalized: + +Flashing Normalized Input +========================= + +If your ``prepareForValidation()`` transforms visible form fields (for example, +trimming strings or canonicalizing values), ``old()`` will return the original +submitted input because the redirect flashes the raw superglobals. To make +``old()`` reflect the normalized values instead, override ``failedValidation()`` +and flash the normalized payload manually: + +.. literalinclude:: form_requests/013.php + :lines: 2- + +***************************************** +How the Framework Resolves Form Requests +***************************************** + +When the router dispatches a controller method or closure route, the framework +inspects the callable's parameter list using reflection. For each parameter +whose type extends ``FormRequest``: + +#. A new instance is created with the current request injected via the + constructor. +#. ``authorize()`` is called. If it returns ``false``, ``failedAuthorization()`` + is called, and its response is returned to the client. +#. ``validationData()`` collects the data to validate. +#. ``prepareForValidation()`` receives that data and may modify it before the + rules are applied. +#. ``run()`` executes the validation rules. If it fails, ``failedValidation()`` + is called, and its response is returned to the client. +#. The validated data is stored internally and available via ``validated()``. +#. The resolved FormRequest object is injected into the controller method or + closure. + +The callable is never invoked if authorization or validation fails. Non-FormRequest +parameters consume URI route segments in declaration order; variadic parameters +receive all remaining segments. + +.. note:: Automatic injection does not apply to ``_remap()`` methods. See + `_remap() Methods`_ above for the manual workaround. diff --git a/user_guide_src/source/incoming/form_requests/001.php b/user_guide_src/source/incoming/form_requests/001.php new file mode 100644 index 000000000000..d8bf7bf464eb --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/001.php @@ -0,0 +1,16 @@ + 'required|min_length[3]|max_length[255]', + 'body' => 'required', + ]; + } +} diff --git a/user_guide_src/source/incoming/form_requests/002.php b/user_guide_src/source/incoming/form_requests/002.php new file mode 100644 index 000000000000..8f3a2115bc76 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/002.php @@ -0,0 +1,18 @@ +validated() returns only the fields declared in rules(). + $data = $request->validated(); + + // save to database + + return redirect()->to('/posts'); + } +} diff --git a/user_guide_src/source/incoming/form_requests/003.php b/user_guide_src/source/incoming/form_requests/003.php new file mode 100644 index 000000000000..e653af09f526 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/003.php @@ -0,0 +1,18 @@ +validated(); + + // update post $id with $data + + return redirect()->to('/posts/' . $id); + } +} diff --git a/user_guide_src/source/incoming/form_requests/004.php b/user_guide_src/source/incoming/form_requests/004.php new file mode 100644 index 000000000000..96f4f2a8fbc2 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/004.php @@ -0,0 +1,29 @@ + 'required|min_length[3]|max_length[255]', + 'body' => 'required', + ]; + } + + public function messages(): array + { + return [ + 'title' => [ + 'required' => 'Post title cannot be empty.', + 'min_length' => 'Post title must be at least {param} characters long.', + ], + 'body' => [ + 'required' => 'Post body cannot be empty.', + ], + ]; + } +} diff --git a/user_guide_src/source/incoming/form_requests/005.php b/user_guide_src/source/incoming/form_requests/005.php new file mode 100644 index 000000000000..8daff0464c7c --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/005.php @@ -0,0 +1,22 @@ + 'required|min_length[3]', + 'body' => 'required', + ]; + } + + public function authorize(): bool + { + // Only authenticated users may submit posts. + return auth()->loggedIn(); + } +} diff --git a/user_guide_src/source/incoming/form_requests/006.php b/user_guide_src/source/incoming/form_requests/006.php new file mode 100644 index 000000000000..f28f8f2a07a5 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/006.php @@ -0,0 +1,25 @@ + 'required|min_length[3]', + 'slug' => 'required|is_unique[posts.slug]', + ]; + } + + protected function prepareForValidation(array $data): array + { + // Derive the slug from the title so it is available for the + // is_unique rule before validation runs. + $data['slug'] = url_title($data['title'] ?? '', '-', true); + + return $data; + } +} diff --git a/user_guide_src/source/incoming/form_requests/007.php b/user_guide_src/source/incoming/form_requests/007.php new file mode 100644 index 000000000000..806765941576 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/007.php @@ -0,0 +1,22 @@ + 'required|min_length[2]']; + } + + // Merge query-string parameters and POST body for this endpoint. + protected function validationData(): array + { + return array_merge( + $this->request->getGet() ?? [], + $this->request->getPost() ?? [], + ); + } +} diff --git a/user_guide_src/source/incoming/form_requests/008.php b/user_guide_src/source/incoming/form_requests/008.php new file mode 100644 index 000000000000..e5d99b9fe72a --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/008.php @@ -0,0 +1,29 @@ + 'required|min_length[3]', + 'body' => 'required', + ]; + } + + // Always respond with JSON, regardless of the request type. + protected function failedValidation(array $errors): ResponseInterface + { + return service('response')->setStatusCode(422)->setJSON(['errors' => $errors]); + } + + // Always respond with 403, regardless of the request type. + protected function failedAuthorization(): ResponseInterface + { + return service('response')->setStatusCode(403); + } +} diff --git a/user_guide_src/source/incoming/form_requests/009.php b/user_guide_src/source/incoming/form_requests/009.php new file mode 100644 index 000000000000..f6feedacb799 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/009.php @@ -0,0 +1,4 @@ +validated(); +// ['title' => 'My post title', 'body' => 'Body text'] diff --git a/user_guide_src/source/incoming/form_requests/010.php b/user_guide_src/source/incoming/form_requests/010.php new file mode 100644 index 000000000000..0da8f97e9d3e --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/010.php @@ -0,0 +1,16 @@ +validated(); + $files = $this->request->getFiles(); + + // ... + } +} diff --git a/user_guide_src/source/incoming/form_requests/011.php b/user_guide_src/source/incoming/form_requests/011.php new file mode 100644 index 000000000000..4d700dd5ad86 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/011.php @@ -0,0 +1,24 @@ +post('posts', static function (StorePostRequest $request): string { + $data = $request->validated(); + + // save to database + + return redirect()->to('/posts'); +}); + +// Route parameter before a FormRequest - same convention as controller methods. +$routes->post('posts/(:num)', static function (int $id, UpdatePostRequest $request): string { + $data = $request->validated(); + + // update post $id with $data + + return redirect()->to('/posts/' . $id); +}); diff --git a/user_guide_src/source/incoming/form_requests/012.php b/user_guide_src/source/incoming/form_requests/012.php new file mode 100644 index 000000000000..ed3cb835414b --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/012.php @@ -0,0 +1,37 @@ +resolveRequest(); + + if ($response instanceof ResponseInterface) { + // Authorization or validation failed - send the response. + return $response; + } + + return $this->update($request, ...$params); + } + + return $this->{$method}(...$params); + } + + private function update(UpdatePostRequest $request, string $id): string + { + $data = $request->validated(); + + // update post $id with $data + + return redirect()->to('/posts/' . $id); + } +} diff --git a/user_guide_src/source/incoming/form_requests/013.php b/user_guide_src/source/incoming/form_requests/013.php new file mode 100644 index 000000000000..b4d22e2fd729 --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/013.php @@ -0,0 +1,44 @@ + 'required|min_length[3]', + 'slug' => 'required|is_unique[posts.slug]', + ]; + } + + protected function prepareForValidation(array $data): array + { + $data['slug'] = url_title($data['title'] ?? '', '-', true); + + return $data; + } + + // Override so that old() reflects the normalized values on redirect. + protected function failedValidation(array $errors): ResponseInterface + { + if ($this->request->is('json') || $this->request->isAJAX()) { + return service('response')->setStatusCode(422)->setJSON(['errors' => $errors]); + } + + // withInput() flashes the original superglobals and the validation + // errors. We then overwrite old input with the normalized payload so + // that old() returns the same values that were validated. + $redirect = redirect()->back()->withInput(); + + service('session')->setFlashdata('_ci_old_input', [ + 'get' => [], + 'post' => $this->prepareForValidation($this->validationData()), + ]); + + return $redirect; + } +} diff --git a/user_guide_src/source/incoming/form_requests/014.php b/user_guide_src/source/incoming/form_requests/014.php new file mode 100644 index 000000000000..9f446c6d272d --- /dev/null +++ b/user_guide_src/source/incoming/form_requests/014.php @@ -0,0 +1,8 @@ +title; // same as $request->validated()['title'] ?? null +$body = $request->body; + +if (isset($request->note)) { + // 'note' was validated and has a non-null value +} diff --git a/user_guide_src/source/incoming/index.rst b/user_guide_src/source/incoming/index.rst index ebcc316e10ee..0a2f518b20c3 100644 --- a/user_guide_src/source/incoming/index.rst +++ b/user_guide_src/source/incoming/index.rst @@ -15,6 +15,7 @@ Controllers handle incoming requests. message request incomingrequest + form_requests content_negotiation methodspoofing restful diff --git a/user_guide_src/source/libraries/validation.rst b/user_guide_src/source/libraries/validation.rst index 87cd520240bd..13f7f88c51df 100644 --- a/user_guide_src/source/libraries/validation.rst +++ b/user_guide_src/source/libraries/validation.rst @@ -411,6 +411,8 @@ data to be validated: Working with Validation *********************** +.. _validation-running: + Running Validation ================== @@ -762,6 +764,16 @@ right after the name of the field the error should belong to:: showError('username', 'my_single') ?> +************* +Form Requests +************* + +.. versionadded:: 4.8.0 + +For a higher-level approach that encapsulates validation rules, custom error +messages, and authorization logic in a dedicated class, see +:doc:`Form Requests `. + ********************* Creating Custom Rules ********************* diff --git a/utils/phpstan-baseline/argument.type.neon b/utils/phpstan-baseline/argument.type.neon index 3bb95bae65fb..f1f948876b16 100644 --- a/utils/phpstan-baseline/argument.type.neon +++ b/utils/phpstan-baseline/argument.type.neon @@ -1,4 +1,4 @@ -# total 82 errors +# total 78 errors parameters: ignoreErrors: @@ -27,26 +27,6 @@ parameters: count: 1 path: ../../system/Database/SQLite3/Builder.php - - - message: '#^Parameter \#2 \$to of method CodeIgniter\\Router\\RouteCollection\:\:add\(\) expects array\|\(Closure\(mixed \.\.\.\)\: \(CodeIgniter\\HTTP\\ResponseInterface\|string\|void\)\)\|string, Closure\(mixed\)\: CodeIgniter\\HTTP\\ResponseInterface given\.$#' - count: 1 - path: ../../tests/system/CodeIgniterTest.php - - - - message: '#^Parameter \#2 \$to of method CodeIgniter\\Router\\RouteCollection\:\:add\(\) expects array\|\(Closure\(mixed \.\.\.\)\: \(CodeIgniter\\HTTP\\ResponseInterface\|string\|void\)\)\|string, Closure\(mixed\)\: \(CodeIgniter\\HTTP\\DownloadResponse\|null\) given\.$#' - count: 1 - path: ../../tests/system/CodeIgniterTest.php - - - - message: '#^Parameter \#2 \$to of method CodeIgniter\\Router\\RouteCollection\:\:add\(\) expects array\|\(Closure\(mixed \.\.\.\)\: \(CodeIgniter\\HTTP\\ResponseInterface\|string\|void\)\)\|string, Closure\(mixed\)\: non\-falsy\-string given\.$#' - count: 1 - path: ../../tests/system/CodeIgniterTest.php - - - - message: '#^Parameter \#2 \$to of method CodeIgniter\\Router\\RouteCollection\:\:add\(\) expects array\|\(Closure\(mixed \.\.\.\)\: \(CodeIgniter\\HTTP\\ResponseInterface\|string\|void\)\)\|string, Closure\(mixed\)\: void given\.$#' - count: 1 - path: ../../tests/system/CodeIgniterTest.php - - message: '#^Parameter \#2 \$mock of static method CodeIgniter\\Config\\BaseService\:\:injectMock\(\) expects object, null given\.$#' count: 4 diff --git a/utils/phpstan-baseline/loader.neon b/utils/phpstan-baseline/loader.neon index 222aa6ade6ec..78a665b0b346 100644 --- a/utils/phpstan-baseline/loader.neon +++ b/utils/phpstan-baseline/loader.neon @@ -1,4 +1,4 @@ -# total 2027 errors +# total 2042 errors includes: - argument.type.neon diff --git a/utils/phpstan-baseline/missingType.callable.neon b/utils/phpstan-baseline/missingType.callable.neon index 5d0d90d58d56..844b30e8d52b 100644 --- a/utils/phpstan-baseline/missingType.callable.neon +++ b/utils/phpstan-baseline/missingType.callable.neon @@ -1,7 +1,12 @@ -# total 12 errors +# total 31 errors parameters: ignoreErrors: + - + message: '#^Property CodeIgniter\\CodeIgniter\:\:\$controller type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/CodeIgniter.php + - message: '#^Class CodeIgniter\\Model has PHPDoc tag @method for method whenNot\(\) parameter \#2 \$callback with no signature specified for callable\.$#' count: 1 @@ -22,26 +27,116 @@ parameters: count: 1 path: ../../system/Model.php + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:add\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:cli\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:create\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:delete\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:getControllerName\(\) has parameter \$handler with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:get\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + - message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:group\(\) has parameter \$params with no signature specified for callable\.$#' count: 1 path: ../../system/Router/RouteCollection.php + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:head\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:match\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:options\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:patch\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:post\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + + - + message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:put\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollection.php + - message: '#^Method CodeIgniter\\Router\\RouteCollection\:\:set404Override\(\) has parameter \$callable with no signature specified for callable\.$#' count: 1 path: ../../system/Router/RouteCollection.php + - + message: '#^Method CodeIgniter\\Router\\RouteCollectionInterface\:\:add\(\) has parameter \$to with no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouteCollectionInterface.php + - message: '#^Method CodeIgniter\\Router\\RouteCollectionInterface\:\:set404Override\(\) has parameter \$callable with no signature specified for callable\.$#' count: 1 path: ../../system/Router/RouteCollectionInterface.php + - + message: '#^Method CodeIgniter\\Router\\Router\:\:controllerName\(\) return type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/Router.php + + - + message: '#^Method CodeIgniter\\Router\\Router\:\:handle\(\) return type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/Router.php + - message: '#^Method CodeIgniter\\Router\\Router\:\:setMatchedRoute\(\) has parameter \$handler with no signature specified for callable\.$#' count: 1 path: ../../system/Router/Router.php + - + message: '#^Property CodeIgniter\\Router\\Router\:\:\$controller type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/Router.php + + - + message: '#^Method CodeIgniter\\Router\\RouterInterface\:\:controllerName\(\) return type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouterInterface.php + + - + message: '#^Method CodeIgniter\\Router\\RouterInterface\:\:handle\(\) return type has no signature specified for Closure\.$#' + count: 1 + path: ../../system/Router/RouterInterface.php + - message: '#^Method CodeIgniter\\View\\Parser\:\:addPlugin\(\) has parameter \$callback with no signature specified for callable\.$#' count: 1 From 694768565bc4505f660deb37150614c7df70e993 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 6 Apr 2026 13:17:15 +0200 Subject: [PATCH 2/3] fix tests --- .../Routes/AutoRouteCollectorTest.php | 42 +++++++++++++++++++ .../Utilities/Routes/ControllerFinderTest.php | 6 ++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/tests/system/Commands/Utilities/Routes/AutoRouteCollectorTest.php b/tests/system/Commands/Utilities/Routes/AutoRouteCollectorTest.php index ef8e43282b92..84f2106ccc21 100644 --- a/tests/system/Commands/Utilities/Routes/AutoRouteCollectorTest.php +++ b/tests/system/Commands/Utilities/Routes/AutoRouteCollectorTest.php @@ -34,6 +34,48 @@ public function testGet(): void $routes = $collector->get(); $expected = [ + [ + 'auto', + 'formRequestController', + '', + '\Tests\Support\Controllers\FormRequestController::index', + ], + [ + 'auto', + 'formRequestController/index[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::index', + ], + [ + 'auto', + 'formRequestController/store[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::store', + ], + [ + 'auto', + 'formRequestController/update[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::update', + ], + [ + 'auto', + 'formRequestController/show[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::show', + ], + [ + 'auto', + 'formRequestController/search[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::search', + ], + [ + 'auto', + 'formRequestController/restricted[/...]', + '', + '\Tests\Support\Controllers\FormRequestController::restricted', + ], [ 'auto', 'hello', diff --git a/tests/system/Commands/Utilities/Routes/ControllerFinderTest.php b/tests/system/Commands/Utilities/Routes/ControllerFinderTest.php index 1f6ecce2ec3e..3fabbd9adcdd 100644 --- a/tests/system/Commands/Utilities/Routes/ControllerFinderTest.php +++ b/tests/system/Commands/Utilities/Routes/ControllerFinderTest.php @@ -15,6 +15,7 @@ use CodeIgniter\Test\CIUnitTestCase; use PHPUnit\Framework\Attributes\Group; +use Tests\Support\Controllers\FormRequestController; use Tests\Support\Controllers\Hello; /** @@ -30,7 +31,8 @@ public function testFind(): void $controllers = $finder->find(); - $this->assertCount(4, $controllers); - $this->assertSame(Hello::class, $controllers[0]); + $this->assertCount(5, $controllers); + $this->assertSame(FormRequestController::class, $controllers[0]); + $this->assertSame(Hello::class, $controllers[1]); } } From 0b455a909271949ea40ddfcabf9809430887c859 Mon Sep 17 00:00:00 2001 From: michalsn Date: Mon, 6 Apr 2026 19:02:14 +0200 Subject: [PATCH 3/3] apply code suggestions Co-authored-by: John Paul E. Balandan, CPA --- .../Generators/Views/formrequest.tpl.php | 2 +- system/HTTP/FormRequest.php | 25 ++++++------------- .../HTTP/Requests/UnauthorizedFormRequest.php | 2 +- tests/system/HTTP/FormRequestTest.php | 10 ++++---- .../source/incoming/form_requests.rst | 6 ++--- .../source/incoming/form_requests/005.php | 2 +- 6 files changed, 18 insertions(+), 29 deletions(-) diff --git a/system/Commands/Generators/Views/formrequest.tpl.php b/system/Commands/Generators/Views/formrequest.tpl.php index cdcbeae966d5..84ce807cb4cf 100644 --- a/system/Commands/Generators/Views/formrequest.tpl.php +++ b/system/Commands/Generators/Views/formrequest.tpl.php @@ -31,7 +31,7 @@ public function rules(): array // /** // * Determines if the current user is authorized to make this request. // */ - // public function authorize(): bool + // public function isAuthorized(): bool // { // return true; // } diff --git a/system/HTTP/FormRequest.php b/system/HTTP/FormRequest.php index cdaa98c30112..7e0968895075 100644 --- a/system/HTTP/FormRequest.php +++ b/system/HTTP/FormRequest.php @@ -13,7 +13,6 @@ namespace CodeIgniter\HTTP; -use CodeIgniter\Exceptions\RuntimeException; use ReflectionNamedType; use ReflectionParameter; @@ -36,22 +35,12 @@ abstract class FormRequest /** * When called by the framework, the current IncomingRequest is injected - * explicitly. When instantiated manually (e.g. in tests or commands), - * the constructor falls back to service('request'). - * - * @throws RuntimeException if used outside of an HTTP request context. + * explicitly. When instantiated manually (e.g. in tests), the constructor + * falls back to service('request'). */ final public function __construct(?IncomingRequest $request = null) { - $resolved = $request ?? service('request'); - - if (! $resolved instanceof IncomingRequest) { - throw new RuntimeException( - static::class . ' requires an IncomingRequest instance, got ' . $resolved::class . '.', - ); - } - - $this->request = $resolved; + $this->request = $request ?? service('request'); } /** @@ -88,12 +77,12 @@ public function messages(): array * * Override in subclasses to add authorization logic: * - * public function authorize(): bool + * public function isAuthorized(): bool * { * return auth()->user()->can('create-posts'); * } */ - public function authorize(): bool + public function isAuthorized(): bool { return true; } @@ -164,7 +153,7 @@ protected function failedValidation(array $errors): ResponseInterface } /** - * Called when the authorize() check returns false. Override to customize. + * Called when the isAuthorized() check returns false. Override to customize. */ protected function failedAuthorization(): ResponseInterface { @@ -261,7 +250,7 @@ protected function validationData(): array */ final public function resolveRequest(): ?ResponseInterface { - if (! $this->authorize()) { + if (! $this->isAuthorized()) { return $this->failedAuthorization(); } diff --git a/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php b/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php index 701b21d5de03..24dc8a6a092c 100644 --- a/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php +++ b/tests/_support/HTTP/Requests/UnauthorizedFormRequest.php @@ -25,7 +25,7 @@ public function rules(): array return ['title' => 'required']; } - public function authorize(): bool + public function isAuthorized(): bool { return false; } diff --git a/tests/system/HTTP/FormRequestTest.php b/tests/system/HTTP/FormRequestTest.php index 588472d504dc..b771c1a55a58 100644 --- a/tests/system/HTTP/FormRequestTest.php +++ b/tests/system/HTTP/FormRequestTest.php @@ -119,7 +119,7 @@ public function testDefaultAuthorizeReturnsTrue(): void { $formRequest = $this->makeFormRequest($this->makeRequest()); - $this->assertTrue($formRequest->authorize()); + $this->assertTrue($formRequest->isAuthorized()); } public function testValidatedReturnsEmptyArrayBeforeResolution(): void @@ -338,7 +338,7 @@ public function rules(): array return []; } - public function authorize(): bool + public function isAuthorized(): bool { return false; } @@ -364,7 +364,7 @@ public function rules(): array return ['title' => 'required']; } - public function authorize(): bool + public function isAuthorized(): bool { self::$order[] = 'authorize'; @@ -381,7 +381,7 @@ protected function prepareForValidation(array $data): array $formRequest->resolveRequest(); - // authorize() must fire before prepareForValidation(); validation never runs. + // isAuthorized() must fire before prepareForValidation(); validation never runs. $this->assertSame(['authorize'], $formRequest::$order); } @@ -443,7 +443,7 @@ public function rules(): array return []; } - public function authorize(): bool + public function isAuthorized(): bool { return false; } diff --git a/user_guide_src/source/incoming/form_requests.rst b/user_guide_src/source/incoming/form_requests.rst index 1bda12d8c88e..13243703629e 100644 --- a/user_guide_src/source/incoming/form_requests.rst +++ b/user_guide_src/source/incoming/form_requests.rst @@ -131,14 +131,14 @@ identical to the ``$errors`` argument of :ref:`saving-validation-rules-to-config Authorization ************* -Override ``authorize()`` to control whether the current user is allowed to make +Override ``isAuthorized()`` to control whether the current user is allowed to make this request. Return ``false`` to reject the request with a 403 Forbidden response before validation even runs. .. literalinclude:: form_requests/005.php :lines: 2- -.. note:: The ``authorize()`` check runs before ``prepareForValidation()`` and +.. note:: The ``isAuthorized()`` check runs before ``prepareForValidation()`` and before validation itself. An unauthorized request never reaches the validation stage. @@ -223,7 +223,7 @@ whose type extends ``FormRequest``: #. A new instance is created with the current request injected via the constructor. -#. ``authorize()`` is called. If it returns ``false``, ``failedAuthorization()`` +#. ``isAuthorized()`` is called. If it returns ``false``, ``failedAuthorization()`` is called, and its response is returned to the client. #. ``validationData()`` collects the data to validate. #. ``prepareForValidation()`` receives that data and may modify it before the diff --git a/user_guide_src/source/incoming/form_requests/005.php b/user_guide_src/source/incoming/form_requests/005.php index 8daff0464c7c..8b6cdd0d1ab9 100644 --- a/user_guide_src/source/incoming/form_requests/005.php +++ b/user_guide_src/source/incoming/form_requests/005.php @@ -14,7 +14,7 @@ public function rules(): array ]; } - public function authorize(): bool + public function isAuthorized(): bool { // Only authenticated users may submit posts. return auth()->loggedIn();