symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
851 stars 313 forks source link

[LiveComponent] BatchActionController does not properly handle redirection response #1300

Closed 6insanes closed 11 months ago

6insanes commented 11 months ago

Hi!

This is a bug report. Bug relates to BatchActionController and LiveComponentSubscriber::onKernelResponse.

I have simple LiveComponent class UserForm with ComponentWithFormTrait and submit LiveAction which looks like this:

#[LiveAction]
public function submit(EntityManagerInterface $defaultEntityManager): Response
{
    $this->submitForm();
    $defaultEntityManager->flush();

    return $this->redirectToRoute('app_user_index');
}

On the web page we have submit button with data-action="live#action" and data-action-name='prevent|submit' attributes. When changing form data and hitting submit button quickly multiple times I got 500 LogicException from /_components/UserForm/_batch

LogicException:
The submitForm() method is being called, but the FormView has already been built. Are you calling $this->getForm() - which creates the FormView - before submitting the form?

  at vendor/symfony/ux-live-component/src/ComponentWithFormTrait.php:151
  at App\Twig\Components\UserForm->submitForm()
     (src/Twig/Components/UserForm.php:36)
  at App\Twig\Components\UserForm->submit(object(EntityManagerGhostEbeb667))
     (vendor/symfony/http-kernel/HttpKernel.php:181)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 2)
     (vendor/symfony/http-kernel/HttpKernel.php:76)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 2, false)
     (vendor/symfony/ux-live-component/src/Controller/BatchActionController.php:43)
  at Symfony\UX\LiveComponent\Controller\BatchActionController->__invoke(object(Request), object(MountedComponent), 'App\\Twig\\Components\\UserForm', array(array('name' => 'submit', 'args' => array()), array('name' => 'submit', 'args' => array())))
     (vendor/symfony/http-kernel/HttpKernel.php:181)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:76)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:197)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php:35)
  at Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
     (vendor/autoload_runtime.php:29)
  at require_once('/app/vendor/autoload_runtime.php')
     (public/index.php:5)                

I think this happens because in BatchActionController condition on line 45 $response->isRedirection() is never true and loop continues and action is executed multiple times even if LiveAction returns redirection response. Loop continues because LiveComponentSubscriber::onKernelResponse interferes and changes subrequests http response code to Response::HTTP_NO_CONTENT which is 204 and Response::isRedirection method checks for >= 300 and < 400.

Possible solution is to add to LiveComponentSubscriber::onKernelResponse check for $event->isMainRequest() because changing response http code and adding REDIRECT_HEADER makes sense only for main request.

php: 8.1 symfony: 6.3 symfony-ux: 2.13.2

6insanes commented 11 months ago

It can be reproduced with the following test snippet

public function testRedirectWithAcceptHeader(): void
{
    $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions'));

    $this->browser()
        ->throwExceptions()
        ->get('/_components/with_actions', ['query' => ['props' => json_encode($dehydrated->getProps())]])
        ->assertSuccessful()
        ->interceptRedirects()
        ->use(function (Crawler $crawler, KernelBrowser $browser) {
            $rootElement = $crawler->filter('ul')->first();
            $liveProps = json_decode($rootElement->attr('data-live-props-value'), true);

            $browser->post('/_components/with_actions/_batch', [
                'body' => [
                    'data' => json_encode([
                        'props' => $liveProps,
                        'actions' => [
                            ['name' => 'redirect'],
                            ['name' => 'exception'],
                        ],
                    ]),
                ],
                'headers' => [
                    'Accept' => ['application/vnd.live-component+html'],
                    'X-CSRF-TOKEN' => $crawler->filter('ul')->first()->attr('data-live-csrf-value')
                ],
            ]);
        })
        ->assertStatus(204)
        ->assertHeaderContains('X-Live-Redirect', '1')
    ;
}
WebMamba commented 11 months ago

Hey @6insanes! Thanks for the report! I managed to reproduce it locally. Are you up for a PR?