UrlGenerator warming up issue

[gazzoy]

Octane Version

2.2.7

Laravel Version

10.40.0

PHP Version

8.2.14

What server type are you using?

Swoole

Server Version

5.1.1

Database Driver & Version

No response

Description

Same as #784, #748.

When using Route::bind() and URL::defaults() with Octane, it seems that UrlGenerator keeps the same instance of RouteUrlGenerator, and so the defaults are cached between requests.

Especially, I believe the issue happens when we use App\Http\Middleware\Authenticate to handle login sessions.

Steps To Reproduce

I created a public repo gazzoy/laravel-octane-urlgenerator-issue with a fresh Laravel v10.40.0, Octane v2.2.7(swoole), Sail v1.27.0(php8.2), and the smallest code to reproduce the issue.

1. clone gazzoy/laravel-octane-urlgenerator-issue
2. put an attached .env.txt on your local path as gazzoy/laravel-octane-urlgenerator-issue/.env.
3. run below commands.

$ cd gazzoy/laravel-octane-urlgenerator-issue
$ docker run --rm \
    -u "$(id -u):$(id -g)" \
    -v "$(pwd):/var/www/html" \
    -w /var/www/html \
    laravelsail/php82-composer:latest \
    composer install --ignore-platform-reqs
$ ./vendor/bin/sail build --no-cache
$ ./vendor/bin/sail up -d
$ ./vendor/bin/sail artisan migrate:refresh --seed

4. open your browser with:
   http://localhost/sample/paul/login
   It should show a page with a text "login page for paul" which is expected.

5. open below url on the same browser:
   http://localhost/sample/john/home
   It redirects to http://localhost/sample/paul/login which doesn't make sense to me. Because, as of I understood, if Route::bind() and URL::defaults() work fine, we should be redirected to http://localhost/sample/john/login. Thefore, I believe it seems URL::defaults() keeps it states?

Having said that, I'm not too sure its actually Octane issue, so please let me know if this is not the case.

Just to clarify, my public repo has 3 commits as follows, but the first and second commits were just installing Laravel and Octane so you should be able to ignore it. The third commit is actually my original code to reproduce the issue.

1. Fresh Laravel installation by Sail
2. Installed Octane
3. Added code to reproduce the issue

Please let me know if you have any questions.

[gazzoy]

@nunomaduro Just a kindly reminder - this is the issue we discussed on #784 (comment)

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[Velizz]

I've been able to replicate this issue, and its definitely a bug in source

[gazzoy]

@Velizz Thank you for confirming its a bug!

[driesvints]

Would greatly appreciate any help figuring this one out.

[gazzoy]

@driesvints @nunomaduro

I'm not sure it could be useful, but I added a failing test #869 for this. As described in PR, it has another problem Route [login] not defined which slightly doesn't make sense to me. Couldn't figure out how to fix, so at this point I just leave it there.

[driesvints]

Thanks @gazzoy

[Shkeats]

@gazzoy in your demo app you aren't returning the user inside your Route::bind() closure. I think this could be the cause of your issue? Let me know.

[AlliBalliBaba]

This issue might also be caused by a stale container reference like #887. I'll try to verify on my experimental branch when I have time.

[AlliBalliBaba]

I was wrong, the RouteUrlGenerator actually just keeps its defaults across requests. If that's not intended, then Octane should probably do something like this in an event listener after each request:

$event->sandbox->make('url')->routeGenerator->defaultParameters = [];

Doing so requires additional changes in laravel/framework since the state of the routeGenerator is currently not accessible.

On a sidenote: Visiting the home url in the example of @gazzoy should always throw an error, since the auth middleware fires before the binding of the route is resolved. You would instead pass the username to the redirect in the Authenticate middleware.

[AlliBalliBaba]

More specifically, it would be necessary to bind the state of the RouteUrlGenerator to the $app instance, since when setting a new $request in the UrlGenerator, we would want to keep defaults that were set during the booting of the app, but would want to discard defaults that were set during a request.

It would probably be hard to get such a PR merged since the changes are necessary in laravel/framework, but the tests need to be written in laravel/octane. Not sure how to best move forward here.

[dbpolito]

I opened #896 which should fix this issue... hopefully this gets merged soon, while it's not merged you guys can bring this to your codebase by creating this file:

class CreateUrlSandbox
{
    public function handle($event): void
    {
        $event->sandbox->instance('url', clone $event->sandbox['url']);
    }
}

and add to your octane config:

        RequestReceived::class => [
            ...Octane::prepareApplicationForNextOperation(),
            ...Octane::prepareApplicationForNextRequest(),
            CreateUrlSandbox::class,
        ],

[gazzoy]

I can confirm #896 fix this issue.

In addition to put CreateUrlSandbox as @dbpolito suggested, I tweak App\Http\Middleware\Authenticate as following(mayby @AlliBalliBaba pointed?), then the redirect works as expected.

App\Http\Middleware\Authenticate.php

    protected function redirectTo(Request $request): ?string
    {
-        return $request->expectsJson() ? null : route('sample.login');
+        $user = $request->route()->parameters()['user'];
+        return $request->expectsJson() ? null : route('sample.login', ['user' => $user]);
    }