From 6df114532f27ad6584511fd97da152021e9f6bba Mon Sep 17 00:00:00 2001 From: Abdul Majid <43958759+abdulmajidcse@users.noreply.github.com> Date: Thu, 5 Nov 2026 20:03:54 +0600 Subject: [PATCH 1/2] fix: correct old input, validation rules, route lookups and error responses - Restore every old() field and read array input - Count characters in min, max and size, and reject impossible dates - Match JSON numbers in in and not_in, and reject trailing newlines - Reject whitespace-only values in required - Keep the dispatched route in currentRouteAction() - Use the dispatched method in currentRouteName() - Treat _method=ANY as an invalid method - Use the fallback in back() for a blank Referer - Send exception headers on web error pages - Use only HTTP error statuses from exception codes in JSON errors - Use relations' jsonSerialize() in JSON responses when an override calls toArray() - Prefer plain text when decrypting legacy ciphertext --- src/Phaseolies/Database/Entity/Model.php | 19 ++- .../Error/Handlers/JsonErrorHandler.php | 22 ++- src/Phaseolies/Helpers/helpers.php | 4 +- src/Phaseolies/Http/Request.php | 3 +- src/Phaseolies/Http/Response.php | 9 +- .../Http/Response/RedirectResponse.php | 2 +- .../Http/Support/ValidationRules.php | 57 +++++-- src/Phaseolies/Session/MessageBag.php | 20 +-- src/Phaseolies/Support/Encryption.php | 8 + src/Phaseolies/Support/Router.php | 13 +- .../Router/InteractsWithCurrentRouter.php | 21 ++- tests/Http/ResponseEdgeCasesTest.php | 149 ++++++++++++++++++ tests/Http/ValidationEdgeCasesTest.php | 94 +++++++++++ tests/Router/CurrentRouteTest.php | 112 +++++++++++++ tests/Session/MessageBagTest.php | 70 ++++++++ 15 files changed, 562 insertions(+), 41 deletions(-) create mode 100644 tests/Http/ResponseEdgeCasesTest.php create mode 100644 tests/Http/ValidationEdgeCasesTest.php create mode 100644 tests/Router/CurrentRouteTest.php create mode 100644 tests/Session/MessageBagTest.php diff --git a/src/Phaseolies/Database/Entity/Model.php b/src/Phaseolies/Database/Entity/Model.php index bc791de2..27d4c3ce 100644 --- a/src/Phaseolies/Database/Entity/Model.php +++ b/src/Phaseolies/Database/Entity/Model.php @@ -707,7 +707,7 @@ public function makeHidden(array $attributes): self } /** - * Number of jsonSerialize() calls in progress (nested relations included). + * Number of JSON serializations in progress (nested relations included). * * @var int */ @@ -719,18 +719,31 @@ public function makeHidden(array $attributes): self * @return array */ public function jsonSerialize(): array + { + return self::serializingJson(fn() => $this->toArray()); + } + + /** + * Run a callback that produces JSON, so loaded relations use their own jsonSerialize() + * + * This also covers a jsonSerialize() override that calls toArray() itself. + * + * @param callable $callback + * @return mixed + */ + public static function serializingJson(callable $callback): mixed { self::$jsonSerializationDepth++; try { - return $this->toArray(); + return $callback(); } finally { self::$jsonSerializationDepth--; } } /** - * Serialize a related model for toArray(): through jsonSerialize() while + * Serialize a related model for toArray(): through jsonSerialize() while producing JSON. * * @param Model $model * @return array diff --git a/src/Phaseolies/Error/Handlers/JsonErrorHandler.php b/src/Phaseolies/Error/Handlers/JsonErrorHandler.php index e140345c..e986c976 100644 --- a/src/Phaseolies/Error/Handlers/JsonErrorHandler.php +++ b/src/Phaseolies/Error/Handlers/JsonErrorHandler.php @@ -3,6 +3,7 @@ namespace Phaseolies\Error\Handlers; use Throwable; +use Phaseolies\Http\Exceptions\HttpException; use Phaseolies\Http\Exceptions\HttpResponseException; use Phaseolies\Error\JsonErrorRenderer; use Phaseolies\Error\Contracts\ErrorHandlerInterface; @@ -30,13 +31,30 @@ public function handle(Throwable $exception): void $statusCode = (int) $exception->getStatusCode() ?: 500; $response = $renderer->render($exception, $statusCode, $responseErrors); } else { - $statusCode = (int) $exception->getCode() ?: 500; - $response = $renderer->render($exception, $statusCode); + $response = $renderer->render($exception, $this->resolveStatusCode($exception)); } $response->prepare(request())->send(); } + /** + * The HTTP status for an exception. Any other exception's code can be any + * number, so it only counts when it is an HTTP error status. + * + * @param Throwable $exception + * @return int + */ + protected function resolveStatusCode(Throwable $exception): int + { + if ($exception instanceof HttpException) { + return $exception->getStatusCode(); + } + + $statusCode = (int) $exception->getCode(); + + return $statusCode >= 400 && $statusCode < 600 ? $statusCode : 500; + } + /** * Determines whether this handler supports the current request context. * diff --git a/src/Phaseolies/Helpers/helpers.php b/src/Phaseolies/Helpers/helpers.php index 50234bcb..e0d8922e 100644 --- a/src/Phaseolies/Helpers/helpers.php +++ b/src/Phaseolies/Helpers/helpers.php @@ -338,9 +338,9 @@ function bcrypt(string $plainText): string * Retrieves the old input value for a given key from the session. * * @param mixed $key - * @return string|null + * @return mixed */ - function old($key): ?string + function old($key): mixed { return MessageBag::old($key); } diff --git a/src/Phaseolies/Http/Request.php b/src/Phaseolies/Http/Request.php index dd56af04..b0724db6 100644 --- a/src/Phaseolies/Http/Request.php +++ b/src/Phaseolies/Http/Request.php @@ -100,7 +100,6 @@ class Request self::METHOD_PATCH, self::METHOD_PURGE, self::METHOD_TRACE, - self::METHOD_ANY, ]; /** @@ -437,7 +436,7 @@ public function isValidMethod(): bool { $method = strtoupper((string) $this->method); - return $method !== self::METHOD_ANY && in_array($method, self::VALID_HTTP_METHODS, true); + return in_array($method, self::VALID_HTTP_METHODS, true); } public function __toString(): string diff --git a/src/Phaseolies/Http/Response.php b/src/Phaseolies/Http/Response.php index ca0dcc78..97bc00f6 100644 --- a/src/Phaseolies/Http/Response.php +++ b/src/Phaseolies/Http/Response.php @@ -10,6 +10,7 @@ use Phaseolies\Http\Controllers\Controller; use Phaseolies\Error\JsonErrorRenderer; use Phaseolies\Database\Entity\Builder; +use Phaseolies\Database\Entity\Model; class Response implements HttpStatus { @@ -643,6 +644,12 @@ public static function dispatchHttpException(HttpException $exception): void http_response_code($statusCode); + foreach ($exception->getHeaders() as $name => $value) { + if (!headers_sent()) { + header($name . ': ' . implode(', ', (array) $value)); + } + } + self::renderErrorPage($statusCode, $message); } @@ -1633,7 +1640,7 @@ public static function toSerializable(mixed $content): mixed */ protected function encodeJsonContent(mixed $content, int $options = 0): string { - return json_encode(static::toSerializable($content), $options); + return Model::serializingJson(fn() => json_encode(static::toSerializable($content), $options)); } /** diff --git a/src/Phaseolies/Http/Response/RedirectResponse.php b/src/Phaseolies/Http/Response/RedirectResponse.php index 3391770b..5d09f51e 100644 --- a/src/Phaseolies/Http/Response/RedirectResponse.php +++ b/src/Phaseolies/Http/Response/RedirectResponse.php @@ -116,7 +116,7 @@ public function to(string $url, int $statusCode = 302, array $headers = [], ?boo public function back($status = 302, $headers = [], string|bool|null $fallback = false): RedirectResponse { $fallbackUrl = is_string($fallback) && $fallback !== '' ? $fallback : '/'; - $url = request()->headers->get('referer') ?? $fallbackUrl; + $url = request()->headers->get('referer') ?: $fallbackUrl; $this->setStatusCode($status)->withHeaders($headers); $this->setTargetUrl($url); diff --git a/src/Phaseolies/Http/Support/ValidationRules.php b/src/Phaseolies/Http/Support/ValidationRules.php index 41fc0c83..3b6ed1cf 100644 --- a/src/Phaseolies/Http/Support/ValidationRules.php +++ b/src/Phaseolies/Http/Support/ValidationRules.php @@ -515,6 +515,11 @@ protected function isInteger(array $input, string $fieldName): bool { $value = $input[$fieldName] ?? ''; + // filter_var() ignores surrounding whitespace, which is not part of an integer. + if (is_string($value) && $value !== trim($value)) { + return false; + } + return filter_var($value, FILTER_VALIDATE_INT) !== false; } @@ -588,7 +593,9 @@ protected function isDateValid(array $input, string $fieldName): bool { $date = $input[$fieldName] ?? ''; - return strtotime($date) !== false; + // strtotime() rolls impossible dates over (February 30 becomes March 2); + // date_parse() reports them as a warning. + return strtotime($date) !== false && date_parse($date)['warning_count'] === 0; } /** @@ -869,7 +876,7 @@ protected function isEmptyFieldRequired(array $input, string $fieldName): bool return empty($value); } - return $value === ''; + return is_string($value) && trim($value) === ''; } /** @@ -882,7 +889,7 @@ protected function isEmptyFieldRequired(array $input, string $fieldName): bool */ protected function isLessThanMin(array $input, string $fieldName, int $value): bool { - return strlen($input[$fieldName]) < $value; + return mb_strlen((string) $input[$fieldName]) < $value; } /** @@ -895,7 +902,7 @@ protected function isLessThanMin(array $input, string $fieldName, int $value): b */ protected function isMoreThanMax(array $input, string $fieldName, int $value): bool { - return strlen($input[$fieldName]) > $value; + return mb_strlen((string) $input[$fieldName]) > $value; } /** @@ -987,7 +994,7 @@ protected function _removeUnderscore(string $string): string */ protected function isAlpha(array $input, string $fieldName): bool { - return (bool) preg_match('/^[a-zA-Z]+$/', $input[$fieldName] ?? ''); + return (bool) preg_match('/^[a-zA-Z]+\z/', $input[$fieldName] ?? ''); } /** @@ -999,7 +1006,7 @@ protected function isAlpha(array $input, string $fieldName): bool */ protected function isAlphaNum(array $input, string $fieldName): bool { - return (bool) preg_match('/^[a-zA-Z0-9]+$/', $input[$fieldName] ?? ''); + return (bool) preg_match('/^[a-zA-Z0-9]+\z/', $input[$fieldName] ?? ''); } /** @@ -1011,7 +1018,7 @@ protected function isAlphaNum(array $input, string $fieldName): bool */ protected function isAlphaDash(array $input, string $fieldName): bool { - return (bool) preg_match('/^[a-zA-Z0-9_\-]+$/', $input[$fieldName] ?? ''); + return (bool) preg_match('/^[a-zA-Z0-9_\-]+\z/', $input[$fieldName] ?? ''); } /** @@ -1023,7 +1030,10 @@ protected function isAlphaDash(array $input, string $fieldName): bool */ protected function isNumeric(array $input, string $fieldName): bool { - return is_numeric($input[$fieldName] ?? ''); + $value = $input[$fieldName] ?? ''; + + // is_numeric() ignores surrounding whitespace, which is not part of a number. + return is_numeric($value) && (!is_string($value) || $value === trim($value)); } /** @@ -1133,6 +1143,19 @@ protected function isDifferent(array $input, string $fieldName, string $otherFie return ($input[$fieldName] ?? '') !== ($input[$otherField] ?? ''); } + /** + * The form of a value to compare with a rule's list, which holds only strings. + * + * A JSON body sends numbers as numbers, and 1 must match "1". + * + * @param mixed $value + * @return mixed + */ + protected function listValue(mixed $value): mixed + { + return is_int($value) || is_float($value) ? (string) $value : $value; + } + /** * Check if the field value is one of the allowed values * @@ -1145,7 +1168,7 @@ protected function isInList(array $input, string $fieldName, string $ruleValue): { $list = array_map('trim', explode(',', $ruleValue)); - return in_array($input[$fieldName] ?? '', $list, true); + return in_array($this->listValue($input[$fieldName] ?? ''), $list, true); } /** @@ -1160,7 +1183,7 @@ protected function isNotInList(array $input, string $fieldName, string $ruleValu { $list = array_map('trim', explode(',', $ruleValue)); - return !in_array($input[$fieldName] ?? '', $list, true); + return !in_array($this->listValue($input[$fieldName] ?? ''), $list, true); } /** @@ -1189,7 +1212,7 @@ protected function isPhone(array $input, string $fieldName): bool { $value = $input[$fieldName] ?? ''; - return (bool) preg_match('/^\+?[0-9\s\-\(\)]{7,20}$/', $value); + return (bool) preg_match('/^\+?[0-9 \-\(\)]{7,20}\z/', $value); } /** @@ -1204,7 +1227,7 @@ protected function isExactDigits(array $input, string $fieldName, int $digits): { $value = (string)($input[$fieldName] ?? ''); - return (bool) preg_match('/^\d{' . $digits . '}$/', $value); + return (bool) preg_match('/^\d{' . $digits . '}\z/', $value); } /** @@ -1219,7 +1242,7 @@ protected function isMinDigits(array $input, string $fieldName, int $min): bool { $value = (string)($input[$fieldName] ?? ''); - return (bool) preg_match('/^\d+$/', $value) && strlen($value) >= $min; + return (bool) preg_match('/^\d+\z/', $value) && strlen($value) >= $min; } /** @@ -1234,7 +1257,7 @@ protected function isMaxDigits(array $input, string $fieldName, int $max): bool { $value = (string)($input[$fieldName] ?? ''); - return (bool) preg_match('/^\d+$/', $value) && strlen($value) <= $max; + return (bool) preg_match('/^\d+\z/', $value) && strlen($value) <= $max; } /** @@ -1254,7 +1277,7 @@ protected function isExactSize(array $input, string $fieldName, int $size): bool return count($value) === $size; } - return strlen((string)$value) === $size; + return mb_strlen((string) $value) === $size; } /** @@ -1295,7 +1318,7 @@ protected function isUuid(array $input, string $fieldName): bool $value = $input[$fieldName] ?? ''; return (bool) preg_match( - '/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i', + '/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}\z/i', $value ); } @@ -1360,7 +1383,7 @@ protected function isSlug(array $input, string $fieldName): bool { $value = $input[$fieldName] ?? ''; - return (bool) preg_match('/^[a-z0-9]+(?:-[a-z0-9]+)*$/', $value); + return (bool) preg_match('/^[a-z0-9]+(?:-[a-z0-9]+)*\z/', $value); } /** diff --git a/src/Phaseolies/Session/MessageBag.php b/src/Phaseolies/Session/MessageBag.php index 82693f74..8072f6e0 100644 --- a/src/Phaseolies/Session/MessageBag.php +++ b/src/Phaseolies/Session/MessageBag.php @@ -35,22 +35,24 @@ public static function flashInput(): void } /** - * Retrieves the old input data that was previously stored in the session.. + * Retrieves the old input data that was previously stored in the session. + * + * The input stays readable for the rest of this request and is not kept for the next one. * * @param string|null $key - * @return string|null + * @return mixed */ - public static function old(?string $key = null): ?string + public static function old(?string $key = null): mixed { - $input = session('input') ?? null; + $session = session(); - if ($key) { - $data = session('input'); - session()->forget('input'); - return $data[$key] ?? null; + if ($session->has('input')) { + $session->putPeek('input', $session->get('input')); } - return $input; + $input = $session->getPeek('input'); + + return $key ? ($input[$key] ?? null) : $input; } /** diff --git a/src/Phaseolies/Support/Encryption.php b/src/Phaseolies/Support/Encryption.php index f9ad9cd8..0708b940 100644 --- a/src/Phaseolies/Support/Encryption.php +++ b/src/Phaseolies/Support/Encryption.php @@ -165,6 +165,14 @@ private function decryptLegacy(string $encryptedData): string|false } } + // Without a MAC, the wrong key can still give valid padding. Prefer text + // without control characters, which a wrong key rarely produces. + foreach ($candidates as $candidate) { + if (preg_match('//u', $candidate) === 1 && preg_match('/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/', $candidate) !== 1) { + return $candidate; + } + } + foreach ($candidates as $candidate) { if (preg_match('//u', $candidate) === 1) { return $candidate; diff --git a/src/Phaseolies/Support/Router.php b/src/Phaseolies/Support/Router.php index 4ddcdb52..fe16f8a7 100644 --- a/src/Phaseolies/Support/Router.php +++ b/src/Phaseolies/Support/Router.php @@ -62,6 +62,13 @@ class Router */ public static array $namedRoutes = []; + /** + * The HTTP method each named route was registered for. + * + * @var array + */ + public static array $namedRouteMethods = []; + /** * Matches a route parameter, `{id}` or `{id:constraint}`. The constraint may * use braces for quantifiers (`{id:\d{4}}`) but cannot nest them any deeper. @@ -214,6 +221,7 @@ public function cacheRoutes(): void $cacheData = [ 'routes' => $this->getCacheableRoutes(), 'namedRoutes' => self::$namedRoutes, + 'namedRouteMethods' => self::$namedRouteMethods, 'routeMiddlewares' => self::$routeMiddlewares, 'timestamp' => time() ]; @@ -321,6 +329,7 @@ public function loadCachedRoutes(): bool self::$routes = $cached['routes'] ?? []; self::$namedRoutes = $cached['namedRoutes'] ?? []; + self::$namedRouteMethods = $cached['namedRouteMethods'] ?? []; self::$routeMiddlewares = $cached['routeMiddlewares'] ?? []; static::$cacheLoaded = true; @@ -551,6 +560,7 @@ protected function addRouteNameToAttributesRouting(string $method, string $path, if ($name) { self::$namedRoutes[$name] = $this->currentRoutePath; + self::$namedRouteMethods[$name] = $method; } return $this; @@ -801,6 +811,7 @@ public function name(string $name): self { if ($this->currentRoutePath) { self::$namedRoutes[$name] = $this->currentRoutePath; + self::$namedRouteMethods[$name] = $this->getCurrentRequestMethod(); } return $this; @@ -1900,7 +1911,7 @@ private function getResolutionResponse($request, $result): Response if (Response::shouldBeJson($result)) { $request->setRequestFormat('json'); $response->headers->set('Content-Type', 'application/json'); - $response->setBody(json_encode(Response::toSerializable($result), JSON_THROW_ON_ERROR)); + $response->setBody(Model::serializingJson(fn() => json_encode(Response::toSerializable($result), JSON_THROW_ON_ERROR))); } else { $response->setBody($result ?? ''); } diff --git a/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php b/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php index 6880e05a..ff8a746a 100644 --- a/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php +++ b/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php @@ -73,9 +73,22 @@ public function currentRouteName(): ?string return null; } - $name = array_search($match['route'], self::$namedRoutes, true); + $fallback = null; - return $name === false ? null : $name; + // Several methods can share a path, each with its own name. + foreach (self::$namedRoutes as $name => $path) { + if ($path !== $match['route']) { + continue; + } + + if ((self::$namedRouteMethods[$name] ?? null) === $match['method']) { + return $name; + } + + $fallback ??= $name; + } + + return $fallback; } /** @@ -85,7 +98,9 @@ public function currentRouteName(): ?string */ public function currentRouteAction(): string|array|null { - $callback = $this->getCallback(request()); + // Not getCallback(): that matches again and would forget the dispatched + // route, which on a cached app may only exist in the uncached tables. + $callback = $this->routeFor(request())['callback'] ?? null; if (is_array($callback)) { return $callback[0] . '@' . $callback[1]; diff --git a/tests/Http/ResponseEdgeCasesTest.php b/tests/Http/ResponseEdgeCasesTest.php new file mode 100644 index 00000000..61668793 --- /dev/null +++ b/tests/Http/ResponseEdgeCasesTest.php @@ -0,0 +1,149 @@ + 'public']; + } + + public function toArray(): array + { + return ['secret' => 'hidden']; + } +} + +class OverridingUser extends Model +{ + public function __construct() + { + } + + public function jsonSerialize(): array + { + $data = $this->toArray(); + unset($data['password']); + + return $data; + } +} + +class ResponseEdgeCasesTest extends TestCase +{ + private MockContainer $container; + + protected function setUp(): void + { + $this->container = new MockContainer(); + Container::setInstance($this->container); + $this->container->bind('str', StringService::class); + } + + protected function tearDown(): void + { + Container::forgetInstance(); + } + + private function request(array $server = [], array $post = []): Request + { + $_GET = $_COOKIE = $_FILES = []; + $_POST = $post; + $_SERVER = $server + ['REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/', 'HTTP_HOST' => 'example.com', 'REMOTE_ADDR' => '127.0.0.1', 'SCRIPT_NAME' => '/index.php', 'SERVER_NAME' => 'example.com', 'SERVER_PORT' => '80']; + + $request = Request::createFromGlobals(); + $this->container->bind('request', fn() => $request); + + return $request; + } + + public function testBackUsesTheFallbackForABlankReferer(): void + { + $this->request(['HTTP_REFERER' => '']); + + $response = (new RedirectResponse('/'))->back(302, [], '/home'); + + $this->assertSame('/home', $response->headers->get('Location')); + } + + public function testBackStillUsesTheReferer(): void + { + $this->request(['HTTP_REFERER' => 'https://example.com/form']); + + $response = (new RedirectResponse('/'))->back(302, [], '/home'); + + $this->assertSame('https://example.com/form', $response->headers->get('Location')); + } + + public function testAnyIsNotAValidRequestMethod(): void + { + $request = $this->request(['REQUEST_METHOD' => 'POST'], ['_method' => 'ANY']); + $this->assertNotNull($request->getMalformedRequestError()); + + $request = $this->request(['REQUEST_METHOD' => 'ANY']); + $this->assertNotNull($request->getMalformedRequestError()); + } + + public static function statusProvider(): array + { + return [ + 'small code' => [new RuntimeException('x', 2), 500], + 'no code' => [new RuntimeException('x'), 500], + 'database driver code' => [new RuntimeException('x', 1045), 500], + 'client error code' => [new RuntimeException('x', 422), 422], + 'server error code' => [new RuntimeException('x', 503), 503], + 'http exception' => [new HttpException(404, 'x'), 404], + ]; + } + + #[DataProvider('statusProvider')] + public function testJsonErrorHandlerOnlyUsesHttpStatuses(\Throwable $exception, int $expected): void + { + $method = new ReflectionMethod(JsonErrorHandler::class, 'resolveStatusCode'); + + $this->assertSame($expected, $method->invoke(new JsonErrorHandler(), $exception)); + } + + public function testJsonResponseUsesRelationOverrideWhenTheParentCallsToArray(): void + { + $user = new OverridingUser(); + + foreach ((new \ReflectionClass(Model::class))->getProperties() as $property) { + if (!$property->isStatic() && !$property->isInitialized($user) && $property->getType()?->allowsNull()) { + $property->setValue($user, null); + } + } + + (new ReflectionProperty(Model::class, 'relations'))->setValue($user, [ + 'posts' => new Collection(HiddenFieldsPost::class, [new HiddenFieldsPost()]), + 'post' => new HiddenFieldsPost(), + ]); + + $json = (new ReflectionMethod(Response::class, 'encodeJsonContent'))->invoke(new Response(), $user); + + $this->assertSame(['posts' => [['only' => 'public']], 'post' => ['only' => 'public']], json_decode($json, true)); + } +} diff --git a/tests/Http/ValidationEdgeCasesTest.php b/tests/Http/ValidationEdgeCasesTest.php new file mode 100644 index 00000000..b75ab42e --- /dev/null +++ b/tests/Http/ValidationEdgeCasesTest.php @@ -0,0 +1,94 @@ +bind('str', StringService::class); + $container->bind('translator', fn() => new class extends Translator { + public function __construct() + { + } + + public function get($key, $replace = [], $locale = null): mixed + { + return $key; + } + + public function trans($key, $replace = [], $locale = null): string + { + return $key; + } + }); + + $_SERVER = ['REQUEST_METHOD' => 'POST', 'REQUEST_URI' => '/', 'HTTP_HOST' => 'example.com', 'REMOTE_ADDR' => '127.0.0.1', 'SCRIPT_NAME' => '/index.php', 'SERVER_NAME' => 'example.com', 'SERVER_PORT' => '80']; + $_GET = $_POST = $_COOKIE = $_FILES = []; + + $request = Request::createFromGlobals(); + $container->bind('request', fn() => $request); + } + + protected function tearDown(): void + { + Container::forgetInstance(); + } + + public static function ruleProvider(): array + { + return [ + 'min counts characters' => ['min:3', 'আম', false], + 'min accepts enough characters' => ['min:3', 'আমি', true], + 'max counts characters' => ['max:3', 'আমি', true], + 'max rejects too many characters' => ['max:3', 'abcd', false], + 'size counts characters' => ['size:3', 'আমি', true], + 'date rejects February 30' => ['date', '2026-02-30', false], + 'date accepts a real date' => ['date', '2026-02-28', true], + 'date accepts a date and time' => ['date', '2026-02-28 10:30:00', true], + 'date accepts a relative date' => ['date', 'next monday', true], + 'in accepts a JSON number' => ['in:1,2', 1, true], + 'in rejects another JSON number' => ['in:1,2', 3, false], + 'in accepts a string' => ['in:1,2', '1', true], + 'not_in rejects a JSON number' => ['not_in:1,2', 1, false], + 'not_in accepts another JSON number' => ['not_in:1,2', 3, true], + 'alpha rejects a trailing newline' => ['alpha', "abc\n", false], + 'alpha_num rejects a trailing newline' => ['alpha_num', "abc1\n", false], + 'alpha_dash rejects a trailing newline' => ['alpha_dash', "a-b\n", false], + 'slug rejects a trailing newline' => ['slug', "a-b\n", false], + 'digits rejects a trailing newline' => ['digits:3', "123\n", false], + 'min_digits rejects a trailing newline' => ['min_digits:2', "123\n", false], + 'max_digits rejects a trailing newline' => ['max_digits:5', "123\n", false], + 'phone rejects a trailing newline' => ['phone', "+8801711111111\n", false], + 'uuid rejects a trailing newline' => ['uuid', "550e8400-e29b-41d4-a716-446655440000\n", false], + 'int rejects a trailing newline' => ['int', "12\n", false], + 'int accepts a number' => ['int', '12', true], + 'int accepts a JSON number' => ['int', 12, true], + 'numeric rejects a trailing newline' => ['numeric', "12\n", false], + 'numeric accepts a JSON number' => ['numeric', 1.5, true], + 'alpha still accepts letters' => ['alpha', 'abc', true], + 'required rejects whitespace' => ['required', " \t\n", false], + 'required accepts zero' => ['required', '0', true], + 'required accepts false' => ['required', false, true], + ]; + } + + #[DataProvider('ruleProvider')] + public function testRule(string $rule, mixed $value, bool $passes): void + { + $this->assertSame($passes, (new Sanitizer(['field' => $value], ['field' => $rule]))->validate()); + } +} diff --git a/tests/Router/CurrentRouteTest.php b/tests/Router/CurrentRouteTest.php new file mode 100644 index 00000000..ac6e5d7e --- /dev/null +++ b/tests/Router/CurrentRouteTest.php @@ -0,0 +1,112 @@ +routeFile) { + ($this->routeFile)($this); + } + } + + public function reloadOriginalClosure(Request $request): mixed + { + return $this->getOriginalClosure($request); + } +} + +class CurrentRouteTest extends TestCase +{ + private MockContainer $container; + private CachedAppRouter $router; + + protected function setUp(): void + { + $this->container = new MockContainer(); + Container::setInstance($this->container); + $this->container->bind('str', StringService::class); + $this->resetRoutes(); + $this->router = new CachedAppRouter(new Gateway()); + } + + protected function tearDown(): void + { + $this->resetRoutes(); + Container::forgetInstance(); + } + + private function resetRoutes(): void + { + Router::$namedRoutes = []; + Router::$namedRouteMethods = []; + + (new \ReflectionProperty(Router::class, 'routes'))->setValue(null, []); + (new \ReflectionProperty(Router::class, 'routeMiddlewares'))->setValue(null, array_fill_keys( + ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'HEAD', 'ANY'], + [] + )); + } + + private function request(string $method, string $uri): Request + { + $_GET = $_POST = $_COOKIE = $_FILES = []; + $_SERVER = ['REQUEST_METHOD' => $method, 'REQUEST_URI' => $uri, 'HTTP_HOST' => 'example.com', 'REMOTE_ADDR' => '127.0.0.1', 'SCRIPT_NAME' => '/index.php', 'SERVER_NAME' => 'example.com', 'SERVER_PORT' => '80']; + + $request = Request::createFromGlobals(); + $this->container->bind('request', fn() => $request); + + return $request; + } + + public function testRouteNameFollowsTheDispatchedMethod(): void + { + $this->router->get('/posts', fn() => 'index')->name('posts.index'); + $this->router->post('/posts', fn() => 'store')->name('posts.store'); + + $this->router->getCallback($this->request('POST', '/posts')); + $this->assertSame('posts.store', $this->router->currentRouteName()); + + $this->router->getCallback($this->request('GET', '/posts')); + $this->assertSame('posts.index', $this->router->currentRouteName()); + } + + public function testRouteNameIsNullForAnUnnamedRoute(): void + { + $this->router->get('/posts', fn() => 'index'); + + $this->router->getCallback($this->request('GET', '/posts')); + + $this->assertNull($this->router->currentRouteName()); + } + + public function testCurrentRouteActionKeepsTheDispatchedRouteOfACachedApp(): void + { + $this->router->routeFile = fn(Router $router) => $router->get('/closure', fn() => 'x') + ->name('closure.route') + ->middleware('auth'); + + $request = $this->request('GET', '/closure'); + + $this->assertFalse($this->router->getCallback($request)); + $this->assertNotFalse($this->router->reloadOriginalClosure($request)); + + $this->assertSame('Closure', $this->router->currentRouteAction()); + $this->assertSame('closure.route', $this->router->currentRouteName()); + $this->assertSame(['auth'], $this->router->getCurrentRouteMiddleware($request)); + } +} diff --git a/tests/Session/MessageBagTest.php b/tests/Session/MessageBagTest.php new file mode 100644 index 00000000..4a3d682f --- /dev/null +++ b/tests/Session/MessageBagTest.php @@ -0,0 +1,70 @@ +session = new Session(new ArrayStore(), ['cookie' => 't', 'lifetime' => 120, 'stateless' => [], 'lottery' => [0, 100]]); + + $container = new MockContainer(); + Container::setInstance($container); + $container->bind('session', fn() => $this->session); + } + + protected function tearDown(): void + { + Container::forgetInstance(); + } + + public function testEveryFieldCanBeRestored(): void + { + $this->session->put('input', ['name' => 'Ada', 'email' => 'ada@example.com']); + + $this->assertSame('Ada', old('name')); + $this->assertSame('ada@example.com', old('email')); + $this->assertSame('Ada', old('name')); + } + + public function testArrayFieldsAndAllInputCanBeRead(): void + { + $this->session->put('input', ['tags' => ['a', 'b']]); + + $this->assertSame(['a', 'b'], old('tags')); + $this->assertSame(['tags' => ['a', 'b']], MessageBag::old()); + } + + public function testInputIsNotKeptForTheNextRequest(): void + { + $this->session->put('input', ['name' => 'Ada']); + + old('name'); + + $this->assertFalse($this->session->has('input')); + } + + public function testMissingFieldIsNull(): void + { + $this->assertNull(old('name')); + } + + public function testInputFlashedInThisRequestIsPreferred(): void + { + $this->session->putPeek('input', ['name' => 'Previous']); + $this->session->put('input', ['name' => 'Current']); + + $this->assertSame('Current', old('name')); + } +} From 8c6f8cda993696b5b6de1a92006bc2653145494e Mon Sep 17 00:00:00 2001 From: Abdul Majid <43958759+abdulmajidcse@users.noreply.github.com> Date: Thu, 5 Nov 2026 22:34:44 +0600 Subject: [PATCH 2/2] fix: address review feedback on route names and JSON responses - currentRouteName() no longer returns another method's name; names keep their path per method, including in the route cache - where() only moves the name of the route it constrains - StreamedJsonResponse uses a relation's jsonSerialize() when encoding models - JsonErrorHandler keeps an HttpException's headers --- .../Error/Handlers/JsonErrorHandler.php | 25 +- .../Response/Stream/StreamedJsonResponse.php | 4 +- src/Phaseolies/Support/Router.php | 37 ++- .../Router/InteractsWithCurrentRouter.php | 17 +- tests/Http/ResponseEdgeCasesTest.php | 97 +++++++- tests/Router/CurrentRouteTest.php | 220 ++++++++++++++++++ 6 files changed, 375 insertions(+), 25 deletions(-) diff --git a/src/Phaseolies/Error/Handlers/JsonErrorHandler.php b/src/Phaseolies/Error/Handlers/JsonErrorHandler.php index e986c976..b7adcb8b 100644 --- a/src/Phaseolies/Error/Handlers/JsonErrorHandler.php +++ b/src/Phaseolies/Error/Handlers/JsonErrorHandler.php @@ -5,6 +5,7 @@ use Throwable; use Phaseolies\Http\Exceptions\HttpException; use Phaseolies\Http\Exceptions\HttpResponseException; +use Phaseolies\Http\Response; use Phaseolies\Error\JsonErrorRenderer; use Phaseolies\Error\Contracts\ErrorHandlerInterface; @@ -24,17 +25,33 @@ public function handle(Throwable $exception): void return; } + $this->buildResponse($exception)->prepare(request())->send(); + } + + /** + * Build the JSON error response for an exception. + * + * @param Throwable $exception + * @return Response + */ + protected function buildResponse(Throwable $exception): Response + { $renderer = new JsonErrorRenderer(); if ($exception instanceof HttpResponseException) { $responseErrors = $exception->getValidationErrors(); $statusCode = (int) $exception->getStatusCode() ?: 500; - $response = $renderer->render($exception, $statusCode, $responseErrors); - } else { - $response = $renderer->render($exception, $this->resolveStatusCode($exception)); + + return $renderer->render($exception, $statusCode, $responseErrors); + } + + $response = $renderer->render($exception, $this->resolveStatusCode($exception)); + + if ($exception instanceof HttpException) { + $response->withHeaders($exception->getHeaders()); } - $response->prepare(request())->send(); + return $response; } /** diff --git a/src/Phaseolies/Http/Response/Stream/StreamedJsonResponse.php b/src/Phaseolies/Http/Response/Stream/StreamedJsonResponse.php index f4e7897c..2dd6c10f 100644 --- a/src/Phaseolies/Http/Response/Stream/StreamedJsonResponse.php +++ b/src/Phaseolies/Http/Response/Stream/StreamedJsonResponse.php @@ -11,6 +11,7 @@ namespace Phaseolies\Http\Response\Stream; +use Phaseolies\Database\Entity\Model; use Phaseolies\Http\Response\JsonResponse; use Phaseolies\Http\Response\Stream\StreamedResponse; @@ -93,7 +94,8 @@ private function streamData(mixed $data, int $jsonEncodingOptions, int $keyEncod return; } - echo json_encode($data ?? new \stdClass(), $jsonEncodingOptions); + // Only the encoding: a generator's own code still sees plain toArray(). + echo Model::serializingJson(fn() => json_encode($data ?? new \stdClass(), $jsonEncodingOptions)); } private function streamArray(array $data, int $jsonEncodingOptions, int $keyEncodingOptions): void diff --git a/src/Phaseolies/Support/Router.php b/src/Phaseolies/Support/Router.php index fe16f8a7..41e9e4fc 100644 --- a/src/Phaseolies/Support/Router.php +++ b/src/Phaseolies/Support/Router.php @@ -63,9 +63,10 @@ class Router public static array $namedRoutes = []; /** - * The HTTP method each named route was registered for. + * The path each named route has per HTTP method. $namedRoutes keeps the one + * path a name generates URLs for; a shared name can differ per method here. * - * @var array + * @var array> */ public static array $namedRouteMethods = []; @@ -329,7 +330,8 @@ public function loadCachedRoutes(): bool self::$routes = $cached['routes'] ?? []; self::$namedRoutes = $cached['namedRoutes'] ?? []; - self::$namedRouteMethods = $cached['namedRouteMethods'] ?? []; + // A cache from before names kept their paths per method resolves by path alone. + self::$namedRouteMethods = array_filter($cached['namedRouteMethods'] ?? [], 'is_array'); self::$routeMiddlewares = $cached['routeMiddlewares'] ?? []; static::$cacheLoaded = true; @@ -559,13 +561,25 @@ protected function addRouteNameToAttributesRouting(string $method, string $path, $this->addRoute($method, $path, $callback, $domain); if ($name) { - self::$namedRoutes[$name] = $this->currentRoutePath; - self::$namedRouteMethods[$name] = $method; + $this->recordRouteName($name, $method); } return $this; } + /** + * Point a name at the last registered route. + * + * @param string $name + * @param string $method + * @return void + */ + protected function recordRouteName(string $name, string $method): void + { + self::$namedRoutes[$name] = $this->currentRoutePath; + self::$namedRouteMethods[$name][$method] = $this->currentRoutePath; + } + /** * Load both file-based and attribute-based routes * @@ -810,8 +824,7 @@ public function failFastOnBadRouteDefinition(mixed $callback): void public function name(string $name): self { if ($this->currentRoutePath) { - self::$namedRoutes[$name] = $this->currentRoutePath; - self::$namedRouteMethods[$name] = $this->getCurrentRequestMethod(); + $this->recordRouteName($name, $this->getCurrentRequestMethod()); } return $this; @@ -1102,6 +1115,16 @@ protected function rekeyCurrentRoute(string $newPath): void } foreach (self::$namedRoutes as $name => $path) { + $recorded = isset(self::$namedRouteMethods[$name]); + + if ($recorded && (self::$namedRouteMethods[$name][$method] ?? null) !== $old) { + continue; + } + + if ($recorded) { + self::$namedRouteMethods[$name][$method] = $newPath; + } + if ($path === $old) { self::$namedRoutes[$name] = $newPath; } diff --git a/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php b/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php index ff8a746a..fdc35455 100644 --- a/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php +++ b/src/Phaseolies/Support/Router/InteractsWithCurrentRouter.php @@ -73,22 +73,19 @@ public function currentRouteName(): ?string return null; } - $fallback = null; - - // Several methods can share a path, each with its own name. + // Several methods can share a path, each with its own name. A name with + // no recorded methods comes from a route cache written before they were. foreach (self::$namedRoutes as $name => $path) { - if ($path !== $match['route']) { - continue; - } + $routePath = isset(self::$namedRouteMethods[$name]) + ? self::$namedRouteMethods[$name][$match['method']] ?? null + : $path; - if ((self::$namedRouteMethods[$name] ?? null) === $match['method']) { + if ($routePath === $match['route']) { return $name; } - - $fallback ??= $name; } - return $fallback; + return null; } /** diff --git a/tests/Http/ResponseEdgeCasesTest.php b/tests/Http/ResponseEdgeCasesTest.php index 61668793..839ab7d3 100644 --- a/tests/Http/ResponseEdgeCasesTest.php +++ b/tests/Http/ResponseEdgeCasesTest.php @@ -8,11 +8,15 @@ use Phaseolies\DI\Container; use Phaseolies\Error\Handlers\JsonErrorHandler; use Phaseolies\Http\Exceptions\HttpException; +use Phaseolies\Http\Exceptions\TooManyRequestsHttpException; use Phaseolies\Http\Request; use Phaseolies\Http\Response; use Phaseolies\Http\Response\RedirectResponse; +use Phaseolies\Http\Response\Stream\StreamedJsonResponse; use Phaseolies\Support\Collection; use Phaseolies\Support\StringService; +use Phaseolies\Translation\FileLoader; +use Phaseolies\Translation\Translator; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use ReflectionMethod; @@ -127,7 +131,96 @@ public function testJsonErrorHandlerOnlyUsesHttpStatuses(\Throwable $exception, $this->assertSame($expected, $method->invoke(new JsonErrorHandler(), $exception)); } + public function testJsonErrorHandlerKeepsTheHttpExceptionHeaders(): void + { + $lang = sys_get_temp_dir() . '/doppar-json-error-lang-' . bin2hex(random_bytes(5)); + mkdir($lang . '/en', 0755, true); + file_put_contents($lang . '/en/validation.php', " 'x', 'rate_limit' => ['message' => 'x'], 'unauthorized' => ['message' => 'x']];"); + $this->container->bind('translator', fn() => new Translator(new FileLoader($lang), 'en')); + + + $response = (new ReflectionMethod(JsonErrorHandler::class, 'buildResponse')) + ->invoke(new JsonErrorHandler(), new TooManyRequestsHttpException(60, 'Slow down')); + + unlink($lang . '/en/validation.php'); + rmdir($lang . '/en'); + rmdir($lang); + + $this->assertSame(429, $response->getStatusCode()); + $this->assertSame('60', $response->headers->get('Retry-After')); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + } + public function testJsonResponseUsesRelationOverrideWhenTheParentCallsToArray(): void + { + $json = (new ReflectionMethod(Response::class, 'encodeJsonContent'))->invoke(new Response(), $this->userWithRelations()); + + $this->assertSame(['posts' => [['only' => 'public']], 'post' => ['only' => 'public']], json_decode($json, true)); + } + + /** + * @return array, callable(array): mixed}> + */ + public static function streamedDataProvider(): array + { + return [ + 'model in an array' => [fn(OverridingUser $user) => ['user' => $user], fn(array $json) => $json['user']], + 'model in a generator' => [ + function (OverridingUser $user) { + yield $user; + }, + fn(array $json) => $json[0], + ], + ]; + } + + #[DataProvider('streamedDataProvider')] + public function testStreamedJsonResponseUsesRelationOverrideWhenTheParentCallsToArray(callable $data, callable $user): void + { + ob_start(); + + try { + (new StreamedJsonResponse($data($this->userWithRelations())))->sendContent(); + } finally { + $json = ob_get_clean(); + } + + $this->assertSame( + ['posts' => [['only' => 'public']], 'post' => ['only' => 'public']], + $user(json_decode($json, true)) + ); + } + + public function testStreamedJsonResponseGeneratorSeesPlainToArray(): void + { + $user = $this->userWithRelations(); + $seen = null; + + $data = (function () use ($user, &$seen) { + $seen = $user->toArray()['post']; + + yield $user; + })(); + + ob_start(); + (new StreamedJsonResponse($data))->sendContent(); + ob_end_clean(); + + $this->assertSame(['secret' => 'hidden'], $seen); + } + + public function testStreamedJsonResponseLeavesToArrayAloneAfterSending(): void + { + $user = $this->userWithRelations(); + + ob_start(); + (new StreamedJsonResponse(['user' => $user]))->sendContent(); + ob_end_clean(); + + $this->assertSame(['secret' => 'hidden'], $user->toArray()['post']); + } + + private function userWithRelations(): OverridingUser { $user = new OverridingUser(); @@ -142,8 +235,6 @@ public function testJsonResponseUsesRelationOverrideWhenTheParentCallsToArray(): 'post' => new HiddenFieldsPost(), ]); - $json = (new ReflectionMethod(Response::class, 'encodeJsonContent'))->invoke(new Response(), $user); - - $this->assertSame(['posts' => [['only' => 'public']], 'post' => ['only' => 'public']], json_decode($json, true)); + return $user; } } diff --git a/tests/Router/CurrentRouteTest.php b/tests/Router/CurrentRouteTest.php index ac6e5d7e..4f916b68 100644 --- a/tests/Router/CurrentRouteTest.php +++ b/tests/Router/CurrentRouteTest.php @@ -8,7 +8,9 @@ use Phaseolies\DI\Container; use Phaseolies\Http\Request; use Phaseolies\Support\Router; +use Phaseolies\Support\Router\Attributes\Route as RouteAttribute; use Phaseolies\Support\StringService; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Tests\Support\Gateway; use Tests\Support\MockContainer; @@ -30,6 +32,30 @@ public function reloadOriginalClosure(Request $request): mixed } } +/** + * Redeclaring the statics gives this router a route cache of its own. + */ +final class RouteCacheRouter extends Router +{ + protected static string $cachePath; + + protected static bool $cacheLoaded = false; + + public static function pointTo(string $path): void + { + static::$cachePath = $path; + static::$cacheLoaded = false; + } +} + +class NamedRouteController +{ + public function handle(): string + { + return 'handled'; + } +} + class CurrentRouteTest extends TestCase { private MockContainer $container; @@ -94,6 +120,200 @@ public function testRouteNameIsNullForAnUnnamedRoute(): void $this->assertNull($this->router->currentRouteName()); } + public function testRouteNameIsNullWhenOnlyAnotherMethodIsNamed(): void + { + $this->router->get('/posts', fn() => 'index')->name('posts.index'); + $this->router->post('/posts', fn() => 'store'); + + $this->router->getCallback($this->request('POST', '/posts')); + $this->assertNull($this->router->currentRouteName()); + + $this->router->getCallback($this->request('GET', '/posts')); + $this->assertSame('posts.index', $this->router->currentRouteName()); + } + + public function testHeadRequestUsesTheNameOfTheGetRouteServingIt(): void + { + $this->router->get('/posts', fn() => 'index')->name('posts.index'); + + $this->router->getCallback($this->request('HEAD', '/posts')); + + $this->assertSame('posts.index', $this->router->currentRouteName()); + } + + public function testSameNameCoversEveryMethodItWasGivenTo(): void + { + $this->router->get('/posts', fn() => 'form')->name('posts.form'); + $this->router->post('/posts', fn() => 'save')->name('posts.form'); + + foreach (['GET', 'POST'] as $method) { + $this->router->getCallback($this->request($method, '/posts')); + $this->assertSame('posts.form', $this->router->currentRouteName()); + } + + $this->assertSame(['GET' => '/posts', 'POST' => '/posts'], Router::$namedRouteMethods['posts.form']); + } + + public function testNameMovedToAnotherPathLeavesThatPathsOtherRoutesAlone(): void + { + $this->router->get('/a', fn() => 'a')->name('shared'); + $this->router->post('/b', fn() => 'b')->name('shared'); + $this->router->get('/b', fn() => 'b')->name('other'); + $this->router->get('/c', fn() => 'c')->name('shared.c'); + $this->router->post('/c', fn() => 'c')->name('shared.c'); + $this->router->put('/c', fn() => 'c'); + + $this->router->getCallback($this->request('GET', '/b')); + $this->assertSame('other', $this->router->currentRouteName()); + + $this->router->getCallback($this->request('POST', '/b')); + $this->assertSame('shared', $this->router->currentRouteName()); + + $this->router->getCallback($this->request('PUT', '/c')); + $this->assertNull($this->router->currentRouteName()); + $this->assertSame('/b', Router::$namedRoutes['shared']); + } + + public function testConstraintOnOneMethodKeepsASharedNameForTheOther(): void + { + $this->router->get('/posts/{id}', fn() => 'show')->name('posts.item'); + $this->router->post('/posts/{id}', fn() => 'update')->name('posts.item')->where(['id' => '\\d+']); + + foreach (['GET', 'POST'] as $method) { + $this->router->getCallback($this->request($method, '/posts/5')); + $this->assertSame('posts.item', $this->router->currentRouteName()); + } + + $this->assertSame( + ['GET' => '/posts/{id}', 'POST' => '/posts/{id:\\d+}'], + Router::$namedRouteMethods['posts.item'] + ); + } + + public function testNameBeforeAConstraintOnEachMethodKeepsTheSharedName(): void + { + $this->router->get('/posts/{id}', fn() => 'show')->name('posts.item')->where(['id' => '\\d+']); + $this->router->post('/posts/{id}', fn() => 'update')->name('posts.item')->where(['id' => '\\d+']); + + foreach (['GET', 'POST'] as $method) { + $this->router->getCallback($this->request($method, '/posts/5')); + $this->assertSame('posts.item', $this->router->currentRouteName()); + } + } + + /** + * @return array, string}> + */ + public static function attributeRouteProvider(): array + { + return [ + 'no constraint' => [[], '/items/{id}'], + 'with constraint' => [['id' => '\d+'], '/items/{id:\d+}'], + ]; + } + + #[DataProvider('attributeRouteProvider')] + public function testAttributeRouteKeepsItsNameForEveryMethod(array $where, string $path): void + { + $route = new RouteAttribute(uri: '/items/{id}', methods: ['GET', 'POST'], name: 'items.save', where: $where); + + (new \ReflectionMethod(Router::class, 'registerAttributeRoute')) + ->invoke($this->router, NamedRouteController::class, 'handle', $route); + + foreach (['GET', 'POST'] as $method) { + $this->router->getCallback($this->request($method, '/items/5')); + $this->assertSame('items.save', $this->router->currentRouteName()); + } + + $this->assertSame($path, Router::$namedRoutes['items.save']); + } + + public function testConstraintLeavesAnotherMethodsRouteNameAlone(): void + { + $this->router->get('/posts/{id}', fn() => 'show')->name('posts.show'); + $this->router->post('/posts/{id}', fn() => 'update')->where(['id' => '\d+']); + + $this->router->getCallback($this->request('GET', '/posts/5')); + + $this->assertSame('posts.show', $this->router->currentRouteName()); + $this->assertSame('/posts/{id}', Router::$namedRoutes['posts.show']); + } + + public function testConstraintMovesTheNameOfItsOwnRoute(): void + { + $this->router->get('/posts/{id}', fn() => 'show')->name('posts.show')->where(['id' => '\d+']); + + $this->router->getCallback($this->request('GET', '/posts/5')); + + $this->assertSame('posts.show', $this->router->currentRouteName()); + $this->assertSame('/posts/{id:\d+}', Router::$namedRoutes['posts.show']); + } + + public function testRouteCacheKeepsEveryMethodOfAName(): void + { + $this->withRouteCache(function (string $cache): void { + $router = new RouteCacheRouter(new Gateway()); + $router->get('/items', [NamedRouteController::class, 'handle'])->name('items.save'); + $router->post('/items', [NamedRouteController::class, 'handle'])->name('items.save'); + $router->cacheRoutes(); + + $this->resetRoutes(); + $this->assertTrue($router->loadCachedRoutes()); + $this->assertSame(['GET' => '/items', 'POST' => '/items'], Router::$namedRouteMethods['items.save']); + + foreach (['GET', 'POST'] as $method) { + $router->getCallback($this->request($method, '/items')); + $this->assertSame('items.save', $router->currentRouteName()); + } + }); + } + + /** + * @return array}> + */ + public static function olderRouteCacheProvider(): array + { + return [ + 'no methods' => [[]], + 'one method per name' => [['namedRouteMethods' => ['items.index' => 'GET']]], + ]; + } + + #[DataProvider('olderRouteCacheProvider')] + public function testNameFromAnOlderRouteCacheStillResolves(array $format): void + { + $this->withRouteCache(function (string $cache) use ($format): void { + file_put_contents($cache, ' ['GET' => ['/items' => [NamedRouteController::class, 'handle']]], + 'namedRoutes' => ['items.index' => '/items'], + 'routeMiddlewares' => [], + ], true) . ';'); + + $router = new RouteCacheRouter(new Gateway()); + $this->assertTrue($router->loadCachedRoutes()); + + $router->getCallback($this->request('GET', '/items')); + $this->assertSame('items.index', $router->currentRouteName()); + }); + } + + private function withRouteCache(callable $test): void + { + $dir = sys_get_temp_dir() . '/doppar-route-names-' . bin2hex(random_bytes(5)); + mkdir($dir, 0755, true); + RouteCacheRouter::pointTo($dir . '/routes.php'); + + try { + $test($dir . '/routes.php'); + } finally { + foreach (glob($dir . '/*') ?: [] as $file) { + unlink($file); + } + + rmdir($dir); + } + } + public function testCurrentRouteActionKeepsTheDispatchedRouteOfACachedApp(): void { $this->router->routeFile = fn(Router $router) => $router->get('/closure', fn() => 'x')