From 64d9ce8e6bcf01e3df47117e45ab9723b6a2e753 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 5 Oct 2026 05:27:42 -0400 Subject: [PATCH 1/2] Check the chained resolver's answer and use the public router RouteNameFallback passed the previous missing-route resolver's answer through unchecked, so a non-string answer was a TypeError inside route() under strict_types; it now reads as not resolved. The scoped-route check also read the URL generator's protected route collection through a bound closure, although Router::has() answers the same question publicly. register() takes the router as an optional second argument, so the old single-argument call keeps working. --- CHANGELOG.md | 8 ++++ src/Providers/InstallerWebServiceProvider.php | 2 +- src/Support/RouteNameFallback.php | 43 ++++++++++++----- tests/Feature/NamingConventionTest.php | 48 +++++++++++++++++++ 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd5d5a1..3209df7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `docs/architecture.md` named the middleware aliases `installer.guard` and `installer.installed`, and both docs pages described a `throttle:60,1` the routes do not carry. +- `RouteNameFallback` passed the previously installed missing-route resolver's + answer through unchecked, so a foreign resolver returning anything but a + string (an object, an int, an array) raised a `TypeError` inside `route()` + under `strict_types`. A non-string answer now reads as "not resolved". +- `RouteNameFallback` asks the public `Router::has()` whether the scoped route + exists instead of reading the URL generator's protected route collection + through a bound closure. `register()` takes the router as an optional second + argument; the single-argument call still works. ## [0.1.0] - 2026-07-11 diff --git a/src/Providers/InstallerWebServiceProvider.php b/src/Providers/InstallerWebServiceProvider.php index fba7da7..138443d 100644 --- a/src/Providers/InstallerWebServiceProvider.php +++ b/src/Providers/InstallerWebServiceProvider.php @@ -89,7 +89,7 @@ public function packageBooted(): void $this->registerRateLimiters(); - RouteNameFallback::register($this->app->make('url')); + RouteNameFallback::register($this->app->make('url'), $this->app->make('router')); } /** diff --git a/src/Support/RouteNameFallback.php b/src/Support/RouteNameFallback.php index b85d8d1..f5ebec7 100644 --- a/src/Support/RouteNameFallback.php +++ b/src/Support/RouteNameFallback.php @@ -5,8 +5,9 @@ namespace Simtabi\Laranail\Installer\Web\Support; use Closure; +use Illuminate\Routing\Router; +use Illuminate\Container\Container; use Illuminate\Routing\UrlGenerator; -use Illuminate\Routing\RouteCollectionInterface; /** * Keeps the pre-0.1.x bare route names (`installer-web.*`) resolving after the @@ -16,7 +17,12 @@ * when a name is NOT found, so it cannot shadow an application's own route. The * generator holds a single resolver, so a resolver registered before this one is * captured and consulted for every name this class does not own — registering - * here never silently discards another package's fallback. + * here never silently discards another package's fallback. That resolver's + * answer is trusted only when it is a string; anything else reads as "not + * resolved", since passing it through is a `TypeError` under strict types. + * + * Whether the scoped route exists is asked of the public `Router`, not read out + * of the generator's protected route collection. * * `Route::has('installer-web.*')` does not go through this hook and answers * false; ask for the scoped name. @@ -32,18 +38,22 @@ final class RouteNameFallback public const string BARE_PREFIX = 'installer-web.'; - public static function register(UrlGenerator $url): void + /** + * @param Router|null $router the router whose routes the generator builds from; + * null resolves the container's `router` (the + * single-argument form callers used before) + */ + public static function register(UrlGenerator $url, ?Router $router = null): void { - $previous = self::read($url, 'missingNamedRouteResolver'); + $router ??= Container::getInstance()->make('router'); + $previous = self::previousResolver($url); $url->resolveMissingNamedRoutesUsing( - static function (string $name, mixed $parameters, ?bool $absolute) use ($url, $previous): ?string { + static function (string $name, mixed $parameters, ?bool $absolute) use ($url, $router, $previous): ?string { if (str_starts_with($name, self::BARE_PREFIX)) { $scoped = self::SCOPED_PREFIX . substr($name, strlen(self::BARE_PREFIX)); - $routes = self::read($url, 'routes'); - - if ($routes instanceof RouteCollectionInterface && $routes->hasNamedRoute($scoped)) { + if ($router->has($scoped)) { trigger_error(sprintf( 'Route name "%s" (laranail/installer-web) is deprecated and will be removed in the next minor after 0.1; use "%s".', $name, @@ -54,16 +64,25 @@ static function (string $name, mixed $parameters, ?bool $absolute) use ($url, $p } } - return is_callable($previous) ? $previous($name, $parameters, $absolute) : null; + if (! is_callable($previous)) { + return null; + } + + $resolved = $previous($name, $parameters, $absolute); + + return is_string($resolved) ? $resolved : null; }, ); } - /** Reads a protected UrlGenerator property; the generator exposes neither through a getter. */ - private static function read(UrlGenerator $url, string $property): mixed + /** + * The generator exposes no getter for its current resolver, so chaining it + * means reading the protected property; this is the one place that does. + */ + private static function previousResolver(UrlGenerator $url): mixed { return Closure::bind( - static fn (UrlGenerator $generator): mixed => $generator->{$property}, + static fn (UrlGenerator $generator): mixed => $generator->missingNamedRouteResolver, null, UrlGenerator::class, )($url); diff --git a/tests/Feature/NamingConventionTest.php b/tests/Feature/NamingConventionTest.php index 9c16c74..d9c77c0 100644 --- a/tests/Feature/NamingConventionTest.php +++ b/tests/Feature/NamingConventionTest.php @@ -112,6 +112,54 @@ function installerWebRouteNames(): array ->and(route('installer-web.index'))->toBe(route('laranail-installer-web.index')); }); +it('treats a non-string answer from the previous resolver as no answer', function (mixed $answer): void { + $url = app('url'); + $url->resolveMissingNamedRoutesUsing(fn (string $name): mixed => $name === 'someone-else.odd' ? $answer : null); + + RouteNameFallback::register($url, app('router')); + + // Unchecked, a foreign object/int/array is a TypeError against the + // fallback's ?string return under strict_types; it must read as "missing". + route('someone-else.odd'); +})->with([ + 'int' => [42], + 'url generator' => [fn (): Illuminate\Routing\UrlGenerator => app('url')], + 'array' => [['http://localhost/elsewhere']], +])->throws(RouteNotFoundException::class); + +it('answers the scoped-route lookup from the public router, not the generator internals', function (): void { + $url = app('url'); + $router = app('router'); + + // Point the generator at an EMPTY collection while the router still holds + // the real one: a fallback reading UrlGenerator::$routes would find no + // scoped route; one asking the public Router finds it. + $real = $router->getRoutes(); + $url->setRoutes(new Illuminate\Routing\RouteCollection); + + RouteNameFallback::register($url, $router); + + $deprecations = []; + set_error_handler(function (int $errno, string $message) use (&$deprecations): bool { + $deprecations[] = $message; + + return true; + }, E_USER_DEPRECATED); + + try { + // The generator then cannot build the URL itself, which surfaces as a + // RouteNotFoundException naming the SCOPED route: proof the lookup + // matched through the router and delegated to the scoped name. + expect(fn (): string => route('installer-web.index')) + ->toThrow(RouteNotFoundException::class, 'laranail-installer-web.index'); + } finally { + restore_error_handler(); + $url->setRoutes($real); + } + + expect($deprecations)->toHaveCount(1); +}); + it('registers its rate limiters under laranail-installer-web.*', function (): void { $names = installerWebLimiterNames(); From e03d6c1760c42f61dd10f110659dae01107a8edf Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Mon, 5 Oct 2026 05:54:31 -0400 Subject: [PATCH 2/2] Import the routing classes in NamingConventionTest CI's Rector run rejected the two tests added in this branch for using inline fully qualified UrlGenerator and RouteCollection. Import them, as Rector's import-names rule and the shared Pint ordering require. --- tests/Feature/NamingConventionTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Feature/NamingConventionTest.php b/tests/Feature/NamingConventionTest.php index d9c77c0..2be37cc 100644 --- a/tests/Feature/NamingConventionTest.php +++ b/tests/Feature/NamingConventionTest.php @@ -5,7 +5,9 @@ use Livewire\Livewire; use Illuminate\Http\Request; use Illuminate\Cache\RateLimiter; +use Illuminate\Routing\UrlGenerator; use Illuminate\Support\Facades\Route; +use Illuminate\Routing\RouteCollection; use Illuminate\Contracts\Console\Kernel; use Simtabi\Laranail\Installer\Web\Livewire\WizardStep; use Simtabi\Laranail\Installer\Web\Support\RouteNameFallback; @@ -123,7 +125,7 @@ function installerWebRouteNames(): array route('someone-else.odd'); })->with([ 'int' => [42], - 'url generator' => [fn (): Illuminate\Routing\UrlGenerator => app('url')], + 'url generator' => [fn (): UrlGenerator => app('url')], 'array' => [['http://localhost/elsewhere']], ])->throws(RouteNotFoundException::class); @@ -135,7 +137,7 @@ function installerWebRouteNames(): array // the real one: a fallback reading UrlGenerator::$routes would find no // scoped route; one asking the public Router finds it. $real = $router->getRoutes(); - $url->setRoutes(new Illuminate\Routing\RouteCollection); + $url->setRoutes(new RouteCollection); RouteNameFallback::register($url, $router);